[PATCH 0/5] [RFC] New path lookup function (V3)

[PATCH 0/5] [RFC] New path lookup function (V3)

[Josef 'Jeff' Sipek]

(For list of changes since V2, see end of this email.)

Stackable file systems, among others, 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 first patch introduces a new lookup function to allow lookup starting
from an arbitrary point in the namespace.  This approach has been suggested
by Christoph Hellwig [2].

The second patch changes sunrpc to use vfs_path_lookup.

The third patch changes nfsctl.c to use vfs_path_lookup.

The fourth patch marks link_path_walk static.

The fifth, and last patch, unexports path_walk because it is no longer
unnecessary to call it directly, and using the new vfs_path_lookup is
cleaner.

For example, the following snippet of code, looks up "some/path/component"
in a directory pointed to by parent_{dentry,vfsmnt}:

err = vfs_path_lookup(parent_dentry, parent_vfsmnt,
		      "some/path/component", 0, &nd);
if (!err) {
	/* exits */

	...

	/* once done, release the references */
	path_release(&nd);
} else if (err == -ENOENT) {
	/* doesn't exist */
} 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://lkml.org/lkml/2007/3/9/95
[2] http://lkml.org/lkml/2007/5/4/51


Changes since V2:

- renamed path_component_lookup to vfs_path_lookup as it allows lookup of
  any path relative to a dentry/vfsmnt (hch)
- kernel doc comment for vfs_path_lookup (hch)
- cleaned up the audit_inode condition (hch)
- rpc_mount should not be dereferenced directly (trond)
- make path_walk static (hch)
- make link_path_walk static (hch)

[PATCH 1/5] fs: Introduce vfs_path_lookup

[Josef 'Jeff' Sipek]

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/namei.c            |   32 ++++++++++++++++++++++++++++++++
 include/linux/namei.h |    2 ++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 3449e0a..090cce4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1175,6 +1175,37 @@ int fastcall path_lookup(const char *name, unsigned int flags,
 	return do_path_lookup(AT_FDCWD, name, flags, nd);
 }
 
+/**
+ * vfs_path_lookup - lookup a file path relative to a dentry-vfsmount pair
+ * @dentry:  pointer to dentry of the base directory
+ * @mnt: pointer to vfs mount of the base directory
+ * @name: pointer to file name
+ * @flags: lookup flags
+ * @nd: pointer to nameidata
+ */
+int vfs_path_lookup(struct dentry *dentry, struct vfsmount *mnt,
+		    const char *name, unsigned int flags, 
+		    struct nameidata *nd)
+{
+	int retval;
+
+	/* same as do_path_lookup */
+	nd->last_type = LAST_ROOT;
+	nd->flags = flags;
+	nd->depth = 0;
+
+	nd->mnt = mntget(mnt);
+	nd->dentry = dget(dentry);
+
+	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;
+
+}
+
 static int __path_lookup_intent_open(int dfd, const char *name,
 		unsigned int lookup_flags, struct nameidata *nd,
 		int open_flags, int create_mode)
@@ -2799,6 +2830,7 @@ EXPORT_SYMBOL(__page_symlink);
 EXPORT_SYMBOL(page_symlink);
 EXPORT_SYMBOL(page_symlink_inode_operations);
 EXPORT_SYMBOL(path_lookup);
+EXPORT_SYMBOL(vfs_path_lookup);
 EXPORT_SYMBOL(path_release);
 EXPORT_SYMBOL(path_walk);
 EXPORT_SYMBOL(permission);
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 0ab27ba..4718788 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -70,6 +70,8 @@ extern int FASTCALL(__user_walk_fd(int dfd, const char __user *, unsigned, struc
 #define user_path_walk_link(name,nd) \
 	__user_walk_fd(AT_FDCWD, name, 0, nd)
 extern int FASTCALL(path_lookup(const char *, unsigned, struct nameidata *));
+extern int vfs_path_lookup(struct dentry *, struct vfsmount *,
+			   const char *, unsigned int, struct nameidata *);
 extern int FASTCALL(path_walk(const char *, struct nameidata *));
 extern int FASTCALL(link_path_walk(const char *, struct nameidata *));
 extern void path_release(struct nameidata *);
-- 
1.5.2.rc1.20.g86b9

[PATCH 2/5] sunrpc: Use vfs_path_lookup

[Josef 'Jeff' Sipek]

use vfs_path_lookup instead of open-coding the necessary functionality.

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
Acked-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 net/sunrpc/rpc_pipe.c |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 9b9ea50..2170329 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -451,21 +451,19 @@ void rpc_put_mount(void)
 static int
 rpc_lookup_parent(char *path, struct nameidata *nd)
 {
+	struct vfsmount *mnt;
+
 	if (path[0] == '\0')
 		return -ENOENT;
-	nd->mnt = rpc_get_mount();
-	if (IS_ERR(nd->mnt)) {
+
+	mnt = rpc_get_mount();
+	if (IS_ERR(mnt)) {
 		printk(KERN_WARNING "%s: %s failed to mount "
 			       "pseudofilesystem \n", __FILE__, __FUNCTION__);
-		return PTR_ERR(nd->mnt);
+		return PTR_ERR(mnt);
 	}
-	mntget(nd->mnt);
-	nd->dentry = dget(rpc_mount->mnt_root);
-	nd->last_type = LAST_ROOT;
-	nd->flags = LOOKUP_PARENT;
-	nd->depth = 0;
 
-	if (path_walk(path, nd)) {
+	if (vfs_path_lookup(mnt->mnt_root, mnt, path, LOOKUP_PARENT, nd)) {
 		printk(KERN_WARNING "%s: %s failed to find path %s\n",
 				__FILE__, __FUNCTION__, path);
 		rpc_put_mount();
-- 
1.5.2.rc1.20.g86b9

[PATCH 3/5] nfsctl: Use vfs_path_lookup

[Josef 'Jeff' Sipek]

use vfs_path_lookup instead of open-coding the necessary functionality.

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/nfsctl.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/fs/nfsctl.c b/fs/nfsctl.c
index c043136..c97df14 100644
--- a/fs/nfsctl.c
+++ b/fs/nfsctl.c
@@ -23,19 +23,14 @@
 static struct file *do_open(char *name, int flags)
 {
 	struct nameidata nd;
+	struct vfsmount *mnt;
 	int error;
 
-	nd.mnt = do_kern_mount("nfsd", 0, "nfsd", NULL);
+	mnt = do_kern_mount("nfsd", 0, "nfsd", NULL);
+	if (IS_ERR(mnt))
+		return (struct file *)mnt;
 
-	if (IS_ERR(nd.mnt))
-		return (struct file *)nd.mnt;
-
-	nd.dentry = dget(nd.mnt->mnt_root);
-	nd.last_type = LAST_ROOT;
-	nd.flags = 0;
-	nd.depth = 0;
-
-	error = path_walk(name, &nd);
+	error = vfs_path_lookup(mnt->mnt_root, mnt, name, 0, &nd);
 	if (error)
 		return ERR_PTR(error);
 
-- 
1.5.2.rc1.20.g86b9

[PATCH 4/5] fs: Mark link_path_walk static

[Josef 'Jeff' Sipek]

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/namei.c            |    5 ++++-
 include/linux/namei.h |    1 -
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 090cce4..d9eb621 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -108,6 +108,9 @@
  * any extra contention...
  */
 
+/* forward declaration */
+static int fastcall link_path_walk(const char *name, struct nameidata *nd);
+
 /* In order to reduce some races, while at the same time doing additional
  * checking and hopefully speeding things up, we copy filenames to the
  * kernel data space before using them..
@@ -999,7 +1002,7 @@ return_err:
  * Retry the whole path once, forcing real lookup requests
  * instead of relying on the dcache.
  */
-int fastcall link_path_walk(const char *name, struct nameidata *nd)
+static int fastcall link_path_walk(const char *name, struct nameidata *nd)
 {
 	struct nameidata save = *nd;
 	int result;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 4718788..d60a5eb 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -73,7 +73,6 @@ extern int FASTCALL(path_lookup(const char *, unsigned, struct nameidata *));
 extern int vfs_path_lookup(struct dentry *, struct vfsmount *,
 			   const char *, unsigned int, struct nameidata *);
 extern int FASTCALL(path_walk(const char *, struct nameidata *));
-extern int FASTCALL(link_path_walk(const char *, struct nameidata *));
 extern void path_release(struct nameidata *);
 extern void path_release_on_umount(struct nameidata *);
 
-- 
1.5.2.rc1.20.g86b9

[PATCH 5/5] fs: Remove path_walk export

[Josef 'Jeff' Sipek]

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/namei.c            |    3 +--
 include/linux/namei.h |    1 -
 2 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index d9eb621..7a98676 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1026,7 +1026,7 @@ static int fastcall link_path_walk(const char *name, struct nameidata *nd)
 	return result;
 }
 
-int fastcall path_walk(const char * name, struct nameidata *nd)
+static int fastcall path_walk(const char * name, struct nameidata *nd)
 {
 	current->total_link_count = 0;
 	return link_path_walk(name, nd);
@@ -2835,7 +2835,6 @@ EXPORT_SYMBOL(page_symlink_inode_operations);
 EXPORT_SYMBOL(path_lookup);
 EXPORT_SYMBOL(vfs_path_lookup);
 EXPORT_SYMBOL(path_release);
-EXPORT_SYMBOL(path_walk);
 EXPORT_SYMBOL(permission);
 EXPORT_SYMBOL(vfs_permission);
 EXPORT_SYMBOL(file_permission);
diff --git a/include/linux/namei.h b/include/linux/namei.h
index d60a5eb..e378e1f 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -72,7 +72,6 @@ extern int FASTCALL(__user_walk_fd(int dfd, const char __user *, unsigned, struc
 extern int FASTCALL(path_lookup(const char *, unsigned, struct nameidata *));
 extern int vfs_path_lookup(struct dentry *, struct vfsmount *,
 			   const char *, unsigned int, struct nameidata *);
-extern int FASTCALL(path_walk(const char *, struct nameidata *));
 extern void path_release(struct nameidata *);
 extern void path_release_on_umount(struct nameidata *);
 
-- 
1.5.2.rc1.20.g86b9

Re: [PATCH 1/5] fs: Introduce vfs_path_lookup

[Christoph Hellwig]

On Sun, May 06, 2007 at 01:41:25PM -0400, Josef 'Jeff' Sipek wrote:
> Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
> ---
>  fs/namei.c            |   32 ++++++++++++++++++++++++++++++++
>  include/linux/namei.h |    2 ++
>  2 files changed, 34 insertions(+), 0 deletions(-)

Looks good to me.

Re: [PATCH 2/5] sunrpc: Use vfs_path_lookup

[Christoph Hellwig]

On Sun, May 06, 2007 at 01:41:26PM -0400, Josef 'Jeff' Sipek wrote:
> use vfs_path_lookup instead of open-coding the necessary functionality.
> 
> Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
> Acked-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>  net/sunrpc/rpc_pipe.c |   16 +++++++---------
>  1 files changed, 7 insertions(+), 9 deletions(-)

Looks good to me.

Re: [PATCH 3/5] nfsctl: Use vfs_path_lookup

[Christoph Hellwig]

On Sun, May 06, 2007 at 01:41:27PM -0400, Josef 'Jeff' Sipek wrote:
> use vfs_path_lookup instead of open-coding the necessary functionality.
> 
> Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
> ---
>  fs/nfsctl.c |   15 +++++----------
>  1 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/nfsctl.c b/fs/nfsctl.c
> index c043136..c97df14 100644
> --- a/fs/nfsctl.c
> +++ b/fs/nfsctl.c
> @@ -23,19 +23,14 @@
>  static struct file *do_open(char *name, int flags)
>  {
>  	struct nameidata nd;
> +	struct vfsmount *mnt;
>  	int error;
>  
> -	nd.mnt = do_kern_mount("nfsd", 0, "nfsd", NULL);
> +	mnt = do_kern_mount("nfsd", 0, "nfsd", NULL);
> +	if (IS_ERR(mnt))
> +		return (struct file *)mnt;
>  
> -	if (IS_ERR(nd.mnt))
> -		return (struct file *)nd.mnt;
> -
> -	nd.dentry = dget(nd.mnt->mnt_root);
> -	nd.last_type = LAST_ROOT;
> -	nd.flags = 0;
> -	nd.depth = 0;
> -
> -	error = path_walk(name, &nd);
> +	error = vfs_path_lookup(mnt->mnt_root, mnt, name, 0, &nd);
>  	if (error)
>  		return ERR_PTR(error);

I think you're leaking a reference on the mount here as we didn't
do a mntget before.  We need to do an additional mntput just after
the lookup to drop the reference from do_kern_mount.

Re: [PATCH 4/5] fs: Mark link_path_walk static

[Christoph Hellwig]

On Sun, May 06, 2007 at 01:41:28PM -0400, Josef 'Jeff' Sipek wrote:
> +/* forward declaration */
> +static int fastcall link_path_walk(const char *name, struct nameidata *nd);

I don't think we need that comment, otherwise this looks good.

Re: [PATCH 5/5] fs: Remove path_walk export

[Christoph Hellwig]

On Sun, May 06, 2007 at 01:41:29PM -0400, Josef 'Jeff' Sipek wrote:
> Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
> ---
>  fs/namei.c            |    3 +--
>  include/linux/namei.h |    1 -
>  2 files changed, 1 insertions(+), 3 deletions(-)

Ok.

[PATCH 4/5] fs: Mark link_path_walk static

[Josef 'Jeff' Sipek]

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/namei.c            |    4 +++-
 include/linux/namei.h |    1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 090cce4..925c62e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -108,6 +108,8 @@
  * any extra contention...
  */
 
+static int fastcall link_path_walk(const char *name, struct nameidata *nd);
+
 /* In order to reduce some races, while at the same time doing additional
  * checking and hopefully speeding things up, we copy filenames to the
  * kernel data space before using them..
@@ -999,7 +1001,7 @@ return_err:
  * Retry the whole path once, forcing real lookup requests
  * instead of relying on the dcache.
  */
-int fastcall link_path_walk(const char *name, struct nameidata *nd)
+static int fastcall link_path_walk(const char *name, struct nameidata *nd)
 {
 	struct nameidata save = *nd;
 	int result;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 4718788..d60a5eb 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -73,7 +73,6 @@ extern int FASTCALL(path_lookup(const char *, unsigned, struct nameidata *));
 extern int vfs_path_lookup(struct dentry *, struct vfsmount *,
 			   const char *, unsigned int, struct nameidata *);
 extern int FASTCALL(path_walk(const char *, struct nameidata *));
-extern int FASTCALL(link_path_walk(const char *, struct nameidata *));
 extern void path_release(struct nameidata *);
 extern void path_release_on_umount(struct nameidata *);
 
-- 
1.5.2.rc1.20.g86b9

[PATCH 1/1] nfsctl: Use vfs_path_lookup

[Josef 'Jeff' Sipek]

use vfs_path_lookup instead of open-coding the necessary functionality.

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/nfsctl.c |   16 ++++++----------
 1 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/fs/nfsctl.c b/fs/nfsctl.c
index c043136..51f1b31 100644
--- a/fs/nfsctl.c
+++ b/fs/nfsctl.c
@@ -23,19 +23,15 @@
 static struct file *do_open(char *name, int flags)
 {
 	struct nameidata nd;
+	struct vfsmount *mnt;
 	int error;
 
-	nd.mnt = do_kern_mount("nfsd", 0, "nfsd", NULL);
+	mnt = do_kern_mount("nfsd", 0, "nfsd", NULL);
+	if (IS_ERR(mnt))
+		return (struct file *)mnt;
 
-	if (IS_ERR(nd.mnt))
-		return (struct file *)nd.mnt;
-
-	nd.dentry = dget(nd.mnt->mnt_root);
-	nd.last_type = LAST_ROOT;
-	nd.flags = 0;
-	nd.depth = 0;
-
-	error = path_walk(name, &nd);
+	error = vfs_path_lookup(mnt->mnt_root, mnt, name, 0, &nd);
+	mntput(mnt);	/* drop do_kern_mount reference */
 	if (error)
 		return ERR_PTR(error);
 
-- 
1.5.2.rc1.20.g86b9

Re: [PATCH 1/1] nfsctl: Use vfs_path_lookup

[Neil Brown]

On Sunday May 6, jsipek@cs.sunysb.edu wrote:
> use vfs_path_lookup instead of open-coding the necessary functionality.
> 
> Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>

Acked-by: NeilBrown <neilb@suse.de>

Thanks,
NeilBrown

> ---
>  fs/nfsctl.c |   16 ++++++----------
>  1 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/nfsctl.c b/fs/nfsctl.c
> index c043136..51f1b31 100644
> --- a/fs/nfsctl.c
> +++ b/fs/nfsctl.c
> @@ -23,19 +23,15 @@
>  static struct file *do_open(char *name, int flags)
>  {
>  	struct nameidata nd;
> +	struct vfsmount *mnt;
>  	int error;
>  
> -	nd.mnt = do_kern_mount("nfsd", 0, "nfsd", NULL);
> +	mnt = do_kern_mount("nfsd", 0, "nfsd", NULL);
> +	if (IS_ERR(mnt))
> +		return (struct file *)mnt;
>  
> -	if (IS_ERR(nd.mnt))
> -		return (struct file *)nd.mnt;
> -
> -	nd.dentry = dget(nd.mnt->mnt_root);
> -	nd.last_type = LAST_ROOT;
> -	nd.flags = 0;
> -	nd.depth = 0;
> -
> -	error = path_walk(name, &nd);
> +	error = vfs_path_lookup(mnt->mnt_root, mnt, name, 0, &nd);
> +	mntput(mnt);	/* drop do_kern_mount reference */
>  	if (error)
>  		return ERR_PTR(error);
>  
> -- 
> 1.5.2.rc1.20.g86b9

[PATCH 4/5] fs: Mark link_path_walk static

[Josef 'Jeff' Sipek]

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/namei.c            |    4 +++-
 include/linux/namei.h |    1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index a30efbc..50285a1 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -107,6 +107,8 @@
  * any extra contention...
  */
 
+static int fastcall link_path_walk(const char *name, struct nameidata *nd);
+
 /* In order to reduce some races, while at the same time doing additional
  * checking and hopefully speeding things up, we copy filenames to the
  * kernel data space before using them..
@@ -998,7 +1000,7 @@ return_err:
  * Retry the whole path once, forcing real lookup requests
  * instead of relying on the dcache.
  */
-int fastcall link_path_walk(const char *name, struct nameidata *nd)
+static int fastcall link_path_walk(const char *name, struct nameidata *nd)
 {
 	struct nameidata save = *nd;
 	int result;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 4718788..d60a5eb 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -73,7 +73,6 @@ extern int FASTCALL(path_lookup(const char *, unsigned, struct nameidata *));
 extern int vfs_path_lookup(struct dentry *, struct vfsmount *,
 			   const char *, unsigned int, struct nameidata *);
 extern int FASTCALL(path_walk(const char *, struct nameidata *));
-extern int FASTCALL(link_path_walk(const char *, struct nameidata *));
 extern void path_release(struct nameidata *);
 extern void path_release_on_umount(struct nameidata *);
 
-- 
1.5.2.rc1.165.gaf9b