diff mbox series

[RFC,v7,05/16] fuse: make args->in_args[0] to be always the header

Message ID 20241127-fuse-uring-for-6-10-rfc4-v7-5-934b3a69baca@ddn.com (mailing list archive)
State New
Headers show
Series fuse: fuse-over-io-uring | expand

Commit Message

Bernd Schubert Nov. 27, 2024, 1:40 p.m. UTC 
This change sets up FUSE operations to have headers in args.in_args[0],
even for opcodes without an actual header. We do this to prepare for
cleanly separating payload from headers in the future.

For opcodes without a header, we use a zero-sized struct as a
placeholder. This approach:
- Keeps things consistent across all FUSE operations
- Will help with payload alignment later
- Avoids future issues when header sizes change

Signed-off-by: Bernd Schubert <bschubert@ddn.com>
---
 fs/fuse/dax.c    | 11 ++++++-----
 fs/fuse/dev.c    |  9 +++++----
 fs/fuse/dir.c    | 32 ++++++++++++++++++--------------
 fs/fuse/fuse_i.h | 13 +++++++++++++
 fs/fuse/xattr.c  |  7 ++++---
 5 files changed, 46 insertions(+), 26 deletions(-)

Comments

Joanne Koong Nov. 28, 2024, 12:27 a.m. UTC | #1 
On Wed, Nov 27, 2024 at 5:41 AM Bernd Schubert <bschubert@ddn.com> wrote:
>
> This change sets up FUSE operations to have headers in args.in_args[0],
> even for opcodes without an actual header. We do this to prepare for
> cleanly separating payload from headers in the future.
>
> For opcodes without a header, we use a zero-sized struct as a
> placeholder. This approach:
> - Keeps things consistent across all FUSE operations
> - Will help with payload alignment later
> - Avoids future issues when header sizes change
>
> Signed-off-by: Bernd Schubert <bschubert@ddn.com>

This LGTM. It might be worth mentioning in the commit message that
this won't add zero-sized arg headers to request types that don't take
in any in args.

Reviewed-by: Joanne Koong <joannelkoong@gmail.com>
> ---
>  fs/fuse/dax.c    | 11 ++++++-----
>  fs/fuse/dev.c    |  9 +++++----
>  fs/fuse/dir.c    | 32 ++++++++++++++++++--------------
>  fs/fuse/fuse_i.h | 13 +++++++++++++
>  fs/fuse/xattr.c  |  7 ++++---
>  5 files changed, 46 insertions(+), 26 deletions(-)
>
> diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> index 12ef91d170bb3091ac35a33d2b9dc38330b00948..44bd30d448e4e8c1d8c6b0499e50564fa0828efc 100644
> --- a/fs/fuse/dax.c
> +++ b/fs/fuse/dax.c
> @@ -240,11 +240,12 @@ static int fuse_send_removemapping(struct inode *inode,
>
>         args.opcode = FUSE_REMOVEMAPPING;
>         args.nodeid = fi->nodeid;
> -       args.in_numargs = 2;
> -       args.in_args[0].size = sizeof(*inargp);
> -       args.in_args[0].value = inargp;
> -       args.in_args[1].size = inargp->count * sizeof(*remove_one);
> -       args.in_args[1].value = remove_one;
> +       args.in_numargs = 3;
> +       fuse_set_zero_arg0(&args);
> +       args.in_args[1].size = sizeof(*inargp);
> +       args.in_args[1].value = inargp;
> +       args.in_args[2].size = inargp->count * sizeof(*remove_one);
> +       args.in_args[2].value = remove_one;
>         return fuse_simple_request(fm, &args);
>  }
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index fd8898b0c1cca4d117982d5208d78078472b0dfb..63c3865aebb7811fdf4a5729b2181ee8321421dc 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1735,7 +1735,7 @@ static int fuse_retrieve(struct fuse_mount *fm, struct inode *inode,
>         args = &ap->args;
>         args->nodeid = outarg->nodeid;
>         args->opcode = FUSE_NOTIFY_REPLY;
> -       args->in_numargs = 2;
> +       args->in_numargs = 3;
>         args->in_pages = true;
>         args->end = fuse_retrieve_end;
>
> @@ -1762,9 +1762,10 @@ static int fuse_retrieve(struct fuse_mount *fm, struct inode *inode,
>         }
>         ra->inarg.offset = outarg->offset;
>         ra->inarg.size = total_len;
> -       args->in_args[0].size = sizeof(ra->inarg);
> -       args->in_args[0].value = &ra->inarg;
> -       args->in_args[1].size = total_len;
> +       fuse_set_zero_arg0(args);
> +       args->in_args[1].size = sizeof(ra->inarg);
> +       args->in_args[1].value = &ra->inarg;
> +       args->in_args[2].size = total_len;
>
>         err = fuse_simple_notify_reply(fm, args, outarg->notify_unique);
>         if (err)
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 54104dd48af7c94b312f1a8671c8905542d456c4..ccb240d4262f9399c9c90434aaeaf76b50f223ad 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -175,9 +175,10 @@ static void fuse_lookup_init(struct fuse_conn *fc, struct fuse_args *args,
>         memset(outarg, 0, sizeof(struct fuse_entry_out));
>         args->opcode = FUSE_LOOKUP;
>         args->nodeid = nodeid;
> -       args->in_numargs = 1;
> -       args->in_args[0].size = name->len + 1;
> -       args->in_args[0].value = name->name;
> +       args->in_numargs = 2;
> +       fuse_set_zero_arg0(args);
> +       args->in_args[1].size = name->len + 1;
> +       args->in_args[1].value = name->name;
>         args->out_numargs = 1;
>         args->out_args[0].size = sizeof(struct fuse_entry_out);
>         args->out_args[0].value = outarg;
> @@ -927,11 +928,12 @@ static int fuse_symlink(struct mnt_idmap *idmap, struct inode *dir,
>         FUSE_ARGS(args);
>
>         args.opcode = FUSE_SYMLINK;
> -       args.in_numargs = 2;
> -       args.in_args[0].size = entry->d_name.len + 1;
> -       args.in_args[0].value = entry->d_name.name;
> -       args.in_args[1].size = len;
> -       args.in_args[1].value = link;
> +       args.in_numargs = 3;
> +       fuse_set_zero_arg0(&args);
> +       args.in_args[1].size = entry->d_name.len + 1;
> +       args.in_args[1].value = entry->d_name.name;
> +       args.in_args[2].size = len;
> +       args.in_args[2].value = link;
>         return create_new_entry(idmap, fm, &args, dir, entry, S_IFLNK);
>  }
>
> @@ -991,9 +993,10 @@ static int fuse_unlink(struct inode *dir, struct dentry *entry)
>
>         args.opcode = FUSE_UNLINK;
>         args.nodeid = get_node_id(dir);
> -       args.in_numargs = 1;
> -       args.in_args[0].size = entry->d_name.len + 1;
> -       args.in_args[0].value = entry->d_name.name;
> +       args.in_numargs = 2;
> +       fuse_set_zero_arg0(&args);
> +       args.in_args[1].size = entry->d_name.len + 1;
> +       args.in_args[1].value = entry->d_name.name;
>         err = fuse_simple_request(fm, &args);
>         if (!err) {
>                 fuse_dir_changed(dir);
> @@ -1014,9 +1017,10 @@ static int fuse_rmdir(struct inode *dir, struct dentry *entry)
>
>         args.opcode = FUSE_RMDIR;
>         args.nodeid = get_node_id(dir);
> -       args.in_numargs = 1;
> -       args.in_args[0].size = entry->d_name.len + 1;
> -       args.in_args[0].value = entry->d_name.name;
> +       args.in_numargs = 2;
> +       fuse_set_zero_arg0(&args);
> +       args.in_args[1].size = entry->d_name.len + 1;
> +       args.in_args[1].value = entry->d_name.name;
>         err = fuse_simple_request(fm, &args);
>         if (!err) {
>                 fuse_dir_changed(dir);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index e6cc3d552b1382fc43bfe5191efc46e956ca268c..e3748751e231d0991c050b31bdd84db0b8016f9f 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -938,6 +938,19 @@ struct fuse_mount {
>         struct rcu_head rcu;
>  };
>
> +/*
> + * Empty header for FUSE opcodes without specific header needs.
> + * Used as a placeholder in args->in_args[0] for consistency
> + * across all FUSE operations, simplifying request handling.
> + */
> +struct fuse_zero_header {};
> +
> +static inline void fuse_set_zero_arg0(struct fuse_args *args)
> +{
> +       args->in_args[0].size = sizeof(struct fuse_zero_header);
> +       args->in_args[0].value = NULL;
> +}
> +
>  static inline struct fuse_mount *get_fuse_mount_super(struct super_block *sb)
>  {
>         return sb->s_fs_info;
> diff --git a/fs/fuse/xattr.c b/fs/fuse/xattr.c
> index 9f568d345c51236ddd421b162820a4ea9b0734f4..93dfb06b6cea045d6df90c61c900680968bda39f 100644
> --- a/fs/fuse/xattr.c
> +++ b/fs/fuse/xattr.c
> @@ -164,9 +164,10 @@ int fuse_removexattr(struct inode *inode, const char *name)
>
>         args.opcode = FUSE_REMOVEXATTR;
>         args.nodeid = get_node_id(inode);
> -       args.in_numargs = 1;
> -       args.in_args[0].size = strlen(name) + 1;
> -       args.in_args[0].value = name;
> +       args.in_numargs = 2;
> +       fuse_set_zero_arg0(&args);
> +       args.in_args[1].size = strlen(name) + 1;
> +       args.in_args[1].value = name;
>         err = fuse_simple_request(fm, &args);
>         if (err == -ENOSYS) {
>                 fm->fc->no_removexattr = 1;
>
> --
> 2.43.0
>
diff mbox series

Patch

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 12ef91d170bb3091ac35a33d2b9dc38330b00948..44bd30d448e4e8c1d8c6b0499e50564fa0828efc 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -240,11 +240,12 @@  static int fuse_send_removemapping(struct inode *inode,
 
 	args.opcode = FUSE_REMOVEMAPPING;
 	args.nodeid = fi->nodeid;
-	args.in_numargs = 2;
-	args.in_args[0].size = sizeof(*inargp);
-	args.in_args[0].value = inargp;
-	args.in_args[1].size = inargp->count * sizeof(*remove_one);
-	args.in_args[1].value = remove_one;
+	args.in_numargs = 3;
+	fuse_set_zero_arg0(&args);
+	args.in_args[1].size = sizeof(*inargp);
+	args.in_args[1].value = inargp;
+	args.in_args[2].size = inargp->count * sizeof(*remove_one);
+	args.in_args[2].value = remove_one;
 	return fuse_simple_request(fm, &args);
 }
 
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index fd8898b0c1cca4d117982d5208d78078472b0dfb..63c3865aebb7811fdf4a5729b2181ee8321421dc 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1735,7 +1735,7 @@  static int fuse_retrieve(struct fuse_mount *fm, struct inode *inode,
 	args = &ap->args;
 	args->nodeid = outarg->nodeid;
 	args->opcode = FUSE_NOTIFY_REPLY;
-	args->in_numargs = 2;
+	args->in_numargs = 3;
 	args->in_pages = true;
 	args->end = fuse_retrieve_end;
 
@@ -1762,9 +1762,10 @@  static int fuse_retrieve(struct fuse_mount *fm, struct inode *inode,
 	}
 	ra->inarg.offset = outarg->offset;
 	ra->inarg.size = total_len;
-	args->in_args[0].size = sizeof(ra->inarg);
-	args->in_args[0].value = &ra->inarg;
-	args->in_args[1].size = total_len;
+	fuse_set_zero_arg0(args);
+	args->in_args[1].size = sizeof(ra->inarg);
+	args->in_args[1].value = &ra->inarg;
+	args->in_args[2].size = total_len;
 
 	err = fuse_simple_notify_reply(fm, args, outarg->notify_unique);
 	if (err)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 54104dd48af7c94b312f1a8671c8905542d456c4..ccb240d4262f9399c9c90434aaeaf76b50f223ad 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -175,9 +175,10 @@  static void fuse_lookup_init(struct fuse_conn *fc, struct fuse_args *args,
 	memset(outarg, 0, sizeof(struct fuse_entry_out));
 	args->opcode = FUSE_LOOKUP;
 	args->nodeid = nodeid;
-	args->in_numargs = 1;
-	args->in_args[0].size = name->len + 1;
-	args->in_args[0].value = name->name;
+	args->in_numargs = 2;
+	fuse_set_zero_arg0(args);
+	args->in_args[1].size = name->len + 1;
+	args->in_args[1].value = name->name;
 	args->out_numargs = 1;
 	args->out_args[0].size = sizeof(struct fuse_entry_out);
 	args->out_args[0].value = outarg;
@@ -927,11 +928,12 @@  static int fuse_symlink(struct mnt_idmap *idmap, struct inode *dir,
 	FUSE_ARGS(args);
 
 	args.opcode = FUSE_SYMLINK;
-	args.in_numargs = 2;
-	args.in_args[0].size = entry->d_name.len + 1;
-	args.in_args[0].value = entry->d_name.name;
-	args.in_args[1].size = len;
-	args.in_args[1].value = link;
+	args.in_numargs = 3;
+	fuse_set_zero_arg0(&args);
+	args.in_args[1].size = entry->d_name.len + 1;
+	args.in_args[1].value = entry->d_name.name;
+	args.in_args[2].size = len;
+	args.in_args[2].value = link;
 	return create_new_entry(idmap, fm, &args, dir, entry, S_IFLNK);
 }
 
@@ -991,9 +993,10 @@  static int fuse_unlink(struct inode *dir, struct dentry *entry)
 
 	args.opcode = FUSE_UNLINK;
 	args.nodeid = get_node_id(dir);
-	args.in_numargs = 1;
-	args.in_args[0].size = entry->d_name.len + 1;
-	args.in_args[0].value = entry->d_name.name;
+	args.in_numargs = 2;
+	fuse_set_zero_arg0(&args);
+	args.in_args[1].size = entry->d_name.len + 1;
+	args.in_args[1].value = entry->d_name.name;
 	err = fuse_simple_request(fm, &args);
 	if (!err) {
 		fuse_dir_changed(dir);
@@ -1014,9 +1017,10 @@  static int fuse_rmdir(struct inode *dir, struct dentry *entry)
 
 	args.opcode = FUSE_RMDIR;
 	args.nodeid = get_node_id(dir);
-	args.in_numargs = 1;
-	args.in_args[0].size = entry->d_name.len + 1;
-	args.in_args[0].value = entry->d_name.name;
+	args.in_numargs = 2;
+	fuse_set_zero_arg0(&args);
+	args.in_args[1].size = entry->d_name.len + 1;
+	args.in_args[1].value = entry->d_name.name;
 	err = fuse_simple_request(fm, &args);
 	if (!err) {
 		fuse_dir_changed(dir);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index e6cc3d552b1382fc43bfe5191efc46e956ca268c..e3748751e231d0991c050b31bdd84db0b8016f9f 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -938,6 +938,19 @@  struct fuse_mount {
 	struct rcu_head rcu;
 };
 
+/*
+ * Empty header for FUSE opcodes without specific header needs.
+ * Used as a placeholder in args->in_args[0] for consistency
+ * across all FUSE operations, simplifying request handling.
+ */
+struct fuse_zero_header {};
+
+static inline void fuse_set_zero_arg0(struct fuse_args *args)
+{
+	args->in_args[0].size = sizeof(struct fuse_zero_header);
+	args->in_args[0].value = NULL;
+}
+
 static inline struct fuse_mount *get_fuse_mount_super(struct super_block *sb)
 {
 	return sb->s_fs_info;
diff --git a/fs/fuse/xattr.c b/fs/fuse/xattr.c
index 9f568d345c51236ddd421b162820a4ea9b0734f4..93dfb06b6cea045d6df90c61c900680968bda39f 100644
--- a/fs/fuse/xattr.c
+++ b/fs/fuse/xattr.c
@@ -164,9 +164,10 @@  int fuse_removexattr(struct inode *inode, const char *name)
 
 	args.opcode = FUSE_REMOVEXATTR;
 	args.nodeid = get_node_id(inode);
-	args.in_numargs = 1;
-	args.in_args[0].size = strlen(name) + 1;
-	args.in_args[0].value = name;
+	args.in_numargs = 2;
+	fuse_set_zero_arg0(&args);
+	args.in_args[1].size = strlen(name) + 1;
+	args.in_args[1].value = name;
 	err = fuse_simple_request(fm, &args);
 	if (err == -ENOSYS) {
 		fm->fc->no_removexattr = 1;