Re: locking from VOP_INACTIVE to VOP_RECLAIM

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

> On 2. Apr 2017, at 18:19, Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
> 
>> Date: Sun, 2 Apr 2017 16:34:20 +0200
>> From: "J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>
>> 
>> Looks like your proposal needs some clarification regarding "vnode lock"
>> and "the lock".
>> 
>> We have two vnode related locks:
>> 
>> - the "vnode lock", located as field "vi_lock" in "struct vnode_impl"
>>  used by genfs_*lock() aka VOP_*LOCK.
>> 
>> - the "genfs lock" located as field "g_lock" in "struct genfs_node"
>>  used by genfs_node_*lock().
>> 
>> Which lock do you want to be held across VOP_INACTIVE and destroyed
>> from VOP_RECLAIM?
> 
> Aha!  That would explain why I had trouble tracking down why an assert
> I was planning to add kept firing.  I was thinking that the genfs node
> lock and the vnode lock were one and the same.
> 
> My proposal was about the vnode lock, not the genfs lock.  Since
> VOP_RECLAIM does *not* destroy the vnode lock, and must leave it
> either locked or unlocked, I suggest leaving it locked -- it's always
> easier to think about subroutines that preserve the state of a lock
> (locked to locked, or unlocked to unlocked) rather than changing it
> (locked to unlocked or unlocked to locked).

Looks good to me so far and please keep in mind that VOP_RECLAIM
must not drop/aquire the lock as it will never get it back for a
vnode in state RECLAIMING.

> (As an aside, we may have some bugs in ffs concerning use of the
> struct inode::i_size field -- some callers seem to think access is
> serialized by the genfs lock, while others, e.g. ufs_readwrite.c, seem
> to use the vnode lock.  This needs to be documented more clearly...)

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