vm_fault_busy_sleep() tests to see whether the page fs->m matches the value looked up at fs->pindex. At that point, a lock is held on fs->object, and it has been held since before vm_fault_object() also looked up fs->pindex in fs->object and stored the result in fs->m. So the values must match, and the test is not necessary. Drop it.
Details
- Reviewers
alc markj kib - Commits
- rG2d6185cf87e8: vm_fault: drop never-true busy_sleep test
Replacing the useless test with an assertion, and asserting that the two values match instead, produced no assertion failures in running the first 204 (and counting) stress2 tests.
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Then why not convert the check into the assertion in the final patch too? IMO it is good to have, since the path from lookup to this busy_sleep() line is somewhat long.
This lookup originated here:
commit 87b646630c4892e21446cd096bea6bcaecea33ac Author: Mark Johnston <markj@FreeBSD.org> Date: Mon Nov 15 11:35:44 2021 -0500 vm_page: Consolidate page busy sleep mechanisms - Modify vm_page_busy_sleep() and vm_page_busy_sleep_unlocked() to take a VM_ALLOC_* flag indicating whether to sleep on shared-busy, and fix up callers. - Modify vm_page_busy_sleep() to return a status indicating whether the object lock was dropped, and fix up callers. - Convert callers of vm_page_sleep_if_busy() to use vm_page_busy_sleep() instead. - Remove vm_page_sleep_if_(x)busy(). No functional change intended. Obtained from: jeff (object_concurrency patches) Reviewed by: kib MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D32947
This lookup was first introduced between stable/4 and stable/6. The lookup was needed in the latter because we were dropping and reacquiring the object lock around the map unlock and possible vput().
I don't want to vm_page_lookup appear here without cause, since it would have to be changed to vm_page_lookup_readonly in a future potential change.
Because I don't want to have to change this function again in a few days when/if vm_page_lookup_readonly is introduced.