Page MenuHomeFreeBSD

Don't block on the range lock in zfs_getpages().
ClosedPublic

Authored by markj on May 14 2020, 3:25 PM.

Diff Detail

Repository
rS FreeBSD src repository - subversion
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.May 14 2020, 3:25 PM

@avg what do you think of this patch?

Fix a syntax error reported by pho@.

Mark, thank you very much for this!
I think that this change is good.
I still wonder if we can do some trick to avoid busying valid pages.
Maybe there is some way to be more sloppy when checking the validity.
E.g., we could possibly rely on a fact that with ZFS the range lock makes sure the validity cannot change because both page-in and page-out would need to lock the range.

Anyway, I am accepting this and leaving the decision up to you :-)
Thanks!

I ran the few stress2 test cases that uses zfs in a loop for 3 hours. No problems seen.

In D24839#548684, @avg wrote:

Mark, thank you very much for this!
I think that this change is good.
I still wonder if we can do some trick to avoid busying valid pages.
Maybe there is some way to be more sloppy when checking the validity.
E.g., we could possibly rely on a fact that with ZFS the range lock makes sure the validity cannot change because both page-in and page-out would need to lock the range.

The VM has no knowledge of the range lock, so it may still perform operations on ZFS vnode pages concurrently with update_pages(). For example, the page daemon may attempt to reclaim the page while update_pages() is copying from the DMU. The busy lock is the only thing synchronizing them at the moment. Before, the VM object lock also provided this, but obviously at a coarser-grained level.

There are other more esoteric cases, for instance it is possible for msync(MS_INVALIDATE) to mark the pages invalid. Again, synchronization there relies on the page busy lock. So, I do not think we can really avoid the busy lock in ZFS.

Anyway, I am accepting this and leaving the decision up to you :-)
Thanks!

This revision was not accepted when it landed; it landed in state Needs Review.May 20 2020, 6:29 PM
This revision was automatically updated to reflect the committed changes.