Date: Wed, 19 Oct 2011 02:07:17 -0400 (EDT) From: Benjamin Kaduk <kaduk@MIT.EDU> To: rmacklem@freebsd.org Cc: freebsd-fs@freebsd.org Subject: lock status of dvp in lookup error return? Message-ID: <alpine.GSO.1.10.1110190139070.882@multics.mit.edu>
next in thread | raw e-mail | index | archive | help
Hi Rick, In tracking down a panic trying to recursively lock a vnode in openafs, I started questioning my behavior in the ISDOTDOT case, in particular whether to drop the dvp lock before the actual call over the network; this naturally led me to look at the NFS code as a reference. Unfortunately, this left me more confused than when I began ... sys/fs/nfs_clvnops.c, in nfs_lookup(): 1211 if (flags & ISDOTDOT) { 1212 ltype = NFSVOPISLOCKED(dvp); 1213 error = vfs_busy(mp, MBF_NOWAIT); 1214 if (error != 0) { 1215 vfs_ref(mp); 1216 NFSVOPUNLOCK(dvp, 0); 1217 error = vfs_busy(mp, 0); 1218 NFSVOPLOCK(dvp, ltype | LK_RETRY); If we fail to busy the mountpoint, drop the directory lock and try again, then relock dvp afterward. 1219 vfs_rel(mp); 1220 if (error == 0 && (dvp->v_iflag & VI_DOOMED)) { 1221 vfs_unbusy(mp); 1222 error = ENOENT; 1223 } 1224 if (error != 0) 1225 return (error); But if the second vfs_busy failed, or dvp is DOOMED, return with dvp locked. 1226 } 1227 NFSVOPUNLOCK(dvp, 0); But now we always unlock dvp. 1228 error = nfscl_nget(mp, dvp, nfhp, cnp, td, &np, NULL, 1229 cnp->cn_lkflags); The call to the network (?) 1230 if (error == 0) 1231 newvp = NFSTOV(np); 1232 vfs_unbusy(mp); 1233 if (newvp != dvp) 1234 NFSVOPLOCK(dvp, ltype | LK_RETRY); 1235 if (dvp->v_iflag & VI_DOOMED) { 1236 if (error == 0) { 1237 if (newvp == dvp) 1238 vrele(newvp); 1239 else 1240 vput(newvp); 1241 } 1242 error = ENOENT; 1243 } 1244 if (error != 0) 1245 return (error); And here if there was an error hearing from the network, we return with dvp still unlocked. 1246 if (attrflag) 1247 (void) nfscl_loadattrcache(&newvp, &nfsva, NULL, NULL, 1248 0, 1); So, I'm still confused about whether I should be unlocking dvp in the error case for ISDOTDOT (though presumably looking at other filesystems would help). This inconsistency in the NFS client looks like a bug at my current level of understanding -- what do you think? Thanks, Ben Kaduk
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.GSO.1.10.1110190139070.882>