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>