Re: locking from VOP_INACTIVE to VOP_RECLAIM

["J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>]

> On 1. Apr 2017, at 20:41, Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
> 
> Currently:
> 
> - For VOP_INACTIVE, vnode lock is held on entry and released on exit.
> - For VOP_RECLAIM, vnode lock is not held on entry.
> 
> I would like to change this so that:
> 
> - For VOP_INACTIVE, vnode lock is held on entry and exit.
> - For VOP_RECLAIM, vnode lock is held on entry.

Changing VOP_INACTIVE to keep the vnode locked looks good to me.

> VOP_RECLAIM then *destroys* the lock, so we don't have to say whether
> it is held or released on exit.  This requires a one-line change to
> vfs_vnode.c, and then mechanically moving VOP_UNLOCK from *_inactive
> to *_reclaim throughout the file systems.

Changing VOP_RECLAIM to destroy the lock forces a complete
rewrite of the vnode lock mechanism.

Currently we have genfs_lock() that takes care to not lock
a reclaimed vnode and genfs_deadlock() that locks unconditionally
if called with LK_RETRY.

Before VOP_RECLAIM we use genfs_lock() and after VOP_RECLAIM
we change the operations vector to dead_vnodeop_p and use
genfs_deadlock().

Operation vn_lock() then takes care not to return a dead vnode if
it gets called with LK_RETRY.

If this is to make the locked state during VOP_RECLAIM assertable
it should be changed to either "locked on entry, locked on return"
or "locked on entry, unlocked on return".

> I did a quick survey of every VOP_INACTIVE in src/sys.  With one
> exception, every one either (a) does VOP_UNLOCK just before return, or
> (b) defers to another VOP_INACTIVE method.  The one exception is
> nfs_inactive, which also queues something up on a workqueue, but I
> can't imagine that it matters whether the vnode lock is held.
> 
> Benefit: Functions that VOP_RECLAIM calls would be able to assert,
> rather than vaguely assume, exclusive ownership of the vnode lock.
> Currently various code paths, e.g. ffs_update, require exclusive
> access to the vnode in question, which can't be asserted right now
> because of the VOP_RECLAIM exception.
> 
> Thoughts?  Objections?

--
J. Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig (Germany)