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

On Sat, Mar 20, 2004 at 04:13:00PM +0900, YAMAMOTO Takashi wrote:
> hi,
>=20
> currently, for some vnode operations, lookup(9) disable caching
> for the benefit of some filesystems including ufs.
>=20
> >	if (cnp->cn_nameiop =3D=3D DELETE ||
> >	    (wantparent && cnp->cn_nameiop !=3D CREATE))
> >		docache =3D 0;
>=20
> however, due to this, our nfs client issues LOOKUP rpcs which isn't
> really needed in the most cases.
>=20
> while it could be workarounded in nfs code,
> i think it's better to push this ufs-ism into each filesystems
> which really need it.
>=20
> i'll commit the attached diffs if no one objects.

I object. If I understand your patch right (which I may not), it seems to=
=20
me you are changing how ufs and friends behave in this case.

I also don't understand how this is a ufs-ism. My take on the line above=20
is that it means we won't create a cache entry for something we're about=20
to delete. How is that ufs-specific?

> Index: kern/vfs_lookup.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- kern/vfs_lookup.c	(revision 462)
> +++ kern/vfs_lookup.c	(working copy)
> @@ -336,9 +336,6 @@ lookup(ndp)
>  	 */
>  	wantparent =3D cnp->cn_flags & (LOCKPARENT | WANTPARENT);
>  	docache =3D (cnp->cn_flags & NOCACHE) ^ NOCACHE;
> -	if (cnp->cn_nameiop =3D=3D DELETE ||
> -	    (wantparent && cnp->cn_nameiop !=3D CREATE))
> -		docache =3D 0;
>  	rdonly =3D cnp->cn_flags & RDONLY;
>  	ndp->ni_dvp =3D NULL;
>  	cnp->cn_flags &=3D ~ISSYMLINK;
> Index: ufs/ext2fs/ext2fs_lookup.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- ufs/ext2fs/ext2fs_lookup.c	(revision 266)
> +++ ufs/ext2fs/ext2fs_lookup.c	(working copy)
> @@ -289,8 +289,8 @@ ext2fs_lookup(v)
>  	int flags =3D cnp->cn_flags;
>  	int nameiop =3D cnp->cn_nameiop;
>  	ino_t foundino;
> -
>  	int	dirblksize =3D VTOI(ap->a_dvp)->i_e2fs->e2fs_bsize;
> +	boolean_t toremove;
> =20
>  	bp =3D NULL;
>  	slotoffset =3D -1;
> @@ -305,8 +305,9 @@ ext2fs_lookup(v)
>  	if ((error =3D VOP_ACCESS(vdp, VEXEC, cred, cnp->cn_proc)) !=3D 0)
>  		return (error);
> =20
> -	if ((flags & ISLASTCN) && (vdp->v_mount->mnt_flag & MNT_RDONLY) &&
> -	    (cnp->cn_nameiop =3D=3D DELETE || cnp->cn_nameiop =3D=3D RENAME))
> +	toremove =3D (flags & ISLASTCN) &&
> +	    (cnp->cn_nameiop =3D=3D DELETE || cnp->cn_nameiop =3D=3D RENAME);
> +	if (toremove && (vdp->v_mount->mnt_flag & MNT_RDONLY))
>  		return (EROFS);
> =20
>  	/*
> @@ -316,8 +317,13 @@ ext2fs_lookup(v)
>  	 * check the name cache to see if the directory/name pair
>  	 * we are looking for is known already.
>  	 */
> -	if ((error =3D cache_lookup(vdp, vpp, cnp)) >=3D 0)
> -		return (error);
> +	if (toremove) {
> +		cache_purge1(vdp, cnp, 0);
> +		cnp->cn_flags &=3D ~MAKEENTRY;
> +	} else {
> +		if ((error =3D cache_lookup(vdp, vpp, cnp)) >=3D 0)
> +			return (error);
> +	}

That's different, and that's my objection. Before your change, we will
look in the name cache even if we're about to rename or delete. So if the
node's known, we find it fast. After your change we will always blow the
cache entry away and then do a to-disk lookup.

I don't understand how that is helpful. Why should we ignore the cache in=
=20
this case?

My understanding is you propogate this change to all other file systems.

Note also that I'm a bit confused by your use of "lookup(9) disable=20
caching" in the very begining of the message. lookup(9) (as I understand=20
it) is not currently disabling USING the cache, it is just disabling=20
creating cache entries for something that's about to be deleted.

Take care,

Bill

--xXmbgvnjoT4axfJE
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (NetBSD)

iD8DBQFAXRc1Wz+3JHUci9cRAv2+AJ93PeQmYGHc3GCQarRddZiYdWRgzwCdG3SG
exjVxh1v3iQevgvt1WRdBP8=
=cmkl
-----END PGP SIGNATURE-----

--xXmbgvnjoT4axfJE--