Page MenuHomeFreeBSD

Avoid unnecessary object locking in vm_page_grab_pages_unlocked().
ClosedPublic

Authored by markj on Sep 2 2020, 7:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 11, 2:07 PM
Unknown Object (File)
Mar 18 2024, 11:38 PM
Unknown Object (File)
Dec 20 2023, 8:05 AM
Unknown Object (File)
Dec 9 2023, 11:43 AM
Unknown Object (File)
Nov 26 2023, 5:38 PM
Unknown Object (File)
Sep 14 2023, 11:54 PM
Unknown Object (File)
Sep 6 2023, 2:42 AM
Unknown Object (File)
Sep 1 2023, 2:35 AM
Subscribers

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 - subversion
Lint
Lint Not Applicable
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.