[PATCHES] fs_context stuff

[PATCHES] fs_context stuff

[Al Viro]

	That stuff had been discussed last cycle, didn't make it into
-next back then.  If nobody objects, I'm throwing that branch into
#for-next.

	vfs_parse_fs_string() calling conventions are inconvenient;
it takes string and length, and just about every caller (with only two
exceptions) passes string, strlen(string).

	Proposal: introduce vfs_parse_fs_qstr() for those that want
the generic form (and pass &QSTR_LEN(string, length) as its argument)
and lose the length argument of vfs_parse_fs_string().

	Callers are happier that way.  The first commit in the series does
the calling convention change and converts existing users; the second one
converts a couple of open-coded vfs_parse_fs_string() in do_nfs4_mount()
to that primitive.  A lot more readable that way, IMO...

	Branch (-rc1-based) lives in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.fs_context
Individual patches in followups.

	Please, review.  If there's no objections, into -next it goes...

Shortlog:
Al Viro (2):
      change the calling conventions for vfs_parse_fs_string()
      do_nfs4_mount(): switch to vfs_parse_fs_string()

Diffstat:
 Documentation/filesystems/mount_api.rst | 10 +++++++-
 Documentation/filesystems/porting.rst   | 10 ++++++++
 drivers/gpu/drm/i915/gem/i915_gemfs.c   |  9 ++-----
 drivers/gpu/drm/v3d/v3d_gemfs.c         |  9 ++-----
 fs/afs/mntpt.c                          |  3 ++-
 fs/fs_context.c                         | 17 ++++++-------
 fs/namespace.c                          |  8 +++---
 fs/nfs/fs_context.c                     |  3 +--
 fs/nfs/namespace.c                      |  3 ++-
 fs/nfs/nfs4super.c                      | 44 +++++++++------------------------
 fs/smb/client/fs_context.c              |  4 +--
 include/linux/fs_context.h              |  9 +++++--
 kernel/trace/trace.c                    |  3 +--
 13 files changed, 58 insertions(+), 74 deletions(-)

[PATCH 1/2] change the calling conventions for vfs_parse_fs_string()

[Al Viro]

Absolute majority of callers are passing the 4th argument equal to
strlen() of the 3rd one.

Drop the v_size argument, add vfs_parse_fs_qstr() for the cases that
want independent length.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 Documentation/filesystems/mount_api.rst | 10 +++++++++-
 Documentation/filesystems/porting.rst   | 10 ++++++++++
 drivers/gpu/drm/i915/gem/i915_gemfs.c   |  9 ++-------
 drivers/gpu/drm/v3d/v3d_gemfs.c         |  9 ++-------
 fs/afs/mntpt.c                          |  3 ++-
 fs/fs_context.c                         | 17 +++++++----------
 fs/namespace.c                          |  8 +++-----
 fs/nfs/fs_context.c                     |  3 +--
 fs/nfs/namespace.c                      |  3 ++-
 fs/smb/client/fs_context.c              |  4 +---
 include/linux/fs_context.h              |  9 +++++++--
 kernel/trace/trace.c                    |  3 +--
 12 files changed, 47 insertions(+), 41 deletions(-)

diff --git a/Documentation/filesystems/mount_api.rst b/Documentation/filesystems/mount_api.rst
index e149b89118c8..c99ab1f7fea4 100644
--- a/Documentation/filesystems/mount_api.rst
+++ b/Documentation/filesystems/mount_api.rst
@@ -504,10 +504,18 @@ returned.
      clear the pointer, but then becomes responsible for disposing of the
      object.
 
+   * ::
+
+       int vfs_parse_fs_qstr(struct fs_context *fc, const char *key,
+			       const struct qstr *value);
+
+     A wrapper around vfs_parse_fs_param() that copies the value string it is
+     passed.
+
    * ::
 
        int vfs_parse_fs_string(struct fs_context *fc, const char *key,
-			       const char *value, size_t v_size);
+			       const char *value);
 
      A wrapper around vfs_parse_fs_param() that copies the value string it is
      passed.
diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
index 85f590254f07..a9ef6851b8c0 100644
--- a/Documentation/filesystems/porting.rst
+++ b/Documentation/filesystems/porting.rst
@@ -1285,3 +1285,13 @@ rather than a VMA, as the VMA at this stage is not yet valid.
 The vm_area_desc provides the minimum required information for a filesystem
 to initialise state upon memory mapping of a file-backed region, and output
 parameters for the file system to set this state.
+
+---
+
+**mandatory**
+
+Calling conventions for vfs_parse_fs_string() have changed; it does *not*
+take length anymore (value ? strlen(value) : 0 is used).  If you want
+a different length, use
+	vfs_parse_fs_qstr(fc, key, &QSTR_LEN(value, len))
+instead.
diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
index a09e2eb47175..8f13ec4ff0d0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
+++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
@@ -11,11 +11,6 @@
 #include "i915_gemfs.h"
 #include "i915_utils.h"
 
-static int add_param(struct fs_context *fc, const char *key, const char *val)
-{
-	return vfs_parse_fs_string(fc, key, val, strlen(val));
-}
-
 void i915_gemfs_init(struct drm_i915_private *i915)
 {
 	struct file_system_type *type;
@@ -48,9 +43,9 @@ void i915_gemfs_init(struct drm_i915_private *i915)
 	fc = fs_context_for_mount(type, SB_KERNMOUNT);
 	if (IS_ERR(fc))
 		goto err;
-	ret = add_param(fc, "source", "tmpfs");
+	ret = vfs_parse_fs_string(fc, "source", "tmpfs");
 	if (!ret)
-		ret = add_param(fc, "huge", "within_size");
+		ret = vfs_parse_fs_string(fc, "huge", "within_size");
 	if (!ret)
 		gemfs = fc_mount_longterm(fc);
 	put_fs_context(fc);
diff --git a/drivers/gpu/drm/v3d/v3d_gemfs.c b/drivers/gpu/drm/v3d/v3d_gemfs.c
index 8ec6ed82b3d9..c1a30166c099 100644
--- a/drivers/gpu/drm/v3d/v3d_gemfs.c
+++ b/drivers/gpu/drm/v3d/v3d_gemfs.c
@@ -7,11 +7,6 @@
 
 #include "v3d_drv.h"
 
-static int add_param(struct fs_context *fc, const char *key, const char *val)
-{
-	return vfs_parse_fs_string(fc, key, val, strlen(val));
-}
-
 void v3d_gemfs_init(struct v3d_dev *v3d)
 {
 	struct file_system_type *type;
@@ -38,9 +33,9 @@ void v3d_gemfs_init(struct v3d_dev *v3d)
 	fc = fs_context_for_mount(type, SB_KERNMOUNT);
 	if (IS_ERR(fc))
 		goto err;
-	ret = add_param(fc, "source", "tmpfs");
+	ret = vfs_parse_fs_string(fc, "source", "tmpfs");
 	if (!ret)
-		ret = add_param(fc, "huge", "within_size");
+		ret = vfs_parse_fs_string(fc, "huge", "within_size");
 	if (!ret)
 		gemfs = fc_mount_longterm(fc);
 	put_fs_context(fc);
diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c
index 9434a5399f2b..1ad048e6e164 100644
--- a/fs/afs/mntpt.c
+++ b/fs/afs/mntpt.c
@@ -137,7 +137,8 @@ static int afs_mntpt_set_params(struct fs_context *fc, struct dentry *mntpt)
 
 		ret = -EINVAL;
 		if (content[size - 1] == '.')
-			ret = vfs_parse_fs_string(fc, "source", content, size - 1);
+			ret = vfs_parse_fs_qstr(fc, "source",
+						&QSTR_LEN(content, size - 1));
 		do_delayed_call(&cleanup);
 		if (ret < 0)
 			return ret;
diff --git a/fs/fs_context.c b/fs/fs_context.c
index 666e61753aed..93b7ebf8d927 100644
--- a/fs/fs_context.c
+++ b/fs/fs_context.c
@@ -161,25 +161,24 @@ int vfs_parse_fs_param(struct fs_context *fc, struct fs_parameter *param)
 EXPORT_SYMBOL(vfs_parse_fs_param);
 
 /**
- * vfs_parse_fs_string - Convenience function to just parse a string.
+ * vfs_parse_fs_qstr - Convenience function to just parse a string.
  * @fc: Filesystem context.
  * @key: Parameter name.
  * @value: Default value.
- * @v_size: Maximum number of bytes in the value.
  */
-int vfs_parse_fs_string(struct fs_context *fc, const char *key,
-			const char *value, size_t v_size)
+int vfs_parse_fs_qstr(struct fs_context *fc, const char *key,
+			const struct qstr *value)
 {
 	int ret;
 
 	struct fs_parameter param = {
 		.key	= key,
 		.type	= fs_value_is_flag,
-		.size	= v_size,
+		.size	= value ? value->len : 0,
 	};
 
 	if (value) {
-		param.string = kmemdup_nul(value, v_size, GFP_KERNEL);
+		param.string = kmemdup_nul(value->name, value->len, GFP_KERNEL);
 		if (!param.string)
 			return -ENOMEM;
 		param.type = fs_value_is_string;
@@ -189,7 +188,7 @@ int vfs_parse_fs_string(struct fs_context *fc, const char *key,
 	kfree(param.string);
 	return ret;
 }
-EXPORT_SYMBOL(vfs_parse_fs_string);
+EXPORT_SYMBOL(vfs_parse_fs_qstr);
 
 /**
  * vfs_parse_monolithic_sep - Parse key[=val][,key[=val]]* mount data
@@ -218,16 +217,14 @@ int vfs_parse_monolithic_sep(struct fs_context *fc, void *data,
 
 	while ((key = sep(&options)) != NULL) {
 		if (*key) {
-			size_t v_len = 0;
 			char *value = strchr(key, '=');
 
 			if (value) {
 				if (unlikely(value == key))
 					continue;
 				*value++ = 0;
-				v_len = strlen(value);
 			}
-			ret = vfs_parse_fs_string(fc, key, value, v_len);
+			ret = vfs_parse_fs_string(fc, key, value);
 			if (ret < 0)
 				break;
 		}
diff --git a/fs/namespace.c b/fs/namespace.c
index ddfd4457d338..88636124c8fe 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1281,8 +1281,7 @@ struct vfsmount *vfs_kern_mount(struct file_system_type *type,
 		return ERR_CAST(fc);
 
 	if (name)
-		ret = vfs_parse_fs_string(fc, "source",
-					  name, strlen(name));
+		ret = vfs_parse_fs_string(fc, "source", name);
 	if (!ret)
 		ret = parse_monolithic_mount_data(fc, data);
 	if (!ret)
@@ -3793,10 +3792,9 @@ static int do_new_mount(struct path *path, const char *fstype, int sb_flags,
 	fc->oldapi = true;
 
 	if (subtype)
-		err = vfs_parse_fs_string(fc, "subtype",
-					  subtype, strlen(subtype));
+		err = vfs_parse_fs_string(fc, "subtype", subtype);
 	if (!err && name)
-		err = vfs_parse_fs_string(fc, "source", name, strlen(name));
+		err = vfs_parse_fs_string(fc, "source", name);
 	if (!err)
 		err = parse_monolithic_mount_data(fc, data);
 	if (!err && !mount_capable(fc))
diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 9e94d18448ff..b4679b7161b0 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -1269,8 +1269,7 @@ static int nfs23_parse_monolithic(struct fs_context *fc,
 			int ret;
 
 			data->context[NFS_MAX_CONTEXT_LEN] = '\0';
-			ret = vfs_parse_fs_string(fc, "context",
-						  data->context, strlen(data->context));
+			ret = vfs_parse_fs_string(fc, "context", data->context);
 			if (ret < 0)
 				return ret;
 #else
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 7f1ec9c67ff2..5735c0448b4c 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -290,7 +290,8 @@ int nfs_do_submount(struct fs_context *fc)
 		nfs_errorf(fc, "NFS: Couldn't determine submount pathname");
 		ret = PTR_ERR(p);
 	} else {
-		ret = vfs_parse_fs_string(fc, "source", p, buffer + 4096 - p);
+		ret = vfs_parse_fs_qstr(fc, "source",
+					&QSTR_LEN(p, buffer + 4096 - p));
 		if (!ret)
 			ret = vfs_get_tree(fc);
 	}
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index 072383899e81..306b76e97783 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -773,16 +773,14 @@ static int smb3_fs_context_parse_monolithic(struct fs_context *fc,
 		}
 
 
-		len = 0;
 		value = strchr(key, '=');
 		if (value) {
 			if (value == key)
 				continue;
 			*value++ = 0;
-			len = strlen(value);
 		}
 
-		ret = vfs_parse_fs_string(fc, key, value, len);
+		ret = vfs_parse_fs_string(fc, key, value);
 		if (ret < 0)
 			break;
 	}
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index 7773eb870039..97b514a79a49 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -134,8 +134,13 @@ extern struct fs_context *fs_context_for_submount(struct file_system_type *fs_ty
 
 extern struct fs_context *vfs_dup_fs_context(struct fs_context *fc);
 extern int vfs_parse_fs_param(struct fs_context *fc, struct fs_parameter *param);
-extern int vfs_parse_fs_string(struct fs_context *fc, const char *key,
-			       const char *value, size_t v_size);
+extern int vfs_parse_fs_qstr(struct fs_context *fc, const char *key,
+				const struct qstr *value);
+static inline int vfs_parse_fs_string(struct fs_context *fc, const char *key,
+			       const char *value)
+{
+	return vfs_parse_fs_qstr(fc, key, value ? &QSTR(value) : NULL);
+}
 int vfs_parse_monolithic_sep(struct fs_context *fc, void *data,
 			     char *(*sep)(char **));
 extern int generic_parse_monolithic(struct fs_context *fc, void *data);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4283ed4e8f59..15375b45fd74 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -10201,8 +10201,7 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
 
 	pr_warn("NOTICE: Automounting of tracing to debugfs is deprecated and will be removed in 2030\n");
 
-	ret = vfs_parse_fs_string(fc, "source",
-				  "tracefs", strlen("tracefs"));
+	ret = vfs_parse_fs_string(fc, "source", "tracefs");
 	if (!ret)
 		mnt = fc_mount(fc);
 	else
-- 
2.47.2

[PATCH 2/2] do_nfs4_mount(): switch to vfs_parse_fs_string()

[Al Viro]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/nfs/nfs4super.c | 44 +++++++++++---------------------------------
 1 file changed, 11 insertions(+), 33 deletions(-)

diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c
index b29a26923ce0..5ec9c83f1ef0 100644
--- a/fs/nfs/nfs4super.c
+++ b/fs/nfs/nfs4super.c
@@ -149,21 +149,9 @@ static int do_nfs4_mount(struct nfs_server *server,
 	struct fs_context *root_fc;
 	struct vfsmount *root_mnt;
 	struct dentry *dentry;
-	size_t len;
+	char *source;
 	int ret;
 
-	struct fs_parameter param = {
-		.key	= "source",
-		.type	= fs_value_is_string,
-		.dirfd	= -1,
-	};
-
-	struct fs_parameter param_fsc = {
-		.key	= "fsc",
-		.type	= fs_value_is_string,
-		.dirfd	= -1,
-	};
-
 	if (IS_ERR(server))
 		return PTR_ERR(server);
 
@@ -181,15 +169,7 @@ static int do_nfs4_mount(struct nfs_server *server,
 	root_ctx->server = server;
 
 	if (ctx->fscache_uniq) {
-		len = strlen(ctx->fscache_uniq);
-		param_fsc.size = len;
-		param_fsc.string = kmemdup_nul(ctx->fscache_uniq, len, GFP_KERNEL);
-		if (param_fsc.string == NULL) {
-			put_fs_context(root_fc);
-			return -ENOMEM;
-		}
-		ret = vfs_parse_fs_param(root_fc, &param_fsc);
-		kfree(param_fsc.string);
+		ret = vfs_parse_fs_string(root_fc, "fsc", ctx->fscache_uniq);
 		if (ret < 0) {
 			put_fs_context(root_fc);
 			return ret;
@@ -197,20 +177,18 @@ static int do_nfs4_mount(struct nfs_server *server,
 	}
 	/* We leave export_path unset as it's not used to find the root. */
 
-	len = strlen(hostname) + 5;
-	param.string = kmalloc(len, GFP_KERNEL);
-	if (param.string == NULL) {
-		put_fs_context(root_fc);
-		return -ENOMEM;
-	}
-
 	/* Does hostname needs to be enclosed in brackets? */
 	if (strchr(hostname, ':'))
-		param.size = snprintf(param.string, len, "[%s]:/", hostname);
+		source = kasprintf(GFP_KERNEL, "[%s]:/", hostname);
 	else
-		param.size = snprintf(param.string, len, "%s:/", hostname);
-	ret = vfs_parse_fs_param(root_fc, &param);
-	kfree(param.string);
+		source = kasprintf(GFP_KERNEL, "%s:/", hostname);
+
+	if (!source) {
+		put_fs_context(root_fc);
+		return -ENOMEM;
+	}
+	ret = vfs_parse_fs_string(root_fc, "source", source);
+	kfree(source);
 	if (ret < 0) {
 		put_fs_context(root_fc);
 		return ret;
-- 
2.47.2

Re: [PATCH 1/2] change the calling conventions for vfs_parse_fs_string()

[Christian Brauner]

On Thu, Aug 28, 2025 at 01:00:39AM +0100, Al Viro wrote:
> Absolute majority of callers are passing the 4th argument equal to
> strlen() of the 3rd one.
> 
> Drop the v_size argument, add vfs_parse_fs_qstr() for the cases that
> want independent length.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>

Re: [PATCH 2/2] do_nfs4_mount(): switch to vfs_parse_fs_string()

[Christian Brauner]

On Thu, Aug 28, 2025 at 01:01:13AM +0100, Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>