svn commit: r186276 - head/sys/kern
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
Modified:
head/sys/kern/vfs_lookup.c
Modified: head/sys/kern/vfs_lookup.c
==
--- head/sys/kern/vfs_lookup.c Thu Dec 18 09:59:24 2008(r186275)
+++ head/sys/kern/vfs_lookup.c Thu Dec 18 11:58:12 2008(r186276)
@@ -814,6 +814,10 @@ success:
if ((cnp->cn_flags & (ISLASTCN | LOCKSHARED | LOCKLEAF)) ==
(ISLASTCN | LOCKLEAF) && VOP_ISLOCKED(dp) != LK_EXCLUSIVE) {
vn_lock(dp, LK_UPGRADE | LK_RETRY);
+ if (dp->v_iflag & VI_DOOMED) {
+ error = ENOENT;
+ goto bad2;
+ }
}
if (vfslocked && dvfslocked)
VFS_UNLOCK_GIANT(dvfslocked); /* Only need one */
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r186276 - head/sys/kern
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()). -- John Baldwin ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r186276 - head/sys/kern
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. pgpOry125033E.pgp Description: PGP signature
Re: svn commit: r186276 - head/sys/kern
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
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r186276 - head/sys/kern
On Wed, Mar 18, 2009 at 12:15:22PM -0400, John Baldwin wrote:
> 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) {
>
There is one place in lookup() that does MNT_UNION handling, it switches
to the covered directory when VOP_LOOKUP returns ENOENT. In current code,
EBADF from VOP_LOOKUP would cause namei to return, but with the change,
it will continue. I doubt that this can cause a problem.
Overall, it looks good to me.
pgpbXh1RobgSO.pgp
Description: PGP signature
