Page MenuHomeFreeBSD

Don't hold the object lock across getpages.
ClosedPublic

Authored by jeff on Jan 4 2020, 8:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 28, 2:14 PM
Unknown Object (File)
Feb 6 2024, 2:41 AM
Unknown Object (File)
Feb 5 2024, 12:31 AM
Unknown Object (File)
Dec 23 2023, 7:17 AM
Unknown Object (File)
Dec 20 2023, 7:38 AM
Unknown Object (File)
Dec 15 2023, 3:35 PM
Unknown Object (File)
Dec 14 2023, 5:07 PM
Unknown Object (File)
Oct 31 2023, 7:59 PM
Subscribers

Details

Summary

Only swap and the device pager want the object lock to perform getpages. vnodes do not. Device pagers only really need the object lock for legacy compat, the individual pagers could simply lock the object around vm_page_replace() calls if we wanted to push this down one more level for devices. Most cdev pagers drop the lock for the bulk of their operation. swap could possibly even use its own lock or a large map of locks if we wanted to further optimize and improve situations like unswapped().

This patch is really mechanical and does not push the synchronization change very far into consumers. I have a long series of commits that will improve the consumers one by one. The first is fault here: https://reviews.freebsd.org/D23034

This does now rely completely on exclusive busy and paging in progress to handle concurrency with other object operations.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 28480
Build 26541: arc lint + arc unit

Event Timeline

jeff added reviewers: alc, dougm, kib, markj.
jeff set the repository for this revision to rS FreeBSD src repository - subversion.
sys/dev/md/md.c
1072

Shouldn't we just drop the lock around the get_pages call? If we are reading cached data the lock will be dropped needlessly.

sys/vm/sg_pager.c
158

Can we assert paging_in_progress > 0?

sys/vm/swap_pager.c
1201

Just a nit: the assignment and lock can be reversed.

sys/vm/vm_fault.c
1084

Don' you need to re-evaluate behind/ahead after the relock, or at least clip ahead to potentially trimmed object size ?

sys/dev/md/md.c
1072

We actually only need the object lock in the vm_page_free() case. I didn't realize this was so wonky in this patch. My follow-up is cleaner. I can tidy this up too.

sys/vm/sg_pager.c
158

I can make sure it's part of the vm_pager_getpages asserts.

sys/vm/vm_fault.c
1084

For vnode we were already relying on the vnode lock to protect against truncate.

I need to look at tmpfs to see if that is also true there. For anonymous objects it is not possible that it shrunk.

sys/vm/vm_fault.c
1084

For tmpfs we do not lock the vnode, since the backing object type is swap. The same is true for shmfd that can be truncated.

sys/vm/vm_fault.c
1084

swap pager already drops the lock during the actual pagein which is after the clustering is determined. So this is not really a new race if it exists. I will still see if I can identify how it is handled.

sys/vm/vm_fault.c
1084

Actually vm_fault_prefault() will simply fail to find the pages so this is harmless in any case.

Add an assert and fix a number of cases missing pip. I do not believe they would have caused bugs before but they may after my collapse change.

sys/vm/vm_swapout.c
577

What ensures that the page isn't unswapped after the lock is dropped? swap_pager_unswapped() can be called without the page being busy, for example in the active queue scan.

sys/vm/vm_swapout.c
577

We do valid checks and busy pages before calling get_pages. unswapped can clear dirty but not valid. We are guaranteed the validity won't change while xbusy or vm object locked. No one should be paging in a valid page.

sys/kern/kern_sendfile.c
272

There is one missing pip_wakeup in an early exit case that I fixed on my branch.

This revision is now accepted and ready to land.Jan 13 2020, 8:19 PM
This revision was automatically updated to reflect the committed changes.