[PATCH 0/1] [RFC] New mode for path_lookup (V1)

[PATCH 0/1] [RFC] New mode for path_lookup (V1)

[Josef Sipek]

Stackable file systems frequently need to lookup paths or path components
starting from an arbitrary point in the namespace (identified by a dentry
and a vfsmount).  Currently, such file systems use lookup_one_len, which is
frowned upon [1] as it does not pass the lookup intent along; not passing a
lookup intent, for example, can trigger BUG_ON's when stacking on top of
NFSv4.

The following patch introduces a new mode to path_lookup to allow lookup to
start from an arbitrary point in the namespace.  This approach has been
suggested by Christoph Hellwig at the Linux Storage & Filesystem workshop in
February of this year.

One indicates that the lookup should be relative to a dentry-vfsmnt pair by
using the LOOKUP_ONE flag.  For example, the following snippet of code,
looks up "pathcomponent" in a directory pointed to by
parent_{dentry,vfsmnt}:

nd.dentry = parent_dentry;
nd.mnt = parent_vfsmnt;

err = path_lookup("pathcomponent", LOOKUP_ONE, &nd);
if (!err) {
	/* exits */

	...

	/* once done, release the references */
	path_release(&nd);
} else if (err == -ENOENT) {
	/* doesn't exits */
} else {
	/* other error */
}

VFS functions such as lookup_create can be used on the nameidata structure
to pass the create intent to the file system.

Currently, there is no easy way to pass the LOOKUP_OPEN intent.  The proper
way would be to call open_namei.

We'd like to get comments about what's necessary to make stackable file
systems do lookups right - this includes potential changes to open_namei. 

Josef 'Jeff' Sipek.

[1] http://marc.info/?l=linux-kernel&m=117343337823760&w=2

[PATCH 1/1] fs: add 4th case to do_path_lookup

[Josef Sipek]

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>

diff --git a/fs/namei.c b/fs/namei.c
index 2995fba..1516a9b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1125,6 +1125,10 @@ static int fastcall do_path_lookup(int dfd, const char *name,
 		nd->mnt = mntget(fs->rootmnt);
 		nd->dentry = dget(fs->root);
 		read_unlock(&fs->lock);
+	} else if (flags & LOOKUP_ONE) {
+		/* nd->mnt and nd->dentry already set, just grab references */
+		mntget(nd->mnt);
+		dget(nd->dentry);
 	} else if (dfd == AT_FDCWD) {
 		read_lock(&fs->lock);
 		nd->mnt = mntget(fs->pwdmnt);
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 92b422b..aa89d97 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -48,6 +48,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
  *  - internal "there are more path compnents" flag
  *  - locked when lookup done with dcache_lock held
  *  - dentry cache is untrusted; force a real lookup
+ *  - lookup path from given dentry/vfsmount pair
  */
 #define LOOKUP_FOLLOW		 1
 #define LOOKUP_DIRECTORY	 2
@@ -55,6 +56,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
 #define LOOKUP_PARENT		16
 #define LOOKUP_NOALT		32
 #define LOOKUP_REVAL		64
+#define LOOKUP_ONE	       128
 /*
  * Intent data
  */

Re: [PATCH 1/1] fs: add 4th case to do_path_lookup

[Andrew Morton]

On Sun, 29 Apr 2007 23:30:12 -0400 Josef Sipek <jsipek@cs.sunysb.edu> wrote:

> Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 2995fba..1516a9b 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1125,6 +1125,10 @@ static int fastcall do_path_lookup(int dfd, const char *name,
>  		nd->mnt = mntget(fs->rootmnt);
>  		nd->dentry = dget(fs->root);
>  		read_unlock(&fs->lock);
> +	} else if (flags & LOOKUP_ONE) {
> +		/* nd->mnt and nd->dentry already set, just grab references */
> +		mntget(nd->mnt);
> +		dget(nd->dentry);
>  	} else if (dfd == AT_FDCWD) {
>  		read_lock(&fs->lock);
>  		nd->mnt = mntget(fs->pwdmnt);
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 92b422b..aa89d97 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -48,6 +48,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
>   *  - internal "there are more path compnents" flag
>   *  - locked when lookup done with dcache_lock held
>   *  - dentry cache is untrusted; force a real lookup
> + *  - lookup path from given dentry/vfsmount pair
>   */
>  #define LOOKUP_FOLLOW		 1
>  #define LOOKUP_DIRECTORY	 2
> @@ -55,6 +56,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
>  #define LOOKUP_PARENT		16
>  #define LOOKUP_NOALT		32
>  #define LOOKUP_REVAL		64
> +#define LOOKUP_ONE	       128

Well the patch passes my too-small-to-care-about test ;)

Unless someone objects I'd suggest that you add it to the unionfs tree.

Re: [PATCH 1/1] fs: add 4th case to do_path_lookup

[Christoph Hellwig]

sorry, I proposed Jeff a reply long ago but haven't done yet.

On Fri, May 04, 2007 at 12:02:00AM -0700, Andrew Morton wrote:
> > @@ -1125,6 +1125,10 @@ static int fastcall do_path_lookup(int dfd, const char *name,
> >  		nd->mnt = mntget(fs->rootmnt);
> >  		nd->dentry = dget(fs->root);
> >  		read_unlock(&fs->lock);
> > +	} else if (flags & LOOKUP_ONE) {
> > +		/* nd->mnt and nd->dentry already set, just grab references */
> > +		mntget(nd->mnt);
> > +		dget(nd->dentry);
> >  	} else if (dfd == AT_FDCWD) {
> >  		read_lock(&fs->lock);
> >  		nd->mnt = mntget(fs->pwdmnt);
> 
> Well the patch passes my too-small-to-care-about test ;)
> 
> Unless someone objects I'd suggest that you add it to the unionfs tree.

The code is obviously correct.  There is one little thing that bothers
me, and that's that nd was purely an output paramter to path_lookup and
do_path_lookup, and no it's an input paramter for the least used case.
It might make sense to just a simple helper ala:



static int path_component_lookup(struct dentry *dentry, struct vfsmount *mnt,
		const char *name, unsigned int flags, struct nameidata *nd)
{
	int retval;

	nd->last_type = LAST_ROOT;
	nd->flags = flags;
	nd->mnt = mntget(mnt);
	nd->dentry = dget(dentry);
	nd->depth = 0;

	retval = path_walk(name, nd);
	if (unlikely(!retval && !audit_dummy_context() &&
	             nd->dentry && nd->dentry->d_inode))
		audit_inode(name, nd->dentry->d_inode);

	return retval;

}

instead.

Re: [PATCH 1/1] fs: add 4th case to do_path_lookup

[Christoph Hellwig]

Oh and btw, net/sunrpc/rpc_pipe.c:rpc_lookup_parent() and
fs/nfsctl.c:do_open() should  be switched to the new code, at which
point the path_walk() export can go.