vm_page_free_noobj: for freeing into the direct pool
Needs ReviewPublic
Actions

Authored by dougm on May 4 2025, 10:59 PM.

Details

Reviewers

markj
kib
alc
andrew
manu

Summary

Define a function vm_page_free_noobj() for removing FREEPOOL_DIRECT pages, and use it everywhere to free objects allocated with a page_alloc_noobj() function. Make vm_page_free assume that its freed pages are bound for FREEPOOL_DEFAULT, as if allocated with a page_alloc() function. Stop using the vm_page 'pool' field to record in an allocated page which pool it should be returned to when freed. Let the callers choice of free functions determine that instead. This makes pool a field used only for free pages.

Test Plan

I ran stress tests for a while with assertions verifying that the value currently stored in m->pool matched the pool parameter being passed to vm_page_free_toq.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

dougm created this revision.May 4 2025, 10:59 PM

Herald added a reviewer: andrew. · View Herald TranscriptMay 4 2025, 10:59 PM

Herald added a reviewer: andrew. · View Herald Transcript

Herald added a reviewer: manu. · View Herald Transcript

Herald added subscribers: riscv, linuxkpi, glebius and 4 others. · View Herald Transcript

dougm requested review of this revision.May 4 2025, 10:59 PM

Drop a few changes wrongly made.

May I ask you why?

This overall looks very strange to me. For instance, a not uncommon pattern is to allocate a page with _noobj(), and then insert it into the object queue some time later. Now it is wrong to allow the default object deallocation code to terminate object' pages: we must ensure to call free_noobj() on them before.

Another question, why is vm_page_free_zero() now only allowed for pages not belonging to objects?

In D50161#1144220, @kib wrote:

May I ask you why?

To free up a field of vm_page, so that it could conceivably used for some other purpose for allocated pages, and perhaps allow the vm_page to shrink.

This overall looks very strange to me. For instance, a not uncommon pattern is to allocate a page with _noobj(), and then insert it into the object queue some time later.

I wasn't aware of that. Can you give me an example?

Now it is wrong to allow the default object deallocation code to terminate object' pages: we must ensure to call free_noobj() on them before.

No. if a page were allocated from the DIRECT pool, added to an object, and then freed as part of that object, it would currently be returned to the DIRECT pool, and with this change would be returned to the DEFAULT pool. I'm not an expert on the benefits of pools, so I don't know whether this would be an improvement or not.

Another question, why is vm_page_free_zero() now only allowed for pages not belonging to objects?

In all the files where vm_page_free_zero is called, the only vm_page_allocation calls are noobj allocations, so I believe that no one is trying to free_zero a DEFAULT pool page.

In D50161#1144293, @dougm wrote:

In D50161#1144220, @kib wrote:

May I ask you why?

To free up a field of vm_page, so that it could conceivably used for some other purpose for allocated pages, and perhaps allow the vm_page to shrink.

Removing that field on its own won't have any impact on the size of struct vm_page, as its size must be a multiple of the platform pointer size. Most of the bits in the pool field are already effectively free for use for allocated pages, since the pool field is only used to encode one of two values. The same is true of the psind field, for instance, and we have free bits elsewhere too.

Do you have some specific alternate use in mind for these fields? If not, it's not clear to me that the churn is worth it in order to free up one or two bits that we already have available elsewhere.