Page MenuHomeFreeBSD

vm_page: Stop handling VM_ALLOC_NOOBJ in vm_page_alloc_domain_after()
ClosedPublic

Authored by markj on Sep 21 2021, 2:43 AM.

Details

Summary

Also modify the function to unconditionally preserve PG_ZERO, so
VM_ALLOC_ZERO is effectively ignored.

MFC after: never

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj requested review of this revision.Sep 21 2021, 2:43 AM
kib added inline comments.
sys/vm/vm_page.c
2143

I do not think () are needed.

This revision is now accepted and ready to land.Sep 21 2021, 12:36 PM
sys/vm/vm_page.c
2073–2079

I wonder if it makes sense to add initial assert that checks that req contains only flags that the function can handle. For this and all other vm_page_alloc* functions.

markj added inline comments.
sys/vm/vm_page.c
2073–2079

Yes, I think it's a good idea.

  • Add assertions to exclude all unexpected flags.
  • Add VM_ALLOC_COUNT_MASK.
  • Ensure that vm_page_grab() does not pass VM_ALLOC_IGN_SBUSY to allocator functions.
This revision now requires review to proceed.Sep 21 2021, 3:13 PM

Mask off VM_ALLOC_IGN_SBUSY in vm_page_grab_valid() as well.

kib added inline comments.
sys/vm/vm_page.h
553

Could this be VM_ALLOC_COUNT(VM_ALLOC_COUNT_MAX - 1) and count < VM_ALLOC_COUNT_MAX could be used for assert in VM_ALLOC_COUNT?

This revision is now accepted and ready to land.Sep 22 2021, 3:23 PM
sys/vm/vm_page.h
553

Are you suggesting using a statement expression there, e.g.,

#define VM_ALLOC_COUNT(count) ({ MPASS(count < VM_ALLOC_COUNT_MAX); count << VM_ALLOC_COUNT_SHIFT; })

?

sys/vm/vm_page.h
553

Yes something along this line for VM_ALLOC_COUNT(). If vm/vm_page.h is used in usermode (even rarely) then __extension__ would be useful/needed as well.

markj marked an inline comment as done.

Add an assertion in VM_ALLOC_COUNT().

This revision now requires review to proceed.Sep 24 2021, 4:04 PM
markj added inline comments.
sys/vm/vm_page.h
553

This chunk is kernel-only.

I am not 100% sure about adding the assertion, it means that VM_ALLOC_COUNT() cannot be assigned to a global var. But I don't think this is likely to arise...

This revision is now accepted and ready to land.Sep 24 2021, 8:21 PM
alc added inline comments.
sys/vm/vm_page.c
2088–2089

We really don't need this variable any longer.

Drop the "pool" variable.

This revision now requires review to proceed.Oct 17 2021, 8:50 PM
This revision was not accepted when it landed; it landed in state Needs Review.Oct 20 2021, 1:23 AM
This revision was automatically updated to reflect the committed changes.