autofs: enable witness for autofs node lock

[rew]

Summary

Previously, an_vnode_lock was initialized with SX_NOWITNESS to silence
lock order reversals. The reversals would occur when autofs_node_vn()
was called with the directory vnode lock held, then lock an_vnode_lock,
then lock the vnode attached to the autofs node. It looked like:

directory vnode -> an_vnode_lock -> vnode attached to autofs node

The established lock order is now vnode -> an_vnode_lock

Currently, we don't have to worry about losing an autofs node during the
unlock/lock as autofs nodes are only removed during an unmount() after
vflush(). When autofs_node_vn() is called, the mountpoint has either
been busied (preventing unmount) or a directory vnode is locked which
prevents vflush() from finishing until the directory vnode is unlocked.

If all this looks like its going in the right direction, my next change
will be to hold am_lock (the lock the protects the autofs node tree),
across autofs_node_vn() - which will be step torwards getting autofs
ready to handle autofs node removals

[kib]

sys/fs/autofs/autofs_vnops.c

693–694

What prevents two threads from observing vp == NULL and then allocate both a new an_vnode?

[rew]

sys/fs/autofs/autofs_vnops.c

693–694

hmm..yea, I see - I was able to trigger this assert by using a pause() to keep the window open

I can think of a few different ways to handle this:

• hold the an_vnode lock across getnewvnode(), vn_lock(vp) and insmntque(vp, mp). It is an LOR to hold an_vnode_lock while calling vn_lock(vp) however, vp isn't accessible by other threads at that point so perhaps that is a safe LOR and pass LK_NOWITNESS to vn_lock() for that instance. Holding the an_node_lock for that long, across routines that *could* block, doesn't seem ideal though.

• different thought...when re-acquiring an_vnode_lock (after insmntque()), if an_vnode != NULL, clean up the vnode that lost the race and return the vnode that won the race

• I looked at tmpfs and saw how it uses tn_vpstate to with TMPFS_VNODE_ALLOCATING to prevent a similar scenario

• I looked at at devfs and it appears to use exclusive dm_lock and devfs_de_interlock to do something similiar

originally, I was planning to follow-up with D57971 which, subsequently, prevents two threads from observing vp == NULL...trying to figure out if this is the correct direction or not

thoughts? curious to hear your feedback

[kib]

sys/fs/autofs/autofs_vnops.c

693–694

Re-checking the av_vnode after re-acquiring the lock is how I would start closing the race. Devfs has special requirements to maintain the dirents so its approach is IMO over-complicated for just this case (it refcounts its internal structures).

[rew]

After re-acquiring an_vnode_lock, check if a vnode is associated with
the autofs node - if one is, clean up the newly created vnode and use
the vnode that is associated with the autofs node

[kib]

sys/fs/autofs/autofs_vnops.c

666–667

you might remove the '{}'