Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 18 Mar 2009 23:01:46 +0200
From:      Kostik Belousov <kostikbel@gmail.com>
To:        John Baldwin <jhb@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r186276 - head/sys/kern
Message-ID:  <20090318210146.GG7716@deviant.kiev.zoral.com.ua>
In-Reply-To: <200903181215.23213.jhb@freebsd.org>
References:  <200812181158.mBIBwC50039690@svn.freebsd.org> <49BAA2C6.2000807@FreeBSD.org> <20090313212229.GW41617@deviant.kiev.zoral.com.ua> <200903181215.23213.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--QWpDgw58+k1mSFBj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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 all=
ows
> > > >  the vnode to be reclaimed.
> > > > =20
> > > >  Tested by:	pho
> > > >  MFC after:	1 month
> > >=20
> > > Would EBADF be more appropriate?  That is what other places that chec=
k=20
> > > VI_DOOMED return for this type of check (e.g. in cache_lookup()).
> >=20
> > 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.
>=20
> Hmm, I think a few places need to be fixed to ENOENT instead of EBADF the=
n:
>=20
> --- //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 =3D 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 =3D EBADF;
> +				error =3D ENOENT;
>  				break;
>  			}
>  			vp =3D 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) =3D=3D 0)
>  				break;
>  			if (dp->v_iflag & VI_DOOMED) {	/* forced unmount */
> -				error =3D EBADF;
> +				error =3D ENOENT;
>  				goto bad;
>  			}
>  			tdp =3D dp;
> @@ -764,9 +764,11 @@
>  	     *ndp->ni_next =3D=3D '/')) {
>  		cnp->cn_flags |=3D 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 =3D EBADF;
> +			/*
> +			 * We can't know whether the directory was mounted with
> +			 * NOSYMFOLLOW, so we can't follow safely.
> +			 */
> +			error =3D ENOENT;
>  			goto bad2;
>  		}
>  		if (dp->v_mount->mnt_flag & MNT_NOSYMFOLLOW) {
>=20

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.

--QWpDgw58+k1mSFBj
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (FreeBSD)

iEYEARECAAYFAknBYTkACgkQC3+MBN1Mb4hHpwCfetOfFkK8nJPRtZnYc1rYsWTZ
TRMAni18qQGC4JYrrn35L/5mjERHFbma
=Dm5Q
-----END PGP SIGNATURE-----

--QWpDgw58+k1mSFBj--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090318210146.GG7716>