vn_lock_pair(): allow to request shared locking

[kib]

Summary

If either of vnodes is shared locked, lock must not be recursed.

[mjg]

just a note this is going to need a __freebsd_version bump due to zfs

sys/kern/vfs_vnops.c

3786

does this matter? will not this transparently upgrade to exclusive internally?

point is while lockmgr is the standard thing today, it adds knowledge about it when it perhaps can be avoided

[rmacklem]

LGTM. mjg@ can decide if he needs this for his
patch against zfs_copy_file_range().

If/when it goes into main, I'll take a look at changing
nfs_copy_file_range() to use it as well.

sys/kern/vfs_vnops.c

3786

I think this is needed here for the check
on lkflags1 done at line#3791 just below.

[markj]

sys/kern/vfs_vnops.c

3797

How do you know that the vnode isn't locked by another thread?

[kib]

sys/kern/vfs_vnops.c

3786

Yes the code needs to differentiate between vnode states for real, to properly unlock. If this causes a negative feeling, we can add a predicate function testing for LK_NOSHARE, to hide the implementation.

3797

This is wrong, of course. Hopefully updated version corrects the block.

[kib]

Do not check sharers to detect recursion. Just handle the case where the current lock type does not match the request (i.e. LK_SHARED lock, but need LK_EXCLUSIVE).

[markj]

sys/kern/vfs_vnops.c

3792

I still don't understand how this can work. ASSERT_VOP_UNLOCKED() just looks at the lock state, it does not consult witness.

3814

3815

3835

Can the three lines above be replaced with

while (!vp1_locked || !vp2_locked)

?

[kib]

sys/kern/vfs_vnops.c

3792

Exactly what are you asking about? VOP_UNLOCK() unlock the lock once, if it is recursed, the lock is still owned. ASSERT_VOP_UNLOCKED() is false-positive-safe, in fact it only checks that the lock is not exclusively owned by the current thread.

[kib]

s/vp2/vp1/ in assert
Untangle loop expression

[markj]

sys/kern/vfs_vnops.c

3792

I see, I missed that it only checks for exclusive ownership.