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

On Tue, Feb 28, 2006 at 12:38:30PM -0500, Greg Troxel wrote:
> In src/sys/coda/coda_vnops.c 1.46, I added LK_RETRY to vn_lock calls.
> This seems correct because vn_lock will (absent LK_RETRY) fail if the
> vnode is already locked (presumably by another thread), but it will
> first sleep until a wakeup happens.

Sorry for the delay, but I think tis is wrong.

> 1) Am I correct that some other process could have VXLOCK on the
>    vnode, that this is really a normal condition, and thus that
>    LK_RETRY is really in order?  (I would think LK_RETRY should only
>    be omitted as part of some deadlock avoidance procedure where the
>    calling code is doing retries, or something like that.)

VXLOCK should only be used when we are recycling the vnode - when we are=20
blowing it away. It is set when we are changing the type of the vnode, in=
=20
part as all of the locking originally was done by the fs.

> 2) Why does vn_lock sleep if it's going to fail?  Is this for
>    caller-managed retry loops?

To wait for the cleanup to finish, I expect.

> 3) For clarity, it seems that LK_INTERLOCK should be cleared after
>    ltsleep, and separately after VOP_LOCK.  I realize this is a case
>    of CSE but it makes it harder to realize that the rules are being
>    followed.

If I understand the code right, I think it's a style thing. We go through=
=20
one or the other side of an if statement, and we clear it after the if.

> Any clues appreciated.
>=20
>=20
> vn_lock body from src/sys/kern/vfs_vnops.c:
>=20
> 	do {
> 		if ((flags & LK_INTERLOCK) =3D=3D 0)
> 			simple_lock(&vp->v_interlock);
> 		if (vp->v_flag & VXLOCK) {
> 			if (flags & LK_NOWAIT) {
> 				simple_unlock(&vp->v_interlock);
> 				return EBUSY;
> 			}
> 			vp->v_flag |=3D VXWANT;
> 			ltsleep(vp, PINOD | PNORELOCK,
> 			    "vn_lock", 0, &vp->v_interlock);
> 			error =3D ENOENT;
> 		} else {
> 			error =3D VOP_LOCK(vp,
> 			    (flags & ~LK_RETRY) | LK_INTERLOCK);
> 			if (error =3D=3D 0 || error =3D=3D EDEADLK || error =3D=3D EBUSY)
> 				return (error);
> 		}
> 		flags &=3D ~LK_INTERLOCK;
> 	} while (flags & LK_RETRY);

Or did I miss something? :-)

The only case I can see where it isn't cleared is if we trigger the return=
=20
after VOP_LOCK(). But in that case, since flags is local, it no longer=20
matters.

Take care,

Bill

--JgQwtEuHJzHdouWu
Content-Type: application/pgp-signature
Content-Disposition: inline

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

iD8DBQFEFJ1JWz+3JHUci9cRAimbAJ9ILH4RI8DSbByOWlkuZQVYtK+z5gCfdW+I
xda34qhxFI1guc5wD7mHQYw=
=9cgt
-----END PGP SIGNATURE-----

--JgQwtEuHJzHdouWu--