On Tue, Nov 28, 2006 at 10:48:18AM -0800, Bill Studenmund wrote:
> Chuck Silvers has also proposed a fix which changes the locking protocol. 
> Now, VOP_LOOKUP() won't unlock the parent. The patch is at 
> ftp://ftp.netbsd.org/pub/NetBSD/misc/chs/lookup/diff.20061119 and there's 
> a patch for 3.X in that directory.
> 
> I think this fix is the right way to go in the long run. I'm uncomfortable
> though pulling this into 3.x as it's a big change.  Actually, it's a big
> change with a number of other, little changes. On the flip side, though, 
> not pulling this change into 3.X means pulling in vrele2() then retiring 
> it in 4.X. :-(

I'm ok with either way for 3.x.


> I don't think we can get rid of PDIRUNLOCK, as the reason we have it still
> exists (unless I missed something). When we look up "..", we have to
> unlock the parent directory, lock ".." (which is really its grandparent,
> the parent of the "parent" directory), then re-lock the parent. That
> re-locking can fail, yet we have no real way to communicate that back up
> the call graph. Think of it as an extra return value. Actually, with this
> change, we won't even notice.

I changed all the ISDOTDOT cases to re-lock the "parent" with
"vn_lock(vp, LK_EXCLUSIVE | LK_RETRY)", which always succeeds.
actually, many of these paths already did that.
the parent vnode might be dead at that point, but that's ok.
but I think we need to change more (if not all) of the dead vnodeops
to be genfs_ebadf instead of genfs_badop.


> The fix for layer_node_find() touches on another PR hitting layered file
> systems. dbj@ and ad@ looked into it, and reported details which I filed
> as PR 35143. I completely missed the fact that if LK_TYPE_MASK & flags is
> zero, then vget() will skip lockmgr(). That's exactly what this code
> needed. The problem with the change, though, is that it still has the
> deadlock issue.

how can a deadlock happen after this change to layer_node_find()?


> I think we should rip out all of the LAYERFS_UPPERLOCK() stuff. All of our
> file systems now support real locking. I think all we need to do is call
> VOP_LOCK() on the lower layer (after turning LK_DRAIN into LK_EXCLUSIVE
> and fixing up the interlock). As long as the lower layer locking routine
> behaves as if it is a lockmgr() call (if you pass in LK_EXCLUSIVE, when
> you return, no one else gets to return until you call VOP_UNLOCK(), taking
> recursion into account), then everything is fine.

this doesn't seem related to the problem at hand, so I'll leave it for later.

-Chuck