vm_phys: reduce touching of page->pool fields

[dougm]

Summary

Change the usage of the pool field in vm_page structs.

Currently, every page belongs to a pool, and the pool field identifies that pool, whether the page is allocated or free.

With this change, the pool field of the first page of a free block is used by the buddy allocator to identify its pool, but the buddy allocator makes no guarantees about the pool field value for allocated pages. The buddy allocator requires that a pool parameter be passed as part of freeing memory. A function that allocates memory may use the pool field of a page to record what pool to pass as that parameter when the memory is freed, but might not need to do so for every allocated page.

Test Plan

Running buildworlds.

[dougm]

In response to offline feedback, reduce needless changes to the pool field, where it is changed from A to B and then back to A.

[dougm]

Break a too-long line.

[dougm]

Add a KASSERT.

[dougm]

In vm_phys_free_pages, I'd be happy to make a change that might not be strictly in the spirit of this change.

I note that m is not dereferenced in the inner do-while loop. It is only computed. So, if it weren't computed in the loop, then after the loop you could have

if (order < VM_NFREEORDER - 1)

pa ^= ((vm_paddr_t)1 << (PAGE_SHIFT + order));

m = &seg->first_page[...];

Then you could replace m_buddy with m.

[dougm]

Avoid an unneeded write to pool in phys_free_pages.

[dougm]

Drop an unneeded assignment to ->pool in _unfree().

[dougm]

Resolve conflicts that arise when ilog2 was introduced to vm_phys.c.

[dougm]

Update after FREEPOOL_LAZYINIT changes.

[dougm]

Remove a blank line.

[dougm]

Update to resolve a vm_reserv.c conflict.

[dougm]

The overall timing results of running a dozen or so make-worlds without (base) and with (pool) this change:

x base.time
+ pool.time
+------------------------------------------------------------------------------+
|+x ++  x+   +    +   x+   +* xx   + xx x  x    x  ++       +                 x|
|    |__________|______M_A_________A_M_______|_______|                         |
+------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x  13       3794.47       3828.59       3810.03     3809.0946     8.4516533
+  13       3794.02       3820.52       3804.11     3804.8223     8.8034663
No difference proven at 95.0% confidence

[dougm]

Remove a needless assignment to m->pool

[dougm]

Reran 13 buildworlds after one-line change; here are the results, compared to the same 13 buildworlds on unmodified kernel:

x base.time
+ pool.time
+------------------------------------------------------------------------------+
|                             +                                                |
|                             +            x        +                          |
|x     x             x++  +x +*x  + x x x  x  + x + +      +                  x|
|              |_________|____M___A_MA____________|__|                         |
+------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x  13       3794.47       3828.59       3810.03     3809.0946     8.4516533
+  13       3803.98       3820.27       3807.45     3810.5046      5.579744
No difference proven at 95.0% confidence

[dougm]

Let vm_phys_alloc_npages clear all pool values in allocated memory, with the function vm_phys_free_npages supplying the pool as a parameter.

[dougm]

Updated stats comparing base buildworld times with times after applying this patch:

x base.time
+ pool3.time
+------------------------------------------------------------------------------+
|                                 +                                            |
|                                 +                                            |
|+ +     x  x  x + x+ x+   x   x  +    * + x    *x+   x +                     x|
|             ||_______________A__M_____________|_____|                        |
+------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x  13       3799.46       3826.48       3807.97       3809.38     7.7810464
+  13       3796.29       3817.71       3809.23       3807.95     6.7123679
No difference proven at 95.0% confidence

[dougm]

base - unmodified, pool-with these changes

x base
+ pool
+------------------------------------------------------------------------------+
|                                          +         x                         |
|+            + +x *+        x  x  +*   x  +x    +*  x x x    +     +         x|
|              |__________|________AM______AM__________|____|                  |
+------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x  13       3800.14       3820.84       3809.25     3809.0408     5.7418775
+  13       3794.66       3817.48       3806.45     3806.2346     6.7908745
No difference proven at 95.0% confidence

[dougm]

No changes, just an update to account for some lines that just got renumbered.

[alc]

This is much better.

[dougm]

Refresh.

[dougm]

Rebase. Resolve a conflict.

[dougm]

Fix unused variable problem.

[markj]

I don't see any particular problems with the patch.

sys/vm/vm_kern.c

956

We could hard-code VM_FREEPOOL_DEFAULT here, like we do in vm_page_startup() ("Initialize pages not covered by phys_avail[]...").

I would suggest keeping the code as it is, but it's something to keep in mind if we want to further reduce use of the pool field.

sys/vm/vm_phys.c

804–805

sys/vm/vm_reserv.c

483–484

I believe we can assume here that the pool is VM_FREEPOOL_DEFAULT; note that the pool is hard-coded in the vm_phys_alloc_pages() call elsewhere in this file. In particular, pages allocated from reservations always belong to a VM object.

958–964

This can be VM_FREEPOOL_DIRECT.

[dougm]

Crash report from pho:

20250123 02:33:01 all (549/951): mmap41.sh
panic: Bad tailq NEXT(0xffffffff81ce3438->tqh_last) != NULL
cpuid = 0
time = 1737596106
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe01821523b0
vpanic() at vpanic+0x136/frame 0xfffffe01821524e0
panic() at panic+0x43/frame 0xfffffe0182152540
vm_phys_free_pages() at vm_phys_free_pages+0x31c/frame 0xfffffe01821525a0
vm_page_zone_release() at vm_page_zone_release+0x70/frame 0xfffffe01821525f0
bucket_drain() at bucket_drain+0x11c/frame 0xfffffe0182152630
zone_put_bucket() at zone_put_bucket+0x1c0/frame 0xfffffe0182152680
cache_free() at cache_free+0x2b4/frame 0xfffffe01821526d0
uma_zfree_arg() at uma_zfree_arg+0x207/frame 0xfffffe0182152720
pctrie_reclaim_resume_cb() at pctrie_reclaim_resume_cb+0x9c/frame 0xfffffe0182152770
vm_object_terminate() at vm_object_terminate+0x132/frame 0xfffffe01821527b0
vm_object_deallocate() at vm_object_deallocate+0x2bb/frame 0xfffffe01821527e0
vm_map_process_deferred() at vm_map_process_deferred+0xa9/frame 0xfffffe0182152800
vm_map_remove() at vm_map_remove+0xcb/frame 0xfffffe0182152830
vmspace_exit() at vmspace_exit+0xa8/frame 0xfffffe0182152860
exit1() at exit1+0x533/frame 0xfffffe01821528d0
sigexit() at sigexit+0x19f/frame 0xfffffe0182152d50
postsig() at postsig+0x1a5/frame 0xfffffe0182152e20
ast_sig() at ast_sig+0x32b/frame 0xfffffe0182152ed0
ast_handler() at ast_handler+0xe8/frame 0xfffffe0182152f10
ast() at ast+0x20/frame 0xfffffe0182152f30
doreti_ast() at doreti_ast+0x1c/frame 0x309c5c2596c0

https://people.freebsd.org/~pho/stress/log/log0569.txt