Page MenuHomeFreeBSD

assorted small fixes to unionfs_lock()
ClosedPublic

Authored by jah on Jan 30 2022, 9:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 13 2024, 12:28 AM
Unknown Object (File)
Feb 5 2024, 4:04 PM
Unknown Object (File)
Dec 20 2023, 6:40 AM
Unknown Object (File)
Dec 5 2023, 11:21 AM
Unknown Object (File)
Nov 29 2023, 9:17 PM
Unknown Object (File)
Nov 6 2023, 11:53 PM
Unknown Object (File)
Nov 4 2023, 9:08 PM
Unknown Object (File)
Oct 5 2023, 10:52 PM
Subscribers

Details

Summary

unionfs: fix assertion order in unionfs_lock()

VOP_LOCK() may be handed a vnode that is concurrently reclaimed.
unionfs_lock() accounts for this by checking for empty vnode private
data under the interlock. But it incorrectly asserts that the vnode
is using the unionfs dispatch table before making this check.
Reverse the order, and also update KASSERT_UNIONFS_VNODE() to provide
more useful information.

Reported by: pho

unionfs: allow lock recursion when reclaiming the root vnode

The unionfs root vnode will always share a lock with its lower vnode.
If unionfs was mounted with the 'below' option, this will also be the
vnode covered by the unionfs mount. During unmount, the covered vnode
will be locked by dounmount() while the unionfs root vnode will be
locked by vgone(). This effectively requires recursion on the same
underlying like, albeit through two different vnodes.

Reported by: pho

unionfs: do not force LK_NOWAIT if VI_OWEINACT is set

I see no apparent need to avoid waiting on the lock just because
vinactive() may be called on another thread while the thread that
cleared the vnode refcount has the lock dropped. In fact, this
can at least lead to a panic of the form "vn_lock: error <errno>
incompatible with flags" if LK_RETRY was passed to VOP_LOCK().
In this case LK_NOWAIT may cause the underlying FS to return an
error which is incompatible with LK_RETRY.

Reported by: pho

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jah requested review of this revision.Jan 30 2022, 9:14 PM
sys/fs/unionfs/union_vnops.c
1956

@kib @markj can you think of a reason why LK_NOWAIT might have been added here?
I see no apparent requirement for this in the VFS, and of course no one ever bothered to comment this non-obvious piece of code.

jah retitled this revision from unionfs: fix assertion order in unionfs_lock() to assorted small fixes to unionfs_lock().Jan 31 2022, 1:56 AM
jah edited the summary of this revision. (Show Details)
kib added inline comments.
sys/fs/unionfs/union_subr.c
446 ↗(On Diff #102136)
sys/fs/unionfs/union_vnops.c
1956

I do not know why this hack was done.

This revision is now accepted and ready to land.Jan 31 2022, 2:34 AM
markj added inline comments.
sys/fs/unionfs/union_vnops.c
79–80

Perhaps use VNASSERT?

1956

I can't see a reason either.

This revision now requires review to proceed.Jan 31 2022, 5:02 PM
This revision is now accepted and ready to land.Jan 31 2022, 5:20 PM

D34109.102171.patch works for me.