lock status of dvp in lookup error return?

[Benjamin Kaduk]

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