Subject: Re: pr 32535 followup
To: Chuck Silvers <firstname.lastname@example.org>
From: Bill Studenmund <email@example.com>
Date: 11/29/2006 09:14:31
Content-Type: text/plain; charset=us-ascii
On Wed, Nov 29, 2006 at 09:06:29AM -0800, Chuck Silvers wrote:
> On Tue, Nov 28, 2006 at 10:48:18AM -0800, Bill Studenmund wrote:
> > Chuck Silvers has also proposed a fix which changes the locking protoco=
> > Now, VOP_LOOKUP() won't unlock the parent. The patch is at=20
> > ftp://ftp.netbsd.org/pub/NetBSD/misc/chs/lookup/diff.20061119 and there=
> > a patch for 3.X in that directory.
> > I think this fix is the right way to go in the long run. I'm uncomforta=
> > 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 retirin=
> > it in 4.X. :-(
> I'm ok with either way for 3.x.
Actually, with 4.x being rumored to branch this week, do we want do to=20
your change for 4.x?
> > I don't think we can get rid of PDIRUNLOCK, as the reason we have it st=
> > 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 th=
> > 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.
Ok. I understand what you're suggesting now. So yes, PDIRUNLOCKED will=20
Sounds good about genfs_ebadf().
> > 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 =
> > 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()?
It can't. I was mainly saying I want to make this change for other reasons=
besides the one you want. ;-)
> > I think we should rip out all of the LAYERFS_UPPERLOCK() stuff. All of =
> > 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(), tak=
> > recursion into account), then everything is fine.
> this doesn't seem related to the problem at hand, so I'll leave it for la=
It isn't. Waiting is fine.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (NetBSD)
-----END PGP SIGNATURE-----