shared-locked lookup for ".." on nullfs

[mjg]

Summary

This shows up with poudriere where the ports tree is notorious for ".." lookups. As the tree is globally shared over numerous nullfs mounts, this effectively globally serializes said lookups.

commit 72680c810e086b0a5a2c30a12c576a42dd7579c9 (mjgzoo/nullfs5)
Author: Mateusz Guzik <[e-mail]>
Date:   Sat Sep 18 12:46:03 2021 +0200

    vfs: retire the no longer used MNTK_LOOKUP_EXCL_DOTDOT flag

commit f0603aeae7a1b29fe6b4974a6bb926b2355d199d
Author: Mateusz Guzik <[e-mail]>
Date:   Mon Mar 7 11:43:43 2022 +0000

    nullfs: hash insertion without vnode lock upgrade
    
    Use the hash lock to serialize instead.
    
    This enables shared-locked ".." lookups.

commit 126df9ffccb75521e79327f171fcc27a7b9af210
Author: Mateusz Guzik <[e-mail]>
Date:   Mon Mar 7 11:40:04 2022 +0000

    vfs: add MNTK_UNLOCKED_INSMNTQUE
    
    Can be used when the fs at hand can synchronize insmntque with other
    means than the vnode lock.

Test Plan

already tested by pho

[jah]

sys/kern/vfs_subr.c

1968–1974

in the failure case, this function calls vput() if a destructor is present.

More generally, what is the purpose of insmntque() requiring the vnode to be locked in the first place?
Mount list and vnode state synchronization should already be ensured by the fact that insmntque1_int() acquires the mount and vnode interlocks. Is it simply to enforce that each filesystem has a path during which it can safely perform local vnode initialization without fear of the vnode being concurrently reclaimed?

[mjg]

sys/kern/vfs_subr.c

1968–1974

Well it is a programming error to pass a destructor in this case, I can add an assert.

Locking here is mostly to prevent exposing a partially constructed vnode (in this case missing ->v_mount).

1968–1974

... for example the vnode can be reachable via other means and the other code may only take the vnode lock.

[markj]

sys/fs/nullfs/null_subr.c

265 ↗

(On Diff #103567)

In the absence of an exclusive vnode lock, shouldn't this initialization be done before making the vnode visible in the mountpoint's list?

[mjg]

• assert on !dtr
• finish initing the vnode before calling instmntque
• stop testing for already set v_object and VIRF_READ -- the vnode is freshly allocated

[mjg]

sys/kern/vfs_subr.c

5040

syncer inherits the flag from null mount so this is needed to avoid running into the assert. it panics on failure anyway. arguably syncer vnode for nullfs should not be getting allocated to begin with, will maybe change that later.

[mjg]

• add back context

[markj]

sys/kern/vfs_subr.c

1968–1974

Why is it "unlocked"? We still own the shared vnode lock in the nullfs case.

[mjg]

sys/kern/vfs_subr.c

1968–1974

the point was to allow non-locked operation to begin later on, the shared lock does not violate it per se, key point being there is no exclusive lock