Page MenuHomeFreeBSD

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

Authored by markj on Sep 16 2021, 4:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 3, 1:09 AM
Unknown Object (File)
Sun, Mar 3, 1:09 AM
Unknown Object (File)
Sun, Mar 3, 1:08 AM
Unknown Object (File)
Sun, Mar 3, 1:08 AM
Unknown Object (File)
Sun, Mar 3, 1:08 AM
Unknown Object (File)
Sun, Mar 3, 1:08 AM
Unknown Object (File)
Sun, Mar 3, 1:04 AM
Unknown Object (File)
Sun, Mar 3, 1:04 AM

Details

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 41937
Build 38825: arc lint + arc unit

Event Timeline

sys/i386/i386/pmap.c
2086–2087

Eliminate m while there?

hselasky added a subscriber: hselasky.

Looks good from my point of view.

This revision is now accepted and ready to land.Sep 17 2021, 7:43 AM
markj marked an inline comment as done.

Eliminate a local var in the i386 pmap.

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

Drop unused VM_ALLOC_NOBUSY from consumers.

This revision now requires review to proceed.Sep 17 2021, 7:09 PM
sys/amd64/amd64/pmap.c
11386–11388

This doesn't look right.

markj marked an inline comment as done.

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

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.

This revision is now accepted and ready to land.Oct 4 2021, 1:41 AM
alc added inline comments.
sys/amd64/amd64/pmap.c
4353

As an aside, this comment should explain why 'pmap' isn't passed to 'pmap_alloc_pt_page'.

4385–4386

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.

4390

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.

sys/amd64/amd64/pmap.c
4353

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()).

4385–4386

Here it makes a bit more sense that the page isn't counted.

4390

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.

sys/amd64/amd64/pmap.c
4385–4386

I do not see why, it is reasonable to account for all page table pages, even if we have two page tables.

4390

Yes it is bug. Would you add VM_ALLOC_ZERO as part of this patch or should we do a separate patch?

sys/amd64/amd64/pmap.c
4385–4386

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.

sys/amd64/amd64/pmap.c
4353

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.

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.