> On 2. Apr 2017, at 18:19, Taylor R Campbell > <campbell+netbsd-tech-k...@mumble.net> wrote: > >> Date: Sun, 2 Apr 2017 16:34:20 +0200 >> From: "J. Hannken-Illjes" <hann...@eis.cs.tu-bs.de> >> >> 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 - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)