Page MenuHomeFreeBSD

Eliminate some needless attempts to merge buddies
Needs ReviewPublic

Authored by on Aug 25 2018, 5:50 PM.



In vm_phys_free_contig, many calls are made to vm_phys_free_pages, which attempts to merge free blocks with buddies to form larger free blocks. We can replace most of those calls with direct calls to vm_freelist_add that do not attempt merging, and call vm_phys_free_pages at most twice to get all the merging that is possible.

Diff Detail

Lint Skipped
Unit Tests Skipped

Event Timeline created this revision.Aug 25 2018, 5:50 PM
kib added a comment.EditedSep 1 2018, 11:57 AM

Please provide the diff context.

I am curious how large impact this change has, do you observe situations where it really makes the difference ? In other words, how often is vm_phys_free_pages() called for the run of >= VM_NFREEORDER ? Is it for free of superpages ?


I think this comment is mis-placed. It seems that it should be near the call to vm_phys_nomerge_free_contig().


Continuation line should use +4 spaces for intent.

I would put this min() calculation into a helper function and moved the long comment above to it.

markj added inline comments.Sep 1 2018, 4:21 PM

m_start would seem to be a more common name for the first page in a contiguous range.

Apply reviewer-recommended changes.

kib accepted this revision.Sep 1 2018, 10:02 PM
kib added inline comments.

return (min(....));

This revision is now accepted and ready to land.Sep 1 2018, 10:02 PM

Add parens, as requested.

This revision now requires review to proceed.Sep 1 2018, 11:41 PM
alc added inline comments.Sep 8 2018, 6:26 PM

There is an undocumented assumption here and in the original vm_phys_free_contig() code: That the contiguous pages don't cross a vm_phys_segs[] boundary. Right now, this function is only called in two places: vm_page_startup() and vm_reserv_break(). In both cases, we know that the contiguous pages won't cross a vm_phys_segs[] boundary. Moreover, we know that coalescing won't occur. In other words, the two calls to vm_phys_free() on the boundaries aren't needed to handle the possibility of coalescing.

When I first talked to Doug about implementing this optimization, I was reluctant to provide a public function that requires the caller to know that coalescing can't occur. However, we already require the caller to know that the contiguous pages won't cross a vm_phys_segs[] boundary, so there is already one unusual requirement on the caller. Given that, should we just explicitly document the requirements, expose this non-coalescing version to callers, and use it in vm_page_startup() and vm_reserv_break()?

kib added inline comments.Sep 12 2018, 2:17 PM

I think it is not hard to assert that m->segind is not changing.

This patch was ready to land, briefly, 6.5 months ago. This is just a refresh to account for unrelated changes to the modified file in the last 6.5 months.