* [PATCH 1/4] fs: remove the unused fs_lookup_param function
2020-09-26 9:20 small pathname lookup cleanups Christoph Hellwig
@ 2020-09-26 9:20 ` Christoph Hellwig
2020-09-26 14:57 ` Al Viro
2020-09-26 9:20 ` [PATCH 2/4] fs: mark filename_lookup static Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2020-09-26 9:20 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
Documentation/filesystems/mount_api.rst | 18 +-------
fs/fs_parser.c | 56 -------------------------
include/linux/fs_parser.h | 5 ---
3 files changed, 2 insertions(+), 77 deletions(-)
diff --git a/Documentation/filesystems/mount_api.rst b/Documentation/filesystems/mount_api.rst
index 29c169c68961f3..dbff847986da47 100644
--- a/Documentation/filesystems/mount_api.rst
+++ b/Documentation/filesystems/mount_api.rst
@@ -254,8 +254,8 @@ manage the filesystem context. They are as follows:
will have been weeded out and fc->sb_flags updated in the context.
Security options will also have been weeded out and fc->security updated.
- The parameter can be parsed with fs_parse() and fs_lookup_param(). Note
- that the source(s) are presented as parameters named "source".
+ The parameter can be parsed with fs_parse(). Note that the source(s) are
+ presented as parameters named "source".
If successful, 0 should be returned or a negative error code otherwise.
@@ -809,17 +809,3 @@ process the parameters it is given.
If the parameter isn't matched, -ENOPARAM will be returned; if the
parameter is matched, but the value is erroneous, -EINVAL will be
returned; otherwise the parameter's option number will be returned.
-
- * ::
-
- int fs_lookup_param(struct fs_context *fc,
- struct fs_parameter *value,
- bool want_bdev,
- struct path *_path);
-
- This takes a parameter that carries a string or filename type and attempts
- to do a path lookup on it. If the parameter expects a blockdev, a check
- is made that the inode actually represents one.
-
- Returns 0 if successful and ``*_path`` will be set; returns a negative
- error code if not.
diff --git a/fs/fs_parser.c b/fs/fs_parser.c
index ab53e42a874aaa..ee40f838b2be91 100644
--- a/fs/fs_parser.c
+++ b/fs/fs_parser.c
@@ -133,62 +133,6 @@ int __fs_parse(struct p_log *log,
}
EXPORT_SYMBOL(__fs_parse);
-/**
- * fs_lookup_param - Look up a path referred to by a parameter
- * @fc: The filesystem context to log errors through.
- * @param: The parameter.
- * @want_bdev: T if want a blockdev
- * @_path: The result of the lookup
- */
-int fs_lookup_param(struct fs_context *fc,
- struct fs_parameter *param,
- bool want_bdev,
- struct path *_path)
-{
- struct filename *f;
- unsigned int flags = 0;
- bool put_f;
- int ret;
-
- switch (param->type) {
- case fs_value_is_string:
- f = getname_kernel(param->string);
- if (IS_ERR(f))
- return PTR_ERR(f);
- put_f = true;
- break;
- case fs_value_is_filename:
- f = param->name;
- put_f = false;
- break;
- default:
- return invalf(fc, "%s: not usable as path", param->key);
- }
-
- f->refcnt++; /* filename_lookup() drops our ref. */
- ret = filename_lookup(param->dirfd, f, flags, _path, NULL);
- if (ret < 0) {
- errorf(fc, "%s: Lookup failure for '%s'", param->key, f->name);
- goto out;
- }
-
- if (want_bdev &&
- !S_ISBLK(d_backing_inode(_path->dentry)->i_mode)) {
- path_put(_path);
- _path->dentry = NULL;
- _path->mnt = NULL;
- errorf(fc, "%s: Non-blockdev passed as '%s'",
- param->key, f->name);
- ret = -ENOTBLK;
- }
-
-out:
- if (put_f)
- putname(f);
- return ret;
-}
-EXPORT_SYMBOL(fs_lookup_param);
-
int fs_param_bad_value(struct p_log *log, struct fs_parameter *param)
{
return inval_plog(log, "Bad value for '%s'", param->key);
diff --git a/include/linux/fs_parser.h b/include/linux/fs_parser.h
index aab0ffc6bac67a..a62ed20fda6d98 100644
--- a/include/linux/fs_parser.h
+++ b/include/linux/fs_parser.h
@@ -73,11 +73,6 @@ static inline int fs_parse(struct fs_context *fc,
return __fs_parse(&fc->log, desc, param, result);
}
-extern int fs_lookup_param(struct fs_context *fc,
- struct fs_parameter *param,
- bool want_bdev,
- struct path *_path);
-
extern int lookup_constant(const struct constant_table tbl[], const char *name, int not_found);
#ifdef CONFIG_VALIDATE_FS_PARSER
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/4] fs: remove the unused fs_lookup_param function
2020-09-26 9:20 ` [PATCH 1/4] fs: remove the unused fs_lookup_param function Christoph Hellwig
@ 2020-09-26 14:57 ` Al Viro
0 siblings, 0 replies; 8+ messages in thread
From: Al Viro @ 2020-09-26 14:57 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-fsdevel
On Sat, Sep 26, 2020 at 11:20:48AM +0200, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> Documentation/filesystems/mount_api.rst | 18 +-------
> fs/fs_parser.c | 56 -------------------------
> include/linux/fs_parser.h | 5 ---
> 3 files changed, 2 insertions(+), 77 deletions(-)
>
> diff --git a/Documentation/filesystems/mount_api.rst b/Documentation/filesystems/mount_api.rst
> index 29c169c68961f3..dbff847986da47 100644
> --- a/Documentation/filesystems/mount_api.rst
> +++ b/Documentation/filesystems/mount_api.rst
> @@ -254,8 +254,8 @@ manage the filesystem context. They are as follows:
> will have been weeded out and fc->sb_flags updated in the context.
> Security options will also have been weeded out and fc->security updated.
>
> - The parameter can be parsed with fs_parse() and fs_lookup_param(). Note
> - that the source(s) are presented as parameters named "source".
> + The parameter can be parsed with fs_parse(). Note that the source(s) are
> + presented as parameters named "source".
Umm... Not sure - I'm not too fond of fs_lookup_param(), but AFAIK there are
efforts to convert more filesystems to new mount API and I don't know if any
of those are currently using it - this work has moved to individual fs trees
now, so it's hard to keep track of.
Generally I would say "if it's out of tree, it's not our problem", but the new
API is fairly recent and conversions of in-tree filesystems aree still in
progress. And they are likely to stay in topical branches in the regular
git trees of those in-tree filesystems while they are being developed, so
right now I would rather be careful with removals in that area, short of
serious problem with the primitive itself. Being able to make lookup_filename()
static is obviously nice, but considering the changes done later in the series...
Let's hold it back for now.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/4] fs: mark filename_lookup static
2020-09-26 9:20 small pathname lookup cleanups Christoph Hellwig
2020-09-26 9:20 ` [PATCH 1/4] fs: remove the unused fs_lookup_param function Christoph Hellwig
@ 2020-09-26 9:20 ` Christoph Hellwig
2020-09-26 9:20 ` [PATCH 3/4] fs: pass a nameidata into filename_lookup Christoph Hellwig
2020-09-26 9:20 ` [PATCH 4/4] fs: simplify vfs_path_lookup Christoph Hellwig
3 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2020-09-26 9:20 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/internal.h | 2 --
fs/namei.c | 2 +-
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/internal.h b/fs/internal.h
index 10517ece45167f..695e12bc285061 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -71,8 +71,6 @@ extern int finish_clean_context(struct fs_context *fc);
/*
* namei.c
*/
-extern int filename_lookup(int dfd, struct filename *name, unsigned flags,
- struct path *path, struct path *root);
extern int vfs_path_lookup(struct dentry *, struct vfsmount *,
const char *, unsigned int, struct path *);
long do_rmdir(int dfd, struct filename *name);
diff --git a/fs/namei.c b/fs/namei.c
index e99e2a9da0f7de..7963f97a130442 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2351,7 +2351,7 @@ static int path_lookupat(struct nameidata *nd, unsigned flags, struct path *path
return err;
}
-int filename_lookup(int dfd, struct filename *name, unsigned flags,
+static int filename_lookup(int dfd, struct filename *name, unsigned flags,
struct path *path, struct path *root)
{
int retval;
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/4] fs: pass a nameidata into filename_lookup
2020-09-26 9:20 small pathname lookup cleanups Christoph Hellwig
2020-09-26 9:20 ` [PATCH 1/4] fs: remove the unused fs_lookup_param function Christoph Hellwig
2020-09-26 9:20 ` [PATCH 2/4] fs: mark filename_lookup static Christoph Hellwig
@ 2020-09-26 9:20 ` Christoph Hellwig
2020-09-26 14:49 ` Al Viro
2020-09-26 9:20 ` [PATCH 4/4] fs: simplify vfs_path_lookup Christoph Hellwig
3 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2020-09-26 9:20 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel
This allows keeping the LOOKUP_ROOT case for vfs_path_lookup entirely
out of the normal fast path.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/namei.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 7963f97a130442..90e1cb008ae449 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2352,22 +2352,18 @@ static int path_lookupat(struct nameidata *nd, unsigned flags, struct path *path
}
static int filename_lookup(int dfd, struct filename *name, unsigned flags,
- struct path *path, struct path *root)
+ struct path *path, struct nameidata *nd)
{
int retval;
- struct nameidata nd;
+
if (IS_ERR(name))
return PTR_ERR(name);
- if (unlikely(root)) {
- nd.root = *root;
- flags |= LOOKUP_ROOT;
- }
- set_nameidata(&nd, dfd, name);
- retval = path_lookupat(&nd, flags | LOOKUP_RCU, path);
+ set_nameidata(nd, dfd, name);
+ retval = path_lookupat(nd, flags | LOOKUP_RCU, path);
if (unlikely(retval == -ECHILD))
- retval = path_lookupat(&nd, flags, path);
+ retval = path_lookupat(nd, flags, path);
if (unlikely(retval == -ESTALE))
- retval = path_lookupat(&nd, flags | LOOKUP_REVAL, path);
+ retval = path_lookupat(nd, flags | LOOKUP_REVAL, path);
if (likely(!retval))
audit_inode(name, path->dentry,
@@ -2450,8 +2446,10 @@ struct dentry *kern_path_locked(const char *name, struct path *path)
int kern_path(const char *name, unsigned int flags, struct path *path)
{
- return filename_lookup(AT_FDCWD, getname_kernel(name),
- flags, path, NULL);
+ struct nameidata nd;
+
+ return filename_lookup(AT_FDCWD, getname_kernel(name), flags, path,
+ &nd);
}
EXPORT_SYMBOL(kern_path);
@@ -2467,10 +2465,12 @@ int vfs_path_lookup(struct dentry *dentry, struct vfsmount *mnt,
const char *name, unsigned int flags,
struct path *path)
{
- struct path root = {.mnt = mnt, .dentry = dentry};
- /* the first argument of filename_lookup() is ignored with root */
+ struct nameidata nd;
+
+ nd.root.mnt = mnt;
+ nd.root.dentry = dentry;
return filename_lookup(AT_FDCWD, getname_kernel(name),
- flags , path, &root);
+ flags | LOOKUP_ROOT, path, &nd);
}
EXPORT_SYMBOL(vfs_path_lookup);
@@ -2643,8 +2643,10 @@ int path_pts(struct path *path)
int user_path_at_empty(int dfd, const char __user *name, unsigned flags,
struct path *path, int *empty)
{
+ struct nameidata nd;
+
return filename_lookup(dfd, getname_flags(name, flags, empty),
- flags, path, NULL);
+ flags, path, &nd);
}
EXPORT_SYMBOL(user_path_at_empty);
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 3/4] fs: pass a nameidata into filename_lookup
2020-09-26 9:20 ` [PATCH 3/4] fs: pass a nameidata into filename_lookup Christoph Hellwig
@ 2020-09-26 14:49 ` Al Viro
0 siblings, 0 replies; 8+ messages in thread
From: Al Viro @ 2020-09-26 14:49 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-fsdevel
On Sat, Sep 26, 2020 at 11:20:50AM +0200, Christoph Hellwig wrote:
> This allows keeping the LOOKUP_ROOT case for vfs_path_lookup entirely
> out of the normal fast path.
... saving you all of a if (unlikely(root)) {....} on it. Not worth
it, and
> + struct nameidata nd;
> +
> + return filename_lookup(AT_FDCWD, getname_kernel(name), flags, path,
> + &nd);
is something I would rather avoid on the general principles - better have
set_nameidata()/restore_nameidata() done always to a local variable. Makes
for somewhat easier analysis.
IOW, NAK
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 4/4] fs: simplify vfs_path_lookup
2020-09-26 9:20 small pathname lookup cleanups Christoph Hellwig
` (2 preceding siblings ...)
2020-09-26 9:20 ` [PATCH 3/4] fs: pass a nameidata into filename_lookup Christoph Hellwig
@ 2020-09-26 9:20 ` Christoph Hellwig
2020-09-26 14:46 ` Al Viro
3 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2020-09-26 9:20 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel
vfs_path_lookup is only used to lookup mount points. So drop the dentry
parameter that is always set to mnt->mnt_root, remove the unused export
and rename the function to mount_path_lookup.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/internal.h | 4 ++--
fs/namei.c | 16 +++-------------
fs/namespace.c | 8 +++-----
3 files changed, 8 insertions(+), 20 deletions(-)
diff --git a/fs/internal.h b/fs/internal.h
index 695e12bc285061..bbdae2648f6b7d 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -71,8 +71,8 @@ extern int finish_clean_context(struct fs_context *fc);
/*
* namei.c
*/
-extern int vfs_path_lookup(struct dentry *, struct vfsmount *,
- const char *, unsigned int, struct path *);
+int mount_path_lookup(struct vfsmount *mnt, const char *name,
+ unsigned int flags, struct path *path);
long do_rmdir(int dfd, struct filename *name);
long do_unlinkat(int dfd, struct filename *name);
int may_linkat(struct path *link);
diff --git a/fs/namei.c b/fs/namei.c
index 90e1cb008ae449..30f7caf5eda79b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2453,26 +2453,16 @@ int kern_path(const char *name, unsigned int flags, struct path *path)
}
EXPORT_SYMBOL(kern_path);
-/**
- * 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
- * @path: pointer to struct path to fill
- */
-int vfs_path_lookup(struct dentry *dentry, struct vfsmount *mnt,
- const char *name, unsigned int flags,
- struct path *path)
+int mount_path_lookup(struct vfsmount *mnt, const char *name,
+ unsigned int flags, struct path *path)
{
struct nameidata nd;
nd.root.mnt = mnt;
- nd.root.dentry = dentry;
+ nd.root.dentry = mnt->mnt_root;
return filename_lookup(AT_FDCWD, getname_kernel(name),
flags | LOOKUP_ROOT, path, &nd);
}
-EXPORT_SYMBOL(vfs_path_lookup);
static int lookup_one_len_common(const char *name, struct dentry *base,
int len, struct qstr *this)
diff --git a/fs/namespace.c b/fs/namespace.c
index bae0e95b3713a3..0e904b27f7baeb 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3368,9 +3368,8 @@ struct dentry *mount_subtree(struct vfsmount *m, const char *name)
ns->mounts++;
list_add(&mnt->mnt_list, &ns->list);
- err = vfs_path_lookup(m->mnt_root, m,
- name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &path);
-
+ err = mount_path_lookup(m, name, LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT,
+ &path);
put_mnt_ns(ns);
if (err)
@@ -4060,8 +4059,7 @@ static int mntns_install(struct nsset *nsset, struct ns_common *ns)
nsproxy->mnt_ns = mnt_ns;
/* Find the root */
- err = vfs_path_lookup(mnt_ns->root->mnt.mnt_root, &mnt_ns->root->mnt,
- "/", LOOKUP_DOWN, &root);
+ err = mount_path_lookup(&mnt_ns->root->mnt, "/", LOOKUP_DOWN, &root);
if (err) {
/* revert to old namespace */
nsproxy->mnt_ns = old_mnt_ns;
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread