--DocE+STaALJfprDB
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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=
l.=20
> > 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=
's=20
> > a patch for 3.X in that directory.
> >=20
> > I think this fix is the right way to go in the long run. I'm uncomforta=
ble
> > 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=
,=20
> > not pulling this change into 3.X means pulling in vrele2() then retirin=
g=20
> > it in 4.X. :-(
>=20
> 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=
ill
> > 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=
is
> > change, we won't even notice.
>=20
> 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
die.

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 =
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.
>=20
> 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=
=20
besides the one you want. ;-)

> > 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(), tak=
ing
> > recursion into account), then everything is fine.
>=20
> this doesn't seem related to the problem at hand, so I'll leave it for la=
ter.

It isn't. Waiting is fine.

Take care,

Bill

--DocE+STaALJfprDB
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (NetBSD)

iD8DBQFFbb/3Wz+3JHUci9cRAtjeAJwMXxh5pd+U6DZsXR41XFVDm4b3MACghDHh
sSF7gDWI4RbchkuEMYIZ7MA=
=LXgm
-----END PGP SIGNATURE-----

--DocE+STaALJfprDB--