svn commit: r186276 - head/sys/kern

[John Baldwin]

On Friday 13 March 2009 5:22:29 pm Kostik Belousov wrote:
> On Fri, Mar 13, 2009 at 02:15:34PM -0400, John Baldwin wrote:
> > Konstantin Belousov wrote:
> > >Author: kib
> > >Date: Thu Dec 18 11:58:12 2008
> > >New Revision: 186276
> > >URL: http://svn.freebsd.org/changeset/base/186276
> > >
> > >Log:
> > >  Do not return success and doomed vnode from lookup. LK_UPGRADE allows
> > >  the vnode to be reclaimed.
> > >  
> > >  Tested by:	pho
> > >  MFC after:	1 month
> > 
> > Would EBADF be more appropriate?  That is what other places that check 
> > VI_DOOMED return for this type of check (e.g. in cache_lookup()).
> 
> I do not think so. When we do namei lookup, there is actually no
> file descriptor to be invalid. The fact the we lost the race with
> forced unmount actually means that there is no more node with
> supplied name.

Hmm, I think a few places need to be fixed to ENOENT instead of EBADF then:

--- //depot/user/jhb/lock/kern/vfs_cache.c
+++ /home/jhb/work/p4/lock/kern/vfs_cache.c
@@ -315,7 +315,7 @@
  * (negative cacheing), a status of ENOENT is returned. If the lookup
  * fails, a status of zero is returned.  If the directory vnode is
  * recycled out from under us due to a forced unmount, a status of
- * EBADF is returned.
+ * ENOENT is returned.
  *
  * vpp is locked and ref'd on return.  If we're looking up DOTDOT, dvp is
  * unlocked.  If we're looking up . an extra ref is taken, but the lock is
@@ -467,7 +467,7 @@
 					/* forced unmount */
 					vrele(*vpp);
 					*vpp = NULL;
-					return (EBADF);
+					return (ENOENT);
 				}
 			} else
 				vn_lock(*vpp, LK_DOWNGRADE | LK_RETRY);
@@ -974,7 +974,7 @@
 		if (vp->v_vflag & VV_ROOT) {
 			if (vp->v_iflag & VI_DOOMED) {	/* forced unmount */
 				CACHE_RUNLOCK();
-				error = EBADF;
+				error = ENOENT;
 				break;
 			}
 			vp = vp->v_mount->mnt_vnodecovered;
--- //depot/user/jhb/lock/kern/vfs_lookup.c
+++ /home/jhb/work/p4/lock/kern/vfs_lookup.c
@@ -602,7 +602,7 @@
 			if ((dp->v_vflag & VV_ROOT) == 0)
 				break;
 			if (dp->v_iflag & VI_DOOMED) {	/* forced unmount */
-				error = EBADF;
+				error = ENOENT;
 				goto bad;
 			}
 			tdp = dp;
@@ -764,9 +764,11 @@
 	     *ndp->ni_next == '/')) {
 		cnp->cn_flags |= ISSYMLINK;
 		if (dp->v_iflag & VI_DOOMED) {
-			/* We can't know whether the directory was mounted with
-			 * NOSYMFOLLOW, so we can't follow safely. */
-			error = EBADF;
+			/*
+			 * We can't know whether the directory was mounted with
+			 * NOSYMFOLLOW, so we can't follow safely.
+			 */
+			error = ENOENT;
 			goto bad2;
 		}
 		if (dp->v_mount->mnt_flag & MNT_NOSYMFOLLOW) {


-- 
John Baldwin