Page MenuHomeFreeBSD

Avoid unnecessary object locking in vm_page_grab_pages_unlocked().
ClosedPublic

Authored by markj on Sep 2 2020, 7:35 PM.

Details

Summary

Conrad pointed out that we are needlessly acquiring the object lock to
call vm_page_grab_pages() even when all of the requested pages were
looked up locklessly. Fix that, and add assertions to help catch this
kind of mistake.

Reported by: cem

Diff Detail

Repository
rS 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 2 2020, 7:35 PM
markj created this revision.
This revision is now accepted and ready to land.Sep 2 2020, 7:38 PM
sys/vm/vm_page.c
4732 ↗(On Diff #76560)

If count>0, what is this test for?

sys/vm/vm_page.c
4732 ↗(On Diff #76560)

Thanks. It was added in the initial commit of vm_page_grab_pages(). All callers of the function now specify count > 0, so we can remove it.

This revision now requires review to proceed.Sep 2 2020, 7:53 PM
This revision is now accepted and ready to land.Sep 2 2020, 7:56 PM
head/sys/vm/vm_page.c
4785

.. but why making the count == 0 case special ? If a caller needs that, why create more burden to use VM KPI ?

4809

This is fine ...

head/sys/vm/vm_page.c
4785

None of the 6 callers of vm_page_grab_pages() in stable/12 will ever specify count == 0. I can't really see a scenario where handling count == 0 would be useful, so I'm inclined to believe that it is better to disallow count == 0 so we can easily catch bugs like this one.