Details

Reviewers

alc
kib
manu
• hselasky

Commits

rGa4667e09e652: Convert vm_page_alloc() callers to use vm_page_alloc_noobj().

Summary

Remove page zeroing code from consumers and stop specifying
VM_ALLOC_NOOBJ. In a few places I also converted an allocation loop to
use VM_ALLOC_WAITOK.

Similarly, convert vm_page_alloc_domain() callers.

Note that callers are now responsible for assigning the pindex.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 41909
Build 38797: arc lint + arc unit

Event Timeline

markj created this revision.Sep 16 2021, 4:56 PM

Herald added a reviewer: manu. · View Herald TranscriptSep 16 2021, 4:56 PM

Herald added subscribers: linuxkpi, emaste, jhibbits and 2 others. · View Herald Transcript

markj requested review of this revision.Sep 16 2021, 4:56 PM

markj added a parent revision: D31985: vm_page: Add a new page allocator interface for anonymous pages.

Harbormaster completed remote builds in B41545: Diff 95254.Sep 16 2021, 4:56 PM

kib added inline comments.Sep 16 2021, 8:08 PM

sys/i386/i386/pmap.c
2088	Eliminate m while there?

Looks good from my point of view.

This revision is now accepted and ready to land.Sep 17 2021, 7:43 AM

Eliminate a local var in the i386 pmap.

This revision now requires review to proceed.Sep 17 2021, 1:16 PM

Harbormaster completed remote builds in B41563: Diff 95284.Sep 17 2021, 1:16 PM

• hselasky accepted this revision.Sep 17 2021, 1:47 PM

This revision is now accepted and ready to land.Sep 17 2021, 1:47 PM

kib accepted this revision.Sep 17 2021, 5:53 PM

Drop unused VM_ALLOC_NOBUSY from consumers.

This revision now requires review to proceed.Sep 17 2021, 7:09 PM

Harbormaster completed remote builds in B41575: Diff 95316.Sep 17 2021, 7:09 PM

alc added inline comments.Sep 18 2021, 6:01 PM

sys/amd64/amd64/pmap.c
11390–11392	This doesn't look right.

Fix a couple of callers that weren't converted properly.

Harbormaster completed remote builds in B41593: Diff 95343.Sep 18 2021, 6:14 PM

Tinderbox fixes.

markj mentioned this in D28810: Let the VM page allocator handle page zeroing.Oct 1 2021, 9:49 PM

Harbormaster completed remote builds in B41909: Diff 96117.Oct 1 2021, 9:49 PM

markj added a child revision: D32005: Introduce vm_page_alloc_noobj_contig().Oct 1 2021, 9:55 PM

There are a number of places where VM_ALLOC_NORMAL is missing, but implied. And, given its definition (as 0), we have no way of enforcing its use. So, I'm ready to suggest that we simply eliminate it.

Drop uses of VM_ALLOC_NORMAL from vm_page_alloc_noobj() and
vm_page_alloc_noobj_domain() callers.

Harbormaster completed remote builds in B41937: Diff 96188.Oct 4 2021, 12:02 AM

kib accepted this revision.Oct 4 2021, 1:41 AM

This revision is now accepted and ready to land.Oct 4 2021, 1:41 AM

alc accepted this revision.Oct 16 2021, 8:37 PM

alc added inline comments.

sys/amd64/amd64/pmap.c
4354	As an aside, this comment should explain why 'pmap' isn't passed to 'pmap_alloc_pt_page'.
4386–4388	As an aside, as above this deserves a comment (as part of a separate patch) so that readers don't think it is an accounting bug.
4392	I feel like I'm missing something here. We only appear to initialize one PTE within the pml5 page and the contents of the rest of the PTEs within the page are unknown.
sys/dev/xen/balloon/balloon.c
237	As an aside, it's curious that we specify "nodump" in the virtio driver but not here.

markj added inline comments.Oct 16 2021, 9:10 PM

sys/amd64/amd64/pmap.c
4354	Is it only because the pmap stats fields are not initialized at this point? If so, I wonder if we should instead initialize them first and count this page (with corresponding changes to pmap_release()).
4386–4388	Here it makes a bit more sense that the page isn't counted.
4392	I think this is a bug.
sys/dev/xen/balloon/balloon.c
237	I can't see a reason for virtio to specify NODUMP: it doesn't call dump_add_page() and I can't see how or why virtio balloon pages would ever be mapped.

kib added inline comments.Oct 16 2021, 9:17 PM

sys/amd64/amd64/pmap.c
4386–4388	I do not see why, it is reasonable to account for all page table pages, even if we have two page tables.
4392	Yes it is bug. Would you add VM_ALLOC_ZERO as part of this patch or should we do a separate patch?

markj added inline comments.Oct 16 2021, 9:49 PM

sys/amd64/amd64/pmap.c
4386–4388	Ah, pmap_resident_count_adj() asserts that the pmap mutex is held, but at this point it is not. And it doesn't seem very useful to manually adjust the count.

alc added inline comments.Oct 16 2021, 11:30 PM

sys/amd64/amd64/pmap.c
4354	No. Think about the early termination optimization in pmap_remove(). For it to work, the root-level PTPs allocated in this function can't be counted in the pmap's resident.

alc added inline comments.Oct 16 2021, 11:53 PM

sys/dev/xen/balloon/balloon.c
237	Yes, you are right. There is no reason for the virtio balloon driver to pass the "no dump" flag.

ehem_freebsd_m5p.com mentioned this in D28982: xen: move common variables and code off of sys/x86/xen/hvm.c.Oct 21 2021, 10:07 PM

Closed by commit rGa4667e09e652: Convert vm_page_alloc() callers to use vm_page_alloc_noobj(). (authored by markj). · Explain WhyNov 3 2021, 4:16 AM

This revision was automatically updated to reflect the committed changes.

markj added a commit: rGa4667e09e652: Convert vm_page_alloc() callers to use vm_page_alloc_noobj()..

Convert vm_page_alloc() callers to use vm_page_alloc_noobj().
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 96117

sys/amd64/amd64/mp_machdep.c

sys/amd64/amd64/pmap.c

sys/amd64/amd64/uma_machdep.c

sys/arm/arm/pmap-v6.c

sys/arm64/arm64/pmap.c

sys/arm64/arm64/uma_machdep.c

sys/arm64/iommu/iommu_pmap.c

sys/compat/linuxkpi/common/src/linux_page.c

sys/dev/drm2/ttm/ttm_page_alloc.c

sys/dev/ti/if_ti.c

sys/dev/virtio/balloon/virtio_balloon.c

sys/dev/xen/balloon/balloon.c

sys/dev/xen/gntdev/gntdev.c

sys/fs/nfs/nfs_commonsubs.c

sys/fs/nfsclient/nfs_clrpcops.c

sys/i386/i386/pmap.c

sys/kern/kern_mbuf.c

sys/kern/uipc_ktls.c

sys/kern/uipc_mbuf.c

sys/kern/vfs_bio.c

sys/mips/mips/pmap.c

sys/mips/mips/uma_machdep.c

sys/powerpc/aim/mmu_oea64.c

sys/powerpc/aim/mmu_radix.c

sys/powerpc/booke/pmap_32.c

sys/powerpc/booke/pmap_64.c

sys/powerpc/powerpc/uma_machdep.c

sys/riscv/riscv/pmap.c

sys/riscv/riscv/uma_machdep.c

sys/vm/uma_core.c

sys/vm/vm_kern.c

sys/vm/vm_page.c

Convert vm_page_alloc() callers to use vm_page_alloc_noobj().ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 96117

sys/amd64/amd64/mp_machdep.c

sys/amd64/amd64/pmap.c

sys/amd64/amd64/uma_machdep.c

sys/arm/arm/pmap-v6.c

sys/arm64/arm64/pmap.c

sys/arm64/arm64/uma_machdep.c

sys/arm64/iommu/iommu_pmap.c

sys/compat/linuxkpi/common/src/linux_page.c

sys/dev/drm2/ttm/ttm_page_alloc.c

sys/dev/ti/if_ti.c

sys/dev/virtio/balloon/virtio_balloon.c

sys/dev/xen/balloon/balloon.c

sys/dev/xen/gntdev/gntdev.c

sys/fs/nfs/nfs_commonsubs.c

sys/fs/nfsclient/nfs_clrpcops.c

sys/i386/i386/pmap.c

sys/kern/kern_mbuf.c

sys/kern/uipc_ktls.c

sys/kern/uipc_mbuf.c

sys/kern/vfs_bio.c

sys/mips/mips/pmap.c

sys/mips/mips/uma_machdep.c

sys/powerpc/aim/mmu_oea64.c

sys/powerpc/aim/mmu_radix.c

sys/powerpc/booke/pmap_32.c

sys/powerpc/booke/pmap_64.c

sys/powerpc/powerpc/uma_machdep.c

sys/riscv/riscv/pmap.c

sys/riscv/riscv/uma_machdep.c

sys/vm/uma_core.c

sys/vm/vm_kern.c

sys/vm/vm_page.c

Convert vm_page_alloc() callers to use vm_page_alloc_noobj().
ClosedPublic
Actions

Revision Contents
Changeset List