Date: Wed, 19 Oct 2011 14:17:46 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Benjamin Kaduk <kaduk@MIT.EDU> Cc: freebsd-fs@freebsd.org, rmacklem@freebsd.org Subject: Re: lock status of dvp in lookup error return? Message-ID: <20111019111746.GP50300@deviant.kiev.zoral.com.ua> In-Reply-To: <alpine.GSO.1.10.1110190139070.882@multics.mit.edu> References: <alpine.GSO.1.10.1110190139070.882@multics.mit.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
--rQ7Ovc9/RBrrr0/1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Wed, Oct 19, 2011 at 02:07:17AM -0400, Benjamin Kaduk wrote:
> Hi Rick,
>=20
> In tracking down a panic trying to recursively lock a vnode in openafs, I=
=20
> started questioning my behavior in the ISDOTDOT case, in particular=20
> whether to drop the dvp lock before the actual call over the network; thi=
s=20
> naturally led me to look at the NFS code as a reference.
> Unfortunately, this left me more confused than when I began ...
>=20
> sys/fs/nfs_clvnops.c, in nfs_lookup():
> 1211 if (flags & ISDOTDOT) {
> 1212 ltype =3D NFSVOPISLOCKED(dvp);
> 1213 error =3D vfs_busy(mp, MBF_NOWAIT);
> 1214 if (error !=3D 0) {
> 1215 vfs_ref(mp);
> 1216 NFSVOPUNLOCK(dvp, 0);
> 1217 error =3D vfs_busy(mp, 0);
> 1218 NFSVOPLOCK(dvp, ltype | LK_RETRY);
>=20
> If we fail to busy the mountpoint, drop the directory lock and try again,=
=20
> then relock dvp afterward.
>=20
> 1219 vfs_rel(mp);
> 1220 if (error =3D=3D 0 && (dvp->v_iflag & VI_DOOM=
ED)) {
> 1221 vfs_unbusy(mp);
> 1222 error =3D ENOENT;
> 1223 }
> 1224 if (error !=3D 0)
> 1225 return (error);
>=20
> But if the second vfs_busy failed, or dvp is DOOMED, return with dvp=20
> locked.
>=20
> 1226 }
> 1227 NFSVOPUNLOCK(dvp, 0);
>=20
> But now we always unlock dvp.
>=20
> 1228 error =3D nfscl_nget(mp, dvp, nfhp, cnp, td, &np,=
=20
> NULL,
> 1229 cnp->cn_lkflags);
>=20
> The call to the network (?)
>=20
> 1230 if (error =3D=3D 0)
> 1231 newvp =3D NFSTOV(np);
> 1232 vfs_unbusy(mp);
> 1233 if (newvp !=3D dvp)
> 1234 NFSVOPLOCK(dvp, ltype | LK_RETRY);
Did you missed line 1234 ?
The code is the copy of the vn_vget_ino(). The logic in the function
might be slightly easier to follow.
> 1235 if (dvp->v_iflag & VI_DOOMED) {
> 1236 if (error =3D=3D 0) {
> 1237 if (newvp =3D=3D dvp)
> 1238 vrele(newvp);
> 1239 else
> 1240 vput(newvp);
> 1241 }
> 1242 error =3D ENOENT;
> 1243 }
> 1244 if (error !=3D 0)
> 1245 return (error);
>=20
> And here if there was an error hearing from the network, we return with=
=20
> dvp still unlocked.
>=20
> 1246 if (attrflag)
> 1247 (void) nfscl_loadattrcache(&newvp, &nfsva,=20
> NULL, NULL,
> 1248 0, 1);
>=20
>=20
> So, I'm still confused about whether I should be unlocking dvp in the=20
> error case for ISDOTDOT (though presumably looking at other filesystems=
=20
> would help). This inconsistency in the NFS client looks like a bug at my=
=20
> current level of understanding -- what do you think?
>=20
> Thanks,
>=20
> Ben Kaduk
> _______________________________________________
> freebsd-fs@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-fs
> To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org"
--rQ7Ovc9/RBrrr0/1
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (FreeBSD)
iEYEARECAAYFAk6esdoACgkQC3+MBN1Mb4gc7wCg45DKU1WoaUBlwI1FrJl1HnPy
pmsAnRkGuubGe5QT55xcNtTpq69GoSFQ
=5Bav
-----END PGP SIGNATURE-----
--rQ7Ovc9/RBrrr0/1--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111019111746.GP50300>