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 path_component_lookup.
The third patch changes nfsctl.c to use path_component_lookup.
The fourth, and last patch, unexports path_walk because it is no longer
unnecessary to call it directly, and using the new path_component_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 = path_component_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
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
fs/namei.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index b547af0..0262594 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2826,7 +2826,6 @@ EXPORT_SYMBOL(page_symlink_inode_operations);
EXPORT_SYMBOL(path_lookup);
EXPORT_SYMBOL(path_component_lookup);
EXPORT_SYMBOL(path_release);
-EXPORT_SYMBOL(path_walk);
EXPORT_SYMBOL(permission);
EXPORT_SYMBOL(vfs_permission);
EXPORT_SYMBOL(file_permission);
--
1.5.0.3.1043.g4342
use path_component_lookup instead of open-coding the necessary
functionality.
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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..b67cb54 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 (path_component_lookup(rpc_mount->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.0.3.1043.g4342
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
fs/namei.c | 26 ++++++++++++++++++++++++++
include/linux/namei.h | 2 ++
2 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 3449e0a..b547af0 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1175,6 +1175,31 @@ int fastcall path_lookup(const char *name, unsigned int flags,
return do_path_lookup(AT_FDCWD, name, flags, nd);
}
+int path_component_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 (likely(retval == 0)) {
+ if (unlikely(!audit_dummy_context() && nd && 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 +2824,7 @@ EXPORT_SYMBOL(__page_symlink);
EXPORT_SYMBOL(page_symlink);
EXPORT_SYMBOL(page_symlink_inode_operations);
EXPORT_SYMBOL(path_lookup);
+EXPORT_SYMBOL(path_component_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..2247397 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 path_component_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.0.3.1043.g4342
use path_component_lookup instead of open-coding the necessary
functionality.
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
fs/nfsctl.c | 15 +++++----------
1 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/fs/nfsctl.c b/fs/nfsctl.c
index c043136..2035dc7 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 = path_component_lookup(mnt->mnt_root, mnt, name, 0, &nd);
if (error)
return ERR_PTR(error);
--
1.5.0.3.1043.g4342
On Sat, 2007-05-05 at 19:09 -0400, Josef 'Jeff' Sipek wrote:
> use path_component_lookup instead of open-coding the necessary
> functionality.
>
> Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
> ---
> 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..b67cb54 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 (path_component_lookup(rpc_mount->mnt_root, mnt, path, LOOKUP_PARENT, nd)) {
> printk(KERN_WARNING "%s: %s failed to find path %s\n",
> __FILE__, __FUNCTION__, path);
> rpc_put_mount();
Just one minor nit: could you make that
if (path_component_lookup(mnt->mnt_root, mnt, path, LOOKUP_PARENT, nd)) {
instead. It really is a bug even for the existing code to be referencing
rpc_mount directly.
Cheers
Trond
use path_component_lookup instead of open-coding the necessary
functionality.
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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..c93a454 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 (path_component_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
I wrote up the suggestion before my first morning tea yesterday
and must admit that the name path_component_lookup is pretty stupid.
We don't just look up a component but any relative path starting
from the vfsmount/dentry pair. How about vfs_path_lookup instead
because it mirrors various other vfs_ function that are dentry based?
Also as a new exported symbol it should get a kerneldoc comment describing
it.
> + if (likely(retval == 0)) {
> + if (unlikely(!audit_dummy_context() && nd && nd->dentry &&
> + nd->dentry->d_inode))
> + audit_inode(name, nd->dentry->d_inode);
> + }
This should get the same simplification I suggested for do_path_lookup.
On Sat, May 05, 2007 at 07:09:34PM -0400, Josef 'Jeff' Sipek wrote:
> Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
> ---
> fs/namei.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
it should be possible to make it static now aswell. (the uml code
using it is #if0'ed and will never be resurrected because it's quite bogus).
Also link_path_walk can become static aswell. It will need a forward
declaration though as it's used recursively.
On Sat, 2007-05-05 at 20:52 -0400, Josef 'Jeff' Sipek wrote:
> use path_component_lookup instead of open-coding the necessary
> functionality.
>
> Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
> ---
> 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..c93a454 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 (path_component_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();
Thanks! That looks good.
Acked-by: Trond Myklebust <[email protected]>
Cheers
Trond