[PATCH v4 0/3] fuse: compound commands

[PATCH v4 0/3] fuse: compound commands

[Horst Birthelmer]

In the discussion about open+getattr here [1] Bernd and Miklos talked
about the need for a compound command in fuse that could send multiple
commands to a fuse server.
    
Here's a propsal for exactly that compound command with an example
(the mentioned open+getattr).
    
The pull request for libfuse is here [2]
That pull request contains a patch for handling compounds 
and a patch for passthrough_hp with a trivial implementation
of compounds that will decode and call the requests sequencially.

[1] https://lore.kernel.org/linux-fsdevel/CAJfpegshcrjXJ0USZ8RRdBy=e0MxmBTJSCE0xnxG8LXgXy-xuQ@mail.gmail.com/
[2] https://github.com/libfuse/libfuse/pull/1418

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
---
Changes in v4:
- removed RFC 
- removed the unnecessary 'parsed' variable in fuse_compound_req, since
  we parse the result only once
- reordered the patches about the helper functions to fill in the fuse
  args for open and getattr calls
- Link to v3: https://lore.kernel.org/r/20260108-fuse-compounds-upstream-v3-0-8dc91ebf3740@ddn.com

Changes in v3:
- simplified the data handling for compound commands
- remove the validating functionality, since it was only a helper for
  development
- remove fuse_compound_request() and use fuse_simple_request()
- add helper functions for creating args for open and attr
- use the newly createn helper functions for arg creation for open and
  getattr
- Link to v2: https://lore.kernel.org/r/20251223-fuse-compounds-upstream-v2-0-0f7b4451c85e@ddn.com

Changes in v2:
- fixed issues with error handling in the compounds as well as in the
  open+getattr
- Link to v1: https://lore.kernel.org/r/20251223-fuse-compounds-upstream-v1-0-7bade663947b@ddn.com

---
Horst Birthelmer (3):
      fuse: add compound command to combine multiple requests
      fuse: create helper functions for filling in fuse args for open and getattr
      fuse: add an implementation of open+getattr

 fs/fuse/Makefile          |   2 +-
 fs/fuse/compound.c        | 270 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/fuse/dir.c             |   9 +-
 fs/fuse/file.c            | 152 +++++++++++++++++++++-----
 fs/fuse/fuse_i.h          |  27 ++++-
 fs/fuse/inode.c           |   6 ++
 fs/fuse/ioctl.c           |   2 +-
 include/uapi/linux/fuse.h |  37 +++++++
 8 files changed, 470 insertions(+), 35 deletions(-)
---
base-commit: 9448598b22c50c8a5bb77a9103e2d49f134c9578
change-id: 20251223-fuse-compounds-upstream-c85b4e39b3d3

Best regards,
-- 
Horst Birthelmer <hbirthelmer@ddn.com>

[PATCH v4 1/3] fuse: add compound command to combine multiple requests

[Horst Birthelmer]

From: Horst Birthelmer <hbirthelmer@ddn.com>

For a FUSE_COMPOUND we add a header that contains information
about how many commands there are in the compound and about the
size of the expected result. This will make the interpretation
in libfuse easier, since we can preallocate the whole result.
Then we append the requests that belong to this compound.

The API for the compound command has:
  fuse_compound_alloc()
  fuse_compound_add()
  fuse_compound_send()
  fuse_compound_free()

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
---
 fs/fuse/Makefile          |   2 +-
 fs/fuse/compound.c        | 270 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/fuse/fuse_i.h          |  12 +++
 include/uapi/linux/fuse.h |  37 +++++++
 4 files changed, 320 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/Makefile b/fs/fuse/Makefile
index 22ad9538dfc4..4c09038ef995 100644
--- a/fs/fuse/Makefile
+++ b/fs/fuse/Makefile
@@ -11,7 +11,7 @@ obj-$(CONFIG_CUSE) += cuse.o
 obj-$(CONFIG_VIRTIO_FS) += virtiofs.o
 
 fuse-y := trace.o	# put trace.o first so we see ftrace errors sooner
-fuse-y += dev.o dir.o file.o inode.o control.o xattr.o acl.o readdir.o ioctl.o
+fuse-y += dev.o dir.o file.o inode.o control.o xattr.o acl.o readdir.o ioctl.o compound.o
 fuse-y += iomode.o
 fuse-$(CONFIG_FUSE_DAX) += dax.o
 fuse-$(CONFIG_FUSE_PASSTHROUGH) += passthrough.o backing.o
diff --git a/fs/fuse/compound.c b/fs/fuse/compound.c
new file mode 100644
index 000000000000..0f1e4c073d8b
--- /dev/null
+++ b/fs/fuse/compound.c
@@ -0,0 +1,270 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * FUSE: Filesystem in Userspace
+ * Copyright (C) 2025
+ *
+ * This file implements compound operations for FUSE, allowing multiple
+ * operations to be batched into a single request to reduce round trips
+ * between kernel and userspace.
+ */
+
+#include "fuse_i.h"
+
+/*
+ * Compound request builder and state tracker and args pointer storage
+ */
+struct fuse_compound_req {
+	struct fuse_mount *fm;
+	struct fuse_compound_in compound_header;
+	struct fuse_compound_out result_header;
+
+	/* Per-operation error codes */
+	int op_errors[FUSE_MAX_COMPOUND_OPS];
+	/* Original fuse_args for response parsing */
+	struct fuse_args *op_args[FUSE_MAX_COMPOUND_OPS];
+};
+
+struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm, u32 flags)
+{
+	struct fuse_compound_req *compound;
+
+	compound = kzalloc(sizeof(*compound), GFP_KERNEL);
+	if (!compound)
+		return ERR_PTR(-ENOMEM);
+
+	compound->fm = fm;
+	compound->compound_header.flags = flags;
+
+	return compound;
+}
+
+void fuse_compound_free(struct fuse_compound_req *compound)
+{
+	if (!compound)
+		return;
+
+	kfree(compound);
+}
+
+int fuse_compound_add(struct fuse_compound_req *compound,
+		      struct fuse_args *args)
+{
+	if (!compound ||
+	    compound->compound_header.count >= FUSE_MAX_COMPOUND_OPS)
+		return -EINVAL;
+
+	if (args->in_pages)
+		return -EINVAL;
+
+	compound->op_args[compound->compound_header.count] = args;
+	compound->compound_header.count++;
+	return 0;
+}
+
+static void *fuse_copy_response_data(struct fuse_args *args,
+				     char *response_data)
+{
+	size_t copied = 0;
+	int i;
+
+	for (i = 0; i < args->out_numargs; i++) {
+		struct fuse_arg current_arg = args->out_args[i];
+		size_t arg_size;
+
+		/*
+		 * Last argument with out_pages: copy to pages
+		 * External payload (in the last out arg) is not supported
+		 * at the moment
+		 */
+		if (i == args->out_numargs - 1 && args->out_pages)
+			return response_data;
+
+		arg_size = current_arg.size;
+
+		if (current_arg.value && arg_size > 0) {
+			memcpy(current_arg.value,
+			       (char *)response_data + copied, arg_size);
+			copied += arg_size;
+		}
+	}
+
+	return (char *)response_data + copied;
+}
+
+int fuse_compound_get_error(struct fuse_compound_req *compound, int op_idx)
+{
+	return compound->op_errors[op_idx];
+}
+
+static void *fuse_compound_parse_one_op(struct fuse_compound_req *compound,
+					int op_index, void *op_out_data,
+					void *response_end)
+{
+	struct fuse_out_header *op_hdr = op_out_data;
+	struct fuse_args *args = compound->op_args[op_index];
+
+	if (op_hdr->len < sizeof(struct fuse_out_header))
+		return NULL;
+
+	/* Check if the entire operation response fits in the buffer */
+	if ((char *)op_out_data + op_hdr->len > (char *)response_end)
+		return NULL;
+
+	if (op_hdr->error != 0)
+		compound->op_errors[op_index] = op_hdr->error;
+
+	if (args && op_hdr->len > sizeof(struct fuse_out_header))
+		return fuse_copy_response_data(args, op_out_data +
+					       sizeof(struct fuse_out_header));
+
+	/* No response data, just advance past the header */
+	return (char *)op_out_data + op_hdr->len;
+}
+
+static int fuse_compound_parse_resp(struct fuse_compound_req *compound,
+				    u32 count, void *response,
+				    size_t response_size)
+{
+	void *op_out_data = response;
+	void *response_end = (char *)response + response_size;
+	int i;
+
+	if (!response || response_size < sizeof(struct fuse_out_header))
+		return -EIO;
+
+	for (i = 0; i < count && i < compound->result_header.count; i++) {
+		op_out_data = fuse_compound_parse_one_op(compound, i,
+							 op_out_data,
+							 response_end);
+		if (!op_out_data)
+			return -EIO;
+	}
+
+	return 0;
+}
+
+ssize_t fuse_compound_send(struct fuse_compound_req *compound)
+{
+	struct fuse_args args = {
+		.opcode = FUSE_COMPOUND,
+		.nodeid = 0,
+		.in_numargs = 2,
+		.out_numargs = 2,
+		.out_argvar = true,
+	};
+	size_t resp_buffer_size;
+	size_t actual_response_size;
+	size_t buffer_pos;
+	size_t total_expected_out_size;
+	void *buffer = NULL;
+	void *resp_payload;
+	ssize_t ret;
+	int i;
+
+	if (!compound) {
+		pr_info_ratelimited("FUSE: compound request is NULL in %s\n",
+				    __func__);
+		return -EINVAL;
+	}
+
+	if (compound->compound_header.count == 0) {
+		pr_info_ratelimited("FUSE: compound request contains no operations\n");
+		return -EINVAL;
+	}
+
+	buffer_pos = 0;
+	total_expected_out_size = 0;
+
+	for (i = 0; i < compound->compound_header.count; i++) {
+		struct fuse_args *op_args = compound->op_args[i];
+		size_t needed_size = sizeof(struct fuse_in_header);
+		int j;
+
+		for (j = 0; j < op_args->in_numargs; j++)
+			needed_size += op_args->in_args[j].size;
+
+		buffer_pos += needed_size;
+
+		for (j = 0; j < op_args->out_numargs; j++)
+			total_expected_out_size += op_args->out_args[j].size;
+	}
+
+	buffer = kvmalloc(buffer_pos, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+
+	buffer_pos = 0;
+	for (i = 0; i < compound->compound_header.count; i++) {
+		struct fuse_args *op_args = compound->op_args[i];
+		struct fuse_in_header *hdr;
+		size_t needed_size = sizeof(struct fuse_in_header);
+		int j;
+
+		for (j = 0; j < op_args->in_numargs; j++)
+			needed_size += op_args->in_args[j].size;
+
+		hdr = (struct fuse_in_header *)(buffer + buffer_pos);
+		memset(hdr, 0, sizeof(*hdr));
+		hdr->len = needed_size;
+		hdr->opcode = op_args->opcode;
+		hdr->nodeid = op_args->nodeid;
+		hdr->uid = from_kuid(compound->fm->fc->user_ns,
+				     current_fsuid());
+		hdr->gid = from_kgid(compound->fm->fc->user_ns,
+				     current_fsgid());
+		hdr->pid = pid_nr_ns(task_pid(current),
+				     compound->fm->fc->pid_ns);
+		buffer_pos += sizeof(*hdr);
+
+		for (j = 0; j < op_args->in_numargs; j++) {
+			memcpy(buffer + buffer_pos, op_args->in_args[j].value,
+			       op_args->in_args[j].size);
+			buffer_pos += op_args->in_args[j].size;
+		}
+	}
+
+	resp_buffer_size = total_expected_out_size +
+			   (compound->compound_header.count *
+			    sizeof(struct fuse_out_header));
+
+	resp_payload = kvmalloc(resp_buffer_size, GFP_KERNEL | __GFP_ZERO);
+	if (!resp_payload) {
+		ret = -ENOMEM;
+		goto out_free_buffer;
+	}
+
+	compound->compound_header.result_size = total_expected_out_size;
+
+	args.in_args[0].size = sizeof(compound->compound_header);
+	args.in_args[0].value = &compound->compound_header;
+	args.in_args[1].size = buffer_pos;
+	args.in_args[1].value = buffer;
+
+	args.out_args[0].size = sizeof(compound->result_header);
+	args.out_args[0].value = &compound->result_header;
+	args.out_args[1].size = resp_buffer_size;
+	args.out_args[1].value = resp_payload;
+
+	ret = fuse_simple_request(compound->fm, &args);
+	if (ret < 0)
+		goto out;
+
+	actual_response_size = args.out_args[1].size;
+
+	if (actual_response_size < sizeof(struct fuse_compound_out)) {
+		pr_info_ratelimited("FUSE: compound response too small (%zu bytes, minimum %zu bytes)\n",
+				    actual_response_size,
+				    sizeof(struct fuse_compound_out));
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = fuse_compound_parse_resp(compound, compound->result_header.count,
+				       (char *)resp_payload,
+				       actual_response_size);
+out:
+	kvfree(resp_payload);
+out_free_buffer:
+	kvfree(buffer);
+	return ret;
+}
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 7f16049387d1..6dddbe2b027b 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1273,6 +1273,18 @@ static inline ssize_t fuse_simple_idmap_request(struct mnt_idmap *idmap,
 int fuse_simple_background(struct fuse_mount *fm, struct fuse_args *args,
 			   gfp_t gfp_flags);
 
+/**
+ * Compound request API
+ */
+struct fuse_compound_req;
+ssize_t fuse_compound_send(struct fuse_compound_req *compound);
+
+struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm, u32 flags);
+int fuse_compound_add(struct fuse_compound_req *compound,
+		      struct fuse_args *args);
+int fuse_compound_get_error(struct fuse_compound_req *compound, int op_idx);
+void fuse_compound_free(struct fuse_compound_req *compound);
+
 /**
  * Assign a unique id to a fuse request
  */
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index c13e1f9a2f12..4464f28c3242 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -664,6 +664,13 @@ enum fuse_opcode {
 	FUSE_STATX		= 52,
 	FUSE_COPY_FILE_RANGE_64	= 53,
 
+	/* A compound request is handled like a single request,
+	 * but contains multiple requests as input.
+	 * This can be used to signal to the fuse server that
+	 * the requests can be combined atomically.
+	 */
+	FUSE_COMPOUND		= 54,
+
 	/* CUSE specific operations */
 	CUSE_INIT		= 4096,
 
@@ -1245,6 +1252,36 @@ struct fuse_supp_groups {
 	uint32_t	groups[];
 };
 
+#define FUSE_MAX_COMPOUND_OPS   16        /* Maximum operations per compound */
+
+/*
+ * Compound request header
+ *
+ * This header is followed by the fuse requests
+ */
+struct fuse_compound_in {
+	uint32_t	count;			/* Number of operations */
+	uint32_t	flags;			/* Compound flags */
+
+	/* Total size of all results.
+	 * This is needed for preallocating the whole result for all
+	 * commands in this compound.
+	 */
+	uint32_t	result_size;
+	uint64_t	reserved;
+};
+
+/*
+ * Compound response header
+ *
+ * This header is followed by complete fuse responses
+ */
+struct fuse_compound_out {
+	uint32_t	count;     /* Number of results */
+	uint32_t	flags;     /* Result flags */
+	uint64_t	reserved;
+};
+
 /**
  * Size of the ring buffer header
  */

-- 
2.51.0

[PATCH v4 2/3] fuse: create helper functions for filling in fuse args for open and getattr

[Horst Birthelmer]

From: Horst Birthelmer <hbirthelmer@ddn.com>

create fuse_getattr_args_fill() and fuse_open_args_fill() to fill in
the parameters for the open and getattr calls.

This is in preparation for implementing open+getattr and does not
represent any functional change.

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
---
 fs/fuse/dir.c    |  9 +--------
 fs/fuse/file.c   | 42 ++++++++++++++++++++++++++++++++++--------
 fs/fuse/fuse_i.h |  8 ++++++++
 3 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 4b6b3d2758ff..ca8b69282c60 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1493,14 +1493,7 @@ static int fuse_do_getattr(struct mnt_idmap *idmap, struct inode *inode,
 		inarg.getattr_flags |= FUSE_GETATTR_FH;
 		inarg.fh = ff->fh;
 	}
-	args.opcode = FUSE_GETATTR;
-	args.nodeid = get_node_id(inode);
-	args.in_numargs = 1;
-	args.in_args[0].size = sizeof(inarg);
-	args.in_args[0].value = &inarg;
-	args.out_numargs = 1;
-	args.out_args[0].size = sizeof(outarg);
-	args.out_args[0].value = &outarg;
+	fuse_getattr_args_fill(&args, get_node_id(inode), &inarg, &outarg);
 	err = fuse_simple_request(fm, &args);
 	if (!err) {
 		if (fuse_invalid_attr(&outarg.attr) ||
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 01bc894e9c2b..53744559455d 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -23,6 +23,39 @@
 #include <linux/task_io_accounting_ops.h>
 #include <linux/iomap.h>
 
+/*
+ * Helper function to initialize fuse_args for OPEN/OPENDIR operations
+ */
+void fuse_open_args_fill(struct fuse_args *args, u64 nodeid, int opcode,
+			 struct fuse_open_in *inarg, struct fuse_open_out *outarg)
+{
+	args->opcode = opcode;
+	args->nodeid = nodeid;
+	args->in_numargs = 1;
+	args->in_args[0].size = sizeof(*inarg);
+	args->in_args[0].value = inarg;
+	args->out_numargs = 1;
+	args->out_args[0].size = sizeof(*outarg);
+	args->out_args[0].value = outarg;
+}
+
+/*
+ * Helper function to initialize fuse_args for GETATTR operations
+ */
+void fuse_getattr_args_fill(struct fuse_args *args, u64 nodeid,
+			     struct fuse_getattr_in *inarg,
+			     struct fuse_attr_out *outarg)
+{
+	args->opcode = FUSE_GETATTR;
+	args->nodeid = nodeid;
+	args->in_numargs = 1;
+	args->in_args[0].size = sizeof(*inarg);
+	args->in_args[0].value = inarg;
+	args->out_numargs = 1;
+	args->out_args[0].size = sizeof(*outarg);
+	args->out_args[0].value = outarg;
+}
+
 static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
 			  unsigned int open_flags, int opcode,
 			  struct fuse_open_out *outargp)
@@ -40,14 +73,7 @@ static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
 		inarg.open_flags |= FUSE_OPEN_KILL_SUIDGID;
 	}
 
-	args.opcode = opcode;
-	args.nodeid = nodeid;
-	args.in_numargs = 1;
-	args.in_args[0].size = sizeof(inarg);
-	args.in_args[0].value = &inarg;
-	args.out_numargs = 1;
-	args.out_args[0].size = sizeof(*outargp);
-	args.out_args[0].value = outargp;
+	fuse_open_args_fill(&args, nodeid, opcode, &inarg, outargp);
 
 	return fuse_simple_request(fm, &args);
 }
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 6dddbe2b027b..98ea41f76623 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1179,6 +1179,14 @@ struct fuse_io_args {
 void fuse_read_args_fill(struct fuse_io_args *ia, struct file *file, loff_t pos,
 			 size_t count, int opcode);
 
+/*
+ * Helper functions to initialize fuse_args for common operations
+ */
+void fuse_open_args_fill(struct fuse_args *args, u64 nodeid, int opcode,
+			 struct fuse_open_in *inarg, struct fuse_open_out *outarg);
+void fuse_getattr_args_fill(struct fuse_args *args, u64 nodeid,
+			    struct fuse_getattr_in *inarg,
+			    struct fuse_attr_out *outarg);
 
 struct fuse_file *fuse_file_alloc(struct fuse_mount *fm, bool release);
 void fuse_file_free(struct fuse_file *ff);

-- 
2.51.0

[PATCH v4 3/3] fuse: add an implementation of open+getattr

[Horst Birthelmer]

From: Horst Birthelmer <hbirthelmer@ddn.com>

The discussion about compound commands in fuse was
started over an argument to add a new operation that
will open a file and return its attributes in the same operation.

Here is a demonstration of that use case with compound commands.

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
---
 fs/fuse/file.c   | 110 +++++++++++++++++++++++++++++++++++++++++++++++--------
 fs/fuse/fuse_i.h |   7 +++-
 fs/fuse/inode.c  |   6 +++
 fs/fuse/ioctl.c  |   2 +-
 4 files changed, 107 insertions(+), 18 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 53744559455d..c0375b32967d 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -152,8 +152,66 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
 	}
 }
 
+static int fuse_compound_open_getattr(struct fuse_mount *fm, u64 nodeid,
+				      int flags, int opcode,
+				      struct fuse_file *ff,
+				      struct fuse_attr_out *outattrp,
+				      struct fuse_open_out *outopenp)
+{
+	struct fuse_compound_req *compound;
+	struct fuse_args open_args = {};
+	struct fuse_args getattr_args = {};
+	struct fuse_open_in open_in = {};
+	struct fuse_getattr_in getattr_in = {};
+	int err;
+
+	compound = fuse_compound_alloc(fm, 0);
+	if (IS_ERR(compound))
+		return PTR_ERR(compound);
+
+	open_in.flags = flags & ~(O_CREAT | O_EXCL | O_NOCTTY);
+	if (!fm->fc->atomic_o_trunc)
+		open_in.flags &= ~O_TRUNC;
+
+	if (fm->fc->handle_killpriv_v2 &&
+	    (open_in.flags & O_TRUNC) && !capable(CAP_FSETID))
+		open_in.open_flags |= FUSE_OPEN_KILL_SUIDGID;
+
+	fuse_open_args_fill(&open_args, nodeid, opcode, &open_in, outopenp);
+
+	err = fuse_compound_add(compound, &open_args);
+	if (err)
+		goto out;
+
+	fuse_getattr_args_fill(&getattr_args, nodeid, &getattr_in, outattrp);
+
+	err = fuse_compound_add(compound, &getattr_args);
+	if (err)
+		goto out;
+
+	err = fuse_compound_send(compound);
+	if (err)
+		goto out;
+
+	err = fuse_compound_get_error(compound, 0);
+	if (err)
+		goto out;
+
+	err = fuse_compound_get_error(compound, 1);
+	if (err)
+		goto out;
+
+	ff->fh = outopenp->fh;
+	ff->open_flags = outopenp->open_flags;
+
+out:
+	fuse_compound_free(compound);
+	return err;
+}
+
 struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
-				 unsigned int open_flags, bool isdir)
+				struct inode *inode,
+				unsigned int open_flags, bool isdir)
 {
 	struct fuse_conn *fc = fm->fc;
 	struct fuse_file *ff;
@@ -179,23 +237,44 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
 	if (open) {
 		/* Store outarg for fuse_finish_open() */
 		struct fuse_open_out *outargp = &ff->args->open_outarg;
-		int err;
+		int err = -ENOSYS;
+
+		if (inode && fc->compound_open_getattr) {
+			struct fuse_attr_out attr_outarg;
+
+			err = fuse_compound_open_getattr(fm, nodeid, open_flags,
+							 opcode, ff,
+							 &attr_outarg, outargp);
+			if (!err)
+				fuse_change_attributes(inode, &attr_outarg.attr,
+						       NULL,
+						       ATTR_TIMEOUT(&attr_outarg),
+						       fuse_get_attr_version(fc));
+		}
+		if (err == -ENOSYS) {
+			err = fuse_send_open(fm, nodeid, open_flags, opcode,
+					     outargp);
+			if (!err) {
+				ff->fh = outargp->fh;
+				ff->open_flags = outargp->open_flags;
+			}
+		}
 
-		err = fuse_send_open(fm, nodeid, open_flags, opcode, outargp);
-		if (!err) {
-			ff->fh = outargp->fh;
-			ff->open_flags = outargp->open_flags;
-		} else if (err != -ENOSYS) {
-			fuse_file_free(ff);
-			return ERR_PTR(err);
-		} else {
-			if (isdir) {
+		if (err) {
+			if (err != -ENOSYS) {
+				/* err is not ENOSYS */
+				fuse_file_free(ff);
+				return ERR_PTR(err);
+			} else {
 				/* No release needed */
 				kfree(ff->args);
 				ff->args = NULL;
-				fc->no_opendir = 1;
-			} else {
-				fc->no_open = 1;
+
+				/* we don't have open */
+				if (isdir)
+					fc->no_opendir = 1;
+				else
+					fc->no_open = 1;
 			}
 		}
 	}
@@ -211,11 +290,10 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
 int fuse_do_open(struct fuse_mount *fm, u64 nodeid, struct file *file,
 		 bool isdir)
 {
-	struct fuse_file *ff = fuse_file_open(fm, nodeid, file->f_flags, isdir);
+	struct fuse_file *ff = fuse_file_open(fm, nodeid, file_inode(file), file->f_flags, isdir);
 
 	if (!IS_ERR(ff))
 		file->private_data = ff;
-
 	return PTR_ERR_OR_ZERO(ff);
 }
 EXPORT_SYMBOL_GPL(fuse_do_open);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 98ea41f76623..e7828405e262 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -924,6 +924,9 @@ struct fuse_conn {
 	/* Use io_uring for communication */
 	unsigned int io_uring;
 
+	/* Does the filesystem support compound operations? */
+	unsigned int compound_open_getattr:1;
+
 	/** Maximum stack depth for passthrough backing files */
 	int max_stack_depth;
 
@@ -1563,7 +1566,9 @@ void fuse_file_io_release(struct fuse_file *ff, struct inode *inode);
 
 /* file.c */
 struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
-				 unsigned int open_flags, bool isdir);
+								struct inode *inode,
+								unsigned int open_flags,
+								bool isdir);
 void fuse_file_release(struct inode *inode, struct fuse_file *ff,
 		       unsigned int open_flags, fl_owner_t id, bool isdir);
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 819e50d66622..a5fd721be96d 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -991,6 +991,12 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
 	fc->blocked = 0;
 	fc->initialized = 0;
 	fc->connected = 1;
+
+	/* pretend fuse server supports compound operations
+	 * until it tells us otherwise.
+	 */
+	fc->compound_open_getattr = 1;
+
 	atomic64_set(&fc->attr_version, 1);
 	atomic64_set(&fc->evict_ctr, 1);
 	get_random_bytes(&fc->scramble_key, sizeof(fc->scramble_key));
diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
index fdc175e93f74..07a02e47b2c3 100644
--- a/fs/fuse/ioctl.c
+++ b/fs/fuse/ioctl.c
@@ -494,7 +494,7 @@ static struct fuse_file *fuse_priv_ioctl_prepare(struct inode *inode)
 	if (!S_ISREG(inode->i_mode) && !isdir)
 		return ERR_PTR(-ENOTTY);
 
-	return fuse_file_open(fm, get_node_id(inode), O_RDONLY, isdir);
+	return fuse_file_open(fm, get_node_id(inode), NULL, O_RDONLY, isdir);
 }
 
 static void fuse_priv_ioctl_cleanup(struct inode *inode, struct fuse_file *ff)

-- 
2.51.0

Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr

[Joanne Koong]

On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>
> From: Horst Birthelmer <hbirthelmer@ddn.com>
>
> The discussion about compound commands in fuse was
> started over an argument to add a new operation that
> will open a file and return its attributes in the same operation.
>
> Here is a demonstration of that use case with compound commands.
>
> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> ---
>  fs/fuse/file.c   | 110 +++++++++++++++++++++++++++++++++++++++++++++++--------
>  fs/fuse/fuse_i.h |   7 +++-
>  fs/fuse/inode.c  |   6 +++
>  fs/fuse/ioctl.c  |   2 +-
>  4 files changed, 107 insertions(+), 18 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 53744559455d..c0375b32967d 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -152,8 +152,66 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
>         }
>  }
>
> +static int fuse_compound_open_getattr(struct fuse_mount *fm, u64 nodeid,
> +                                     int flags, int opcode,
> +                                     struct fuse_file *ff,
> +                                     struct fuse_attr_out *outattrp,
> +                                     struct fuse_open_out *outopenp)
> +{
> +       struct fuse_compound_req *compound;
> +       struct fuse_args open_args = {};
> +       struct fuse_args getattr_args = {};
> +       struct fuse_open_in open_in = {};
> +       struct fuse_getattr_in getattr_in = {};
> +       int err;
> +
> +       compound = fuse_compound_alloc(fm, 0);
> +       if (IS_ERR(compound))
> +               return PTR_ERR(compound);
> +
> +       open_in.flags = flags & ~(O_CREAT | O_EXCL | O_NOCTTY);
> +       if (!fm->fc->atomic_o_trunc)
> +               open_in.flags &= ~O_TRUNC;
> +
> +       if (fm->fc->handle_killpriv_v2 &&
> +           (open_in.flags & O_TRUNC) && !capable(CAP_FSETID))
> +               open_in.open_flags |= FUSE_OPEN_KILL_SUIDGID;
> +
> +       fuse_open_args_fill(&open_args, nodeid, opcode, &open_in, outopenp);
> +
> +       err = fuse_compound_add(compound, &open_args);
> +       if (err)
> +               goto out;
> +
> +       fuse_getattr_args_fill(&getattr_args, nodeid, &getattr_in, outattrp);
> +
> +       err = fuse_compound_add(compound, &getattr_args);
> +       if (err)
> +               goto out;
> +
> +       err = fuse_compound_send(compound);
> +       if (err)
> +               goto out;
> +
> +       err = fuse_compound_get_error(compound, 0);
> +       if (err)
> +               goto out;
> +
> +       err = fuse_compound_get_error(compound, 1);
> +       if (err)
> +               goto out;

Hmm, if the open succeeds but the getattr fails, why not process it
kernel-side as a success for the open? Especially since on the server
side, libfuse will disassemble the compound request into separate
ones, so the server has no idea the open is even part of a compound.

I haven't looked at the rest of the patch yet but this caught my
attention when i was looking at how fuse_compound_get_error() gets
used.

Thanks,
Joanne

> +
> +       ff->fh = outopenp->fh;
> +       ff->open_flags = outopenp->open_flags;
> +
> +out:
> +       fuse_compound_free(compound);
> +       return err;
> +}
> +

Re: [PATCH v4 2/3] fuse: create helper functions for filling in fuse args for open and getattr

[Joanne Koong]

On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>
> From: Horst Birthelmer <hbirthelmer@ddn.com>
>
> create fuse_getattr_args_fill() and fuse_open_args_fill() to fill in
> the parameters for the open and getattr calls.
>
> This is in preparation for implementing open+getattr and does not
> represent any functional change.
>
> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> ---
>  fs/fuse/dir.c    |  9 +--------
>  fs/fuse/file.c   | 42 ++++++++++++++++++++++++++++++++++--------
>  fs/fuse/fuse_i.h |  8 ++++++++
>  3 files changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 4b6b3d2758ff..ca8b69282c60 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -1493,14 +1493,7 @@ static int fuse_do_getattr(struct mnt_idmap *idmap, struct inode *inode,
>                 inarg.getattr_flags |= FUSE_GETATTR_FH;
>                 inarg.fh = ff->fh;
>         }
> -       args.opcode = FUSE_GETATTR;
> -       args.nodeid = get_node_id(inode);
> -       args.in_numargs = 1;
> -       args.in_args[0].size = sizeof(inarg);
> -       args.in_args[0].value = &inarg;
> -       args.out_numargs = 1;
> -       args.out_args[0].size = sizeof(outarg);
> -       args.out_args[0].value = &outarg;
> +       fuse_getattr_args_fill(&args, get_node_id(inode), &inarg, &outarg);
>         err = fuse_simple_request(fm, &args);
>         if (!err) {
>                 if (fuse_invalid_attr(&outarg.attr) ||
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 01bc894e9c2b..53744559455d 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -23,6 +23,39 @@
>  #include <linux/task_io_accounting_ops.h>
>  #include <linux/iomap.h>
>
> +/*
> + * Helper function to initialize fuse_args for OPEN/OPENDIR operations
> + */
> +void fuse_open_args_fill(struct fuse_args *args, u64 nodeid, int opcode,
> +                        struct fuse_open_in *inarg, struct fuse_open_out *outarg)
> +{
> +       args->opcode = opcode;
> +       args->nodeid = nodeid;
> +       args->in_numargs = 1;
> +       args->in_args[0].size = sizeof(*inarg);
> +       args->in_args[0].value = inarg;
> +       args->out_numargs = 1;
> +       args->out_args[0].size = sizeof(*outarg);
> +       args->out_args[0].value = outarg;
> +}
> +
> +/*
> + * Helper function to initialize fuse_args for GETATTR operations
> + */
> +void fuse_getattr_args_fill(struct fuse_args *args, u64 nodeid,
> +                            struct fuse_getattr_in *inarg,
> +                            struct fuse_attr_out *outarg)
> +{
> +       args->opcode = FUSE_GETATTR;
> +       args->nodeid = nodeid;
> +       args->in_numargs = 1;
> +       args->in_args[0].size = sizeof(*inarg);
> +       args->in_args[0].value = inarg;
> +       args->out_numargs = 1;
> +       args->out_args[0].size = sizeof(*outarg);
> +       args->out_args[0].value = outarg;
> +}

sorry to be so nitpicky but I think we should move this to the
fuse/dir.c file since that's where the main fuse_do_getattr() function
lives

> +
>  static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
>                           unsigned int open_flags, int opcode,
>                           struct fuse_open_out *outargp)
> @@ -40,14 +73,7 @@ static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
>                 inarg.open_flags |= FUSE_OPEN_KILL_SUIDGID;
>         }
>
> -       args.opcode = opcode;
> -       args.nodeid = nodeid;
> -       args.in_numargs = 1;
> -       args.in_args[0].size = sizeof(inarg);
> -       args.in_args[0].value = &inarg;
> -       args.out_numargs = 1;
> -       args.out_args[0].size = sizeof(*outargp);
> -       args.out_args[0].value = outargp;
> +       fuse_open_args_fill(&args, nodeid, opcode, &inarg, outargp);
>
>         return fuse_simple_request(fm, &args);
>  }
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 6dddbe2b027b..98ea41f76623 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1179,6 +1179,14 @@ struct fuse_io_args {
>  void fuse_read_args_fill(struct fuse_io_args *ia, struct file *file, loff_t pos,
>                          size_t count, int opcode);
>
> +/*
> + * Helper functions to initialize fuse_args for common operations
> + */
> +void fuse_open_args_fill(struct fuse_args *args, u64 nodeid, int opcode,
> +                        struct fuse_open_in *inarg, struct fuse_open_out *outarg);

I don't think we need this for fuse_open_args_fill() here since it'll
be used only in the scope of fuse/file.c

Thanks,
Joanne

> +void fuse_getattr_args_fill(struct fuse_args *args, u64 nodeid,
> +                           struct fuse_getattr_in *inarg,
> +                           struct fuse_attr_out *outarg);
>
>  struct fuse_file *fuse_file_alloc(struct fuse_mount *fm, bool release);
>  void fuse_file_free(struct fuse_file *ff);
>
> --
> 2.51.0
>

Re: [PATCH v4 1/3] fuse: add compound command to combine multiple requests

[Joanne Koong]

On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>
> From: Horst Birthelmer <hbirthelmer@ddn.com>
>
> For a FUSE_COMPOUND we add a header that contains information
> about how many commands there are in the compound and about the
> size of the expected result. This will make the interpretation
> in libfuse easier, since we can preallocate the whole result.
> Then we append the requests that belong to this compound.
>
> The API for the compound command has:
>   fuse_compound_alloc()
>   fuse_compound_add()
>   fuse_compound_send()
>   fuse_compound_free()
>
> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> ---
>  fs/fuse/Makefile          |   2 +-
>  fs/fuse/compound.c        | 270 ++++++++++++++++++++++++++++++++++++++++++++++
>  fs/fuse/fuse_i.h          |  12 +++
>  include/uapi/linux/fuse.h |  37 +++++++
>  4 files changed, 320 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fuse/Makefile b/fs/fuse/Makefile
> index 22ad9538dfc4..4c09038ef995 100644
> --- a/fs/fuse/Makefile
> +++ b/fs/fuse/Makefile
> @@ -11,7 +11,7 @@ obj-$(CONFIG_CUSE) += cuse.o
>  obj-$(CONFIG_VIRTIO_FS) += virtiofs.o
>
>  fuse-y := trace.o      # put trace.o first so we see ftrace errors sooner
> -fuse-y += dev.o dir.o file.o inode.o control.o xattr.o acl.o readdir.o ioctl.o
> +fuse-y += dev.o dir.o file.o inode.o control.o xattr.o acl.o readdir.o ioctl.o compound.o
>  fuse-y += iomode.o
>  fuse-$(CONFIG_FUSE_DAX) += dax.o
>  fuse-$(CONFIG_FUSE_PASSTHROUGH) += passthrough.o backing.o
> diff --git a/fs/fuse/compound.c b/fs/fuse/compound.c
> new file mode 100644
> index 000000000000..0f1e4c073d8b
> --- /dev/null
> +++ b/fs/fuse/compound.c
> @@ -0,0 +1,270 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * FUSE: Filesystem in Userspace
> + * Copyright (C) 2025
> + *
> + * This file implements compound operations for FUSE, allowing multiple
> + * operations to be batched into a single request to reduce round trips
> + * between kernel and userspace.
> + */
> +
> +#include "fuse_i.h"
> +
> +/*
> + * Compound request builder and state tracker and args pointer storage
> + */
> +struct fuse_compound_req {
> +       struct fuse_mount *fm;
> +       struct fuse_compound_in compound_header;
> +       struct fuse_compound_out result_header;
> +
> +       /* Per-operation error codes */
> +       int op_errors[FUSE_MAX_COMPOUND_OPS];
> +       /* Original fuse_args for response parsing */
> +       struct fuse_args *op_args[FUSE_MAX_COMPOUND_OPS];
> +};
> +
> +struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm, u32 flags)
> +{
> +       struct fuse_compound_req *compound;
> +
> +       compound = kzalloc(sizeof(*compound), GFP_KERNEL);
> +       if (!compound)
> +               return ERR_PTR(-ENOMEM);

imo this logic is cleaner with just returning NULL here and then on
the caller side doing

    compound = fuse_compound_alloc(fm, 0);
    if (!compound)
          return -ENOMEM;

instead of having to do

    if (IS_ERR(compound))
           return PTR_ERR(compound);

and dealing with all the err_ptr/ptr_err stuff

> +
> +       compound->fm = fm;
> +       compound->compound_header.flags = flags;
> +
> +       return compound;
> +}
> +
> +void fuse_compound_free(struct fuse_compound_req *compound)
> +{
> +       if (!compound)
> +               return;

The implementation of kfree (in mm/slub.c) already has a null check

> +
> +       kfree(compound);
> +}
> +
> +int fuse_compound_add(struct fuse_compound_req *compound,
> +                     struct fuse_args *args)
> +{
> +       if (!compound ||
> +           compound->compound_header.count >= FUSE_MAX_COMPOUND_OPS)
> +               return -EINVAL;
> +
> +       if (args->in_pages)
> +               return -EINVAL;
> +
> +       compound->op_args[compound->compound_header.count] = args;
> +       compound->compound_header.count++;
> +       return 0;
> +}
> +
> +static void *fuse_copy_response_data(struct fuse_args *args,
> +                                    char *response_data)
> +{
> +       size_t copied = 0;
> +       int i;
> +
> +       for (i = 0; i < args->out_numargs; i++) {
> +               struct fuse_arg current_arg = args->out_args[i];

struct fuse_arg *current_arg = &args->out_args[i]; instead? that would
avoid the extra stack copy

> +               size_t arg_size;
> +
> +               /*
> +                * Last argument with out_pages: copy to pages
> +                * External payload (in the last out arg) is not supported
> +                * at the moment
> +                */
> +               if (i == args->out_numargs - 1 && args->out_pages)
> +                       return response_data;

imo this should be detected and error-ed out at the
fuse_compound_send() layer or is there some reason we can't do that?

> +
> +               arg_size = current_arg.size;
> +
> +               if (current_arg.value && arg_size > 0) {

If this is part of the args->out_numargs tally, then I think it's
guaranteed here that arg->value is non-NULL and arg->size > 0 (iirc,
the only exception to this is if out_pages is set, then arg->value is
null, but that's already protected against above).

> +                       memcpy(current_arg.value,
> +                              (char *)response_data + copied, arg_size);

Instead of doing "response_data + copied" arithmetic and needing the
copied variable, what about just updating the ptr as we go? i think
that'd look cleaner

> +                       copied += arg_size;
> +               }
> +       }
> +
> +       return (char *)response_data + copied;

is this cast needed? afaict, response_data is already a char *?

> +}
> +
> +int fuse_compound_get_error(struct fuse_compound_req *compound, int op_idx)
> +{
> +       return compound->op_errors[op_idx];
> +}

Hmm, looking at how this gets used by fuse_compound_open_getattr(), it
seems like a more useful api is one that scans through op_errors for
all the op indexes in the compound and returns back the first error it
encounters; then the caller doesn't need to call
fuse_compound_get_error() for every op index in the compound.

> +
> +static void *fuse_compound_parse_one_op(struct fuse_compound_req *compound,
> +                                       int op_index, void *op_out_data,
> +                                       void *response_end)
> +{
> +       struct fuse_out_header *op_hdr = op_out_data;
> +       struct fuse_args *args = compound->op_args[op_index];

op_index is taken straight from "compound->result_header.count" which
can be any value set by the server. I think we need to either add
checking for op_index value here or add checking for that in
fuse_compound_send() before it calls fuse_compound_parse_resp()

> +
> +       if (op_hdr->len < sizeof(struct fuse_out_header))
> +               return NULL;
> +
> +       /* Check if the entire operation response fits in the buffer */
> +       if ((char *)op_out_data + op_hdr->len > (char *)response_end)

Is there a reason this doesn't just define response_end as a char * in
fuse_compound_parse_resp() and pass that as a char * to this function?

> +               return NULL;
> +
> +       if (op_hdr->error != 0)

nit: this could just be "if (op_hdr->error)"

> +               compound->op_errors[op_index] = op_hdr->error;

If this errors out, is the memcpy still needed? or should this just return NULL?

> +
> +       if (args && op_hdr->len > sizeof(struct fuse_out_header))

imo,  args should be checked right after the "struct fuse_args *args =
compound->op_args[op_index];" line in the beginning and if it's null,
t hen fuse_compound_parse_one_op() should return NULL.

> +               return fuse_copy_response_data(args, op_out_data +
> +                                              sizeof(struct fuse_out_header));
> +
> +       /* No response data, just advance past the header */
> +       return (char *)op_out_data + op_hdr->len;
> +}
> +
> +static int fuse_compound_parse_resp(struct fuse_compound_req *compound,
> +                                   u32 count, void *response,
> +                                   size_t response_size)
> +{
> +       void *op_out_data = response;

imo it's cleaner to just use response instead of defining a new
op_out_data variable. And if we change the response arg to char *
response instead of void * response, then that gets rid of a lot of
the char * casting.

> +       void *response_end = (char *)response + response_size;
> +       int i;
> +
> +       if (!response || response_size < sizeof(struct fuse_out_header))
> +               return -EIO;
> +
> +       for (i = 0; i < count && i < compound->result_header.count; i++) {

count is already compound->result_header.count or am I missing something here?

> +               op_out_data = fuse_compound_parse_one_op(compound, i,
> +                                                        op_out_data,
> +                                                        response_end);
> +               if (!op_out_data)
> +                       return -EIO;
> +       }
> +
> +       return 0;
> +}
> +
> +ssize_t fuse_compound_send(struct fuse_compound_req *compound)
> +{
> +       struct fuse_args args = {
> +               .opcode = FUSE_COMPOUND,
> +               .nodeid = 0,

nit: this line can be removed

> +               .in_numargs = 2,
> +               .out_numargs = 2,
> +               .out_argvar = true,
> +       };
> +       size_t resp_buffer_size;
> +       size_t actual_response_size;
> +       size_t buffer_pos;
> +       size_t total_expected_out_size;
> +       void *buffer = NULL;
> +       void *resp_payload;
> +       ssize_t ret;
> +       int i;
> +
> +       if (!compound) {
> +               pr_info_ratelimited("FUSE: compound request is NULL in %s\n",
> +                                   __func__);
> +               return -EINVAL;
> +       }
> +
> +       if (compound->compound_header.count == 0) {
> +               pr_info_ratelimited("FUSE: compound request contains no operations\n");
> +               return -EINVAL;
> +       }

imo we can get rid of these two checks

> +
> +       buffer_pos = 0;
> +       total_expected_out_size = 0;

Could you move this initialization to the top where the variables get defined?

> +
> +       for (i = 0; i < compound->compound_header.count; i++) {

compound->compound_header.count gets used several times in this
function. i think it's worth declaring this as a separate (and less
verbose :)) variable

> +               struct fuse_args *op_args = compound->op_args[i];
> +               size_t needed_size = sizeof(struct fuse_in_header);
> +               int j;
> +
> +               for (j = 0; j < op_args->in_numargs; j++)
> +                       needed_size += op_args->in_args[j].size;
> +
> +               buffer_pos += needed_size;
> +
> +               for (j = 0; j < op_args->out_numargs; j++)
> +                       total_expected_out_size += op_args->out_args[j].size;
> +       }
> +
> +       buffer = kvmalloc(buffer_pos, GFP_KERNEL);

nit: imo it's cleaner to rename resp_buffer_size to buffer_size and
use that variable for this instead of using buffer_pos

imo I don't think we need kvmalloc here or below for the resp buffer
given that compound requests don't support in_pages or out_pages.

> +       if (!buffer)
> +               return -ENOMEM;
> +
> +       buffer_pos = 0;
> +       for (i = 0; i < compound->compound_header.count; i++) {
> +               struct fuse_args *op_args = compound->op_args[i];
> +               struct fuse_in_header *hdr;
> +               size_t needed_size = sizeof(struct fuse_in_header);
> +               int j;
> +
> +               for (j = 0; j < op_args->in_numargs; j++)
> +                       needed_size += op_args->in_args[j].size;

don't we already have the op_args->in_args[] needed size from the
computation abvoe? could we just reuse that?

> +
> +               hdr = (struct fuse_in_header *)(buffer + buffer_pos);
> +               memset(hdr, 0, sizeof(*hdr));
> +               hdr->len = needed_size;
> +               hdr->opcode = op_args->opcode;
> +               hdr->nodeid = op_args->nodeid;
> +               hdr->uid = from_kuid(compound->fm->fc->user_ns,
> +                                    current_fsuid());
> +               hdr->gid = from_kgid(compound->fm->fc->user_ns,
> +                                    current_fsgid());
> +               hdr->pid = pid_nr_ns(task_pid(current),
> +                                    compound->fm->fc->pid_ns);

at this point i think it's worth just defining fc at the top of the
function and using that here

> +               buffer_pos += sizeof(*hdr);
> +
> +               for (j = 0; j < op_args->in_numargs; j++) {
> +                       memcpy(buffer + buffer_pos, op_args->in_args[j].value,
> +                              op_args->in_args[j].size);
> +                       buffer_pos += op_args->in_args[j].size;
> +               }
> +       }

imo this would look nicer as a separate helper function

> +
> +       resp_buffer_size = total_expected_out_size +
> +                          (compound->compound_header.count *
> +                           sizeof(struct fuse_out_header));
> +
> +       resp_payload = kvmalloc(resp_buffer_size, GFP_KERNEL | __GFP_ZERO);

kvzalloc()?

> +       if (!resp_payload) {
> +               ret = -ENOMEM;
> +               goto out_free_buffer;
> +       }
> +
> +       compound->compound_header.result_size = total_expected_out_size;
> +
> +       args.in_args[0].size = sizeof(compound->compound_header);
> +       args.in_args[0].value = &compound->compound_header;
> +       args.in_args[1].size = buffer_pos;
> +       args.in_args[1].value = buffer;
> +
> +       args.out_args[0].size = sizeof(compound->result_header);
> +       args.out_args[0].value = &compound->result_header;
> +       args.out_args[1].size = resp_buffer_size;
> +       args.out_args[1].value = resp_payload;
> +
> +       ret = fuse_simple_request(compound->fm, &args);
> +       if (ret < 0)
> +               goto out;
> +
> +       actual_response_size = args.out_args[1].size;

since out_argvar was set for the FUSE_COMPOUND request args, ret is
already args.out_args[1].size (see  __fuse_simple_request())

> +
> +       if (actual_response_size < sizeof(struct fuse_compound_out)) {

can you explain why this checks against size of struct
fuse_compound_out? afaict from the logic in
fuse_compound_parse_resp(), actual_response_size doesn't include the
size of the compound_out struct?

Thanks,
Joanne


> +               pr_info_ratelimited("FUSE: compound response too small (%zu bytes, minimum %zu bytes)\n",
> +                                   actual_response_size,
> +                                   sizeof(struct fuse_compound_out));
> +               ret = -EINVAL;
> +               goto out;
> +       }
> +

Re: Re: [PATCH v4 1/3] fuse: add compound command to combine multiple requests

[Horst Birthelmer]

Hi Joanne,

thanks for taking the time!

On Wed, Jan 14, 2026 at 06:40:59PM -0800, Joanne Koong wrote:
> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >
> > From: Horst Birthelmer <hbirthelmer@ddn.com>
> >
> > For a FUSE_COMPOUND we add a header that contains information
> > about how many commands there are in the compound and about the
> > size of the expected result. This will make the interpretation
> > in libfuse easier, since we can preallocate the whole result.
> > Then we append the requests that belong to this compound.
> >
> > The API for the compound command has:
> >   fuse_compound_alloc()
> >   fuse_compound_add()
> >   fuse_compound_send()
> >   fuse_compound_free()
> >
> > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > ---
> >  fs/fuse/Makefile          |   2 +-
> >  fs/fuse/compound.c        | 270 ++++++++++++++++++++++++++++++++++++++++++++++
> >  fs/fuse/fuse_i.h          |  12 +++
> >  include/uapi/linux/fuse.h |  37 +++++++
> >  4 files changed, 320 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/fuse/Makefile b/fs/fuse/Makefile
> > +};
> > +
> > +struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm, u32 flags)
> > +{
> > +       struct fuse_compound_req *compound;
> > +
> > +       compound = kzalloc(sizeof(*compound), GFP_KERNEL);
> > +       if (!compound)
> > +               return ERR_PTR(-ENOMEM);
> 
> imo this logic is cleaner with just returning NULL here and then on
> the caller side doing
> 
>     compound = fuse_compound_alloc(fm, 0);
>     if (!compound)
>           return -ENOMEM;
> 
> instead of having to do
> 
>     if (IS_ERR(compound))
>            return PTR_ERR(compound);
> 
> and dealing with all the err_ptr/ptr_err stuff

I agree, will be fixed in the next version

> 
> > +
> > +       compound->fm = fm;
> > +       compound->compound_header.flags = flags;
> > +
> > +       return compound;
> > +}
> > +
> > +void fuse_compound_free(struct fuse_compound_req *compound)
> > +{
> > +       if (!compound)
> > +               return;
> 
> The implementation of kfree (in mm/slub.c) already has a null check
> 

I hadn't thought about this while I did the simplifications for last veriosn but
then we actually don't need fuse_compound_free() at all.
We could take that out and just use kfree() for the compound, don't we?

> > +
> > +       kfree(compound);
> > +}
> > +
> > +int fuse_compound_add(struct fuse_compound_req *compound,
> > +                     struct fuse_args *args)
> > +{
> > +       if (!compound ||
> > +           compound->compound_header.count >= FUSE_MAX_COMPOUND_OPS)
> > +               return -EINVAL;
> > +
> > +       if (args->in_pages)
> > +               return -EINVAL;
> > +
> > +       compound->op_args[compound->compound_header.count] = args;
> > +       compound->compound_header.count++;
> > +       return 0;
> > +}
> > +
> > +static void *fuse_copy_response_data(struct fuse_args *args,
> > +                                    char *response_data)
> > +{
> > +       size_t copied = 0;
> > +       int i;
> > +
> > +       for (i = 0; i < args->out_numargs; i++) {
> > +               struct fuse_arg current_arg = args->out_args[i];
> 
> struct fuse_arg *current_arg = &args->out_args[i]; instead? that would
> avoid the extra stack copy
> 
> > +               size_t arg_size;
> > +
> > +               /*
> > +                * Last argument with out_pages: copy to pages
> > +                * External payload (in the last out arg) is not supported
> > +                * at the moment
> > +                */
> > +               if (i == args->out_numargs - 1 && args->out_pages)
> > +                       return response_data;
> 
> imo this should be detected and error-ed out at the
> fuse_compound_send() layer or is there some reason we can't do that?

I have already taken this part out for the next version, since we don't have in_pages and out_pages at the moment anyway. Those can be added later when we actually use them.

> 
> > +
> > +               arg_size = current_arg.size;
> > +
> > +               if (current_arg.value && arg_size > 0) {
> 
> If this is part of the args->out_numargs tally, then I think it's
> guaranteed here that arg->value is non-NULL and arg->size > 0 (iirc,
> the only exception to this is if out_pages is set, then arg->value is
> null, but that's already protected against above).
> 
> > +                       memcpy(current_arg.value,
> > +                              (char *)response_data + copied, arg_size);
> 
> Instead of doing "response_data + copied" arithmetic and needing the
> copied variable, what about just updating the ptr as we go? i think
> that'd look cleaner
> 
> > +                       copied += arg_size;
> > +               }
> > +       }
> > +
> > +       return (char *)response_data + copied;
> 
> is this cast needed? afaict, response_data is already a char *?
> 
> > +}
> > +
> > +int fuse_compound_get_error(struct fuse_compound_req *compound, int op_idx)
> > +{
> > +       return compound->op_errors[op_idx];
> > +}
> 
> Hmm, looking at how this gets used by fuse_compound_open_getattr(), it
> seems like a more useful api is one that scans through op_errors for
> all the op indexes in the compound and returns back the first error it
> encounters; then the caller doesn't need to call
> fuse_compound_get_error() for every op index in the compound.

I think wee need both.
Imagine LOOKUP+CREATE you want to know if just the first had an error and the second succeeded but sometimes you just want to know if there was an error like in OPEN+GETATTR.

> 
> > +
> > +static void *fuse_compound_parse_one_op(struct fuse_compound_req *compound,
> > +                                       int op_index, void *op_out_data,
> > +                                       void *response_end)
> > +{
> > +       struct fuse_out_header *op_hdr = op_out_data;
> > +       struct fuse_args *args = compound->op_args[op_index];
> 
> op_index is taken straight from "compound->result_header.count" which
> can be any value set by the server. I think we need to either add
> checking for op_index value here or add checking for that in
> fuse_compound_send() before it calls fuse_compound_parse_resp()
> 
> > +
> > +       if (op_hdr->len < sizeof(struct fuse_out_header))
> > +               return NULL;
> > +
> > +       /* Check if the entire operation response fits in the buffer */
> > +       if ((char *)op_out_data + op_hdr->len > (char *)response_end)
> 
> Is there a reason this doesn't just define response_end as a char * in
> fuse_compound_parse_resp() and pass that as a char * to this function?

I can't think of one at the moment.
I rewrote that part multiple times, so there is some inconsistency.

> 
> > +               return NULL;
> > +
> > +       if (op_hdr->error != 0)
> 
> nit: this could just be "if (op_hdr->error)"
> 
> > +               compound->op_errors[op_index] = op_hdr->error;
> 
> If this errors out, is the memcpy still needed? or should this just return NULL?
> 
> > +
> > +       if (args && op_hdr->len > sizeof(struct fuse_out_header))
> 
> imo,  args should be checked right after the "struct fuse_args *args =
> compound->op_args[op_index];" line in the beginning and if it's null,
> t hen fuse_compound_parse_one_op() should return NULL.
> 
> > +               return fuse_copy_response_data(args, op_out_data +
> > +                                              sizeof(struct fuse_out_header));
> > +
> > +       /* No response data, just advance past the header */
> > +       return (char *)op_out_data + op_hdr->len;
> > +}
> > +
> > +static int fuse_compound_parse_resp(struct fuse_compound_req *compound,
> > +                                   u32 count, void *response,
> > +                                   size_t response_size)
> > +{
> > +       void *op_out_data = response;
> 
> imo it's cleaner to just use response instead of defining a new
> op_out_data variable. And if we change the response arg to char *
> response instead of void * response, then that gets rid of a lot of
> the char * casting.
> 
> > +       void *response_end = (char *)response + response_size;
> > +       int i;
> > +
> > +       if (!response || response_size < sizeof(struct fuse_out_header))
> > +               return -EIO;
> > +
> > +       for (i = 0; i < count && i < compound->result_header.count; i++) {
> 
> count is already compound->result_header.count or am I missing something here?
> 
> > +               op_out_data = fuse_compound_parse_one_op(compound, i,
> > +                                                        op_out_data,
> > +                                                        response_end);
> > +               if (!op_out_data)
> > +                       return -EIO;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +ssize_t fuse_compound_send(struct fuse_compound_req *compound)
> > +{
> > +       struct fuse_args args = {
> > +               .opcode = FUSE_COMPOUND,
> > +               .nodeid = 0,
> 
> nit: this line can be removed
> 
> > +               .in_numargs = 2,
> > +               .out_numargs = 2,
> > +               .out_argvar = true,
> > +       };
> > +       size_t resp_buffer_size;
> > +       size_t actual_response_size;
> > +       size_t buffer_pos;
> > +       size_t total_expected_out_size;
> > +       void *buffer = NULL;
> > +       void *resp_payload;
> > +       ssize_t ret;
> > +       int i;
> > +
> > +       if (!compound) {
> > +               pr_info_ratelimited("FUSE: compound request is NULL in %s\n",
> > +                                   __func__);
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (compound->compound_header.count == 0) {
> > +               pr_info_ratelimited("FUSE: compound request contains no operations\n");
> > +               return -EINVAL;
> > +       }
> 
> imo we can get rid of these two checks
I actually put them in to see when a fuse server misbehaves but this can easily be removed, we would just lose the information that we probably got a nulled out result.
> 
> > +
> > +       buffer_pos = 0;
> > +       total_expected_out_size = 0;
> 
> Could you move this initialization to the top where the variables get defined?
> 
> > +
> > +       for (i = 0; i < compound->compound_header.count; i++) {
> 
> compound->compound_header.count gets used several times in this
> function. i think it's worth declaring this as a separate (and less
> verbose :)) variable
> 
> > +               struct fuse_args *op_args = compound->op_args[i];
> > +               size_t needed_size = sizeof(struct fuse_in_header);
> > +               int j;
> > +
> > +               for (j = 0; j < op_args->in_numargs; j++)
> > +                       needed_size += op_args->in_args[j].size;
> > +
> > +               buffer_pos += needed_size;
> > +
> > +               for (j = 0; j < op_args->out_numargs; j++)
> > +                       total_expected_out_size += op_args->out_args[j].size;
> > +       }
> > +
> > +       buffer = kvmalloc(buffer_pos, GFP_KERNEL);
> 
> nit: imo it's cleaner to rename resp_buffer_size to buffer_size and
> use that variable for this instead of using buffer_pos
> 
> imo I don't think we need kvmalloc here or below for the resp buffer
> given that compound requests don't support in_pages or out_pages.

yes, this is a victim of my try to do the in/out_pages part as well and then abandoning it for the timebeing.

> 
> > +       if (!buffer)
> > +               return -ENOMEM;
> > +
> > +       buffer_pos = 0;
> > +       for (i = 0; i < compound->compound_header.count; i++) {
> > +               struct fuse_args *op_args = compound->op_args[i];
> > +               struct fuse_in_header *hdr;
> > +               size_t needed_size = sizeof(struct fuse_in_header);
> > +               int j;
> > +
> > +               for (j = 0; j < op_args->in_numargs; j++)
> > +                       needed_size += op_args->in_args[j].size;
> 
> don't we already have the op_args->in_args[] needed size from the
> computation abvoe? could we just reuse that?
> 
> > +
> > +               hdr = (struct fuse_in_header *)(buffer + buffer_pos);
> > +               memset(hdr, 0, sizeof(*hdr));
> > +               hdr->len = needed_size;
> > +               hdr->opcode = op_args->opcode;
> > +               hdr->nodeid = op_args->nodeid;
> > +               hdr->uid = from_kuid(compound->fm->fc->user_ns,
> > +                                    current_fsuid());
> > +               hdr->gid = from_kgid(compound->fm->fc->user_ns,
> > +                                    current_fsgid());
> > +               hdr->pid = pid_nr_ns(task_pid(current),
> > +                                    compound->fm->fc->pid_ns);
> 
> at this point i think it's worth just defining fc at the top of the
> function and using that here
> 
> > +               buffer_pos += sizeof(*hdr);
> > +
> > +               for (j = 0; j < op_args->in_numargs; j++) {
> > +                       memcpy(buffer + buffer_pos, op_args->in_args[j].value,
> > +                              op_args->in_args[j].size);
> > +                       buffer_pos += op_args->in_args[j].size;
> > +               }
> > +       }
> 
> imo this would look nicer as a separate helper function
> 
> > +
> > +       resp_buffer_size = total_expected_out_size +
> > +                          (compound->compound_header.count *
> > +                           sizeof(struct fuse_out_header));
> > +
> > +       resp_payload = kvmalloc(resp_buffer_size, GFP_KERNEL | __GFP_ZERO);
> 
> kvzalloc()?

totally agree

> 
> > +       if (!resp_payload) {
> > +               ret = -ENOMEM;
> > +               goto out_free_buffer;
> > +       }
> > +
> > +       compound->compound_header.result_size = total_expected_out_size;
> > +
> > +       args.in_args[0].size = sizeof(compound->compound_header);
> > +       args.in_args[0].value = &compound->compound_header;
> > +       args.in_args[1].size = buffer_pos;
> > +       args.in_args[1].value = buffer;
> > +
> > +       args.out_args[0].size = sizeof(compound->result_header);
> > +       args.out_args[0].value = &compound->result_header;
> > +       args.out_args[1].size = resp_buffer_size;
> > +       args.out_args[1].value = resp_payload;
> > +
> > +       ret = fuse_simple_request(compound->fm, &args);
> > +       if (ret < 0)
> > +               goto out;
> > +
> > +       actual_response_size = args.out_args[1].size;
> 
> since out_argvar was set for the FUSE_COMPOUND request args, ret is
> already args.out_args[1].size (see  __fuse_simple_request())

yes, thanks for the hint ...

> 
> > +
> > +       if (actual_response_size < sizeof(struct fuse_compound_out)) {
> 
> can you explain why this checks against size of struct
> fuse_compound_out? afaict from the logic in
> fuse_compound_parse_resp(), actual_response_size doesn't include the
> size of the compound_out struct?

This in hindsight looks like a messed up idea to protect against the first calculations in fuse_compound_parse_resp() but that wouldn't work.
I will recheck that and correct accordingly.

> 
> Thanks,
> Joanne
> 
> 
> > +               pr_info_ratelimited("FUSE: compound response too small (%zu bytes, minimum %zu bytes)\n",
> > +                                   actual_response_size,
> > +                                   sizeof(struct fuse_compound_out));
> > +               ret = -EINVAL;
> > +               goto out;
> > +       }
> > +
> 

Thanks a lot for the review, I appreciate it.

Horst

Re: Re: [PATCH v4 2/3] fuse: create helper functions for filling in fuse args for open and getattr

[Horst Birthelmer]

On Wed, Jan 14, 2026 at 06:37:48PM -0800, Joanne Koong wrote:
> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >
> > From: Horst Birthelmer <hbirthelmer@ddn.com>
> >
> > create fuse_getattr_args_fill() and fuse_open_args_fill() to fill in
> > the parameters for the open and getattr calls.
> >
> > This is in preparation for implementing open+getattr and does not
> > represent any functional change.
> >
> > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > ---
> >  fs/fuse/dir.c    |  9 +--------
> >  fs/fuse/file.c   | 42 ++++++++++++++++++++++++++++++++++--------
> >  fs/fuse/fuse_i.h |  8 ++++++++
> >  3 files changed, 43 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > index 4b6b3d2758ff..ca8b69282c60 100644
> > --- a/fs/fuse/dir.c
> > +++ b/fs/fuse/dir.c
> > @@ -1493,14 +1493,7 @@ static int fuse_do_getattr(struct mnt_idmap *idmap, struct inode *inode,
> >                 inarg.getattr_flags |= FUSE_GETATTR_FH;
> >                 inarg.fh = ff->fh;
> >         }
> > -       args.opcode = FUSE_GETATTR;
> > -       args.nodeid = get_node_id(inode);
> > -       args.in_numargs = 1;
> > -       args.in_args[0].size = sizeof(inarg);
> > -       args.in_args[0].value = &inarg;
> > -       args.out_numargs = 1;
> > -       args.out_args[0].size = sizeof(outarg);
> > -       args.out_args[0].value = &outarg;
> > +       fuse_getattr_args_fill(&args, get_node_id(inode), &inarg, &outarg);
> >         err = fuse_simple_request(fm, &args);
> >         if (!err) {
> >                 if (fuse_invalid_attr(&outarg.attr) ||
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 01bc894e9c2b..53744559455d 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -23,6 +23,39 @@
> >  #include <linux/task_io_accounting_ops.h>
> >  #include <linux/iomap.h>
> >
> > +/*
> > + * Helper function to initialize fuse_args for OPEN/OPENDIR operations
> > + */
> > +void fuse_open_args_fill(struct fuse_args *args, u64 nodeid, int opcode,
> > +                        struct fuse_open_in *inarg, struct fuse_open_out *outarg)
> > +{
> > +       args->opcode = opcode;
> > +       args->nodeid = nodeid;
> > +       args->in_numargs = 1;
> > +       args->in_args[0].size = sizeof(*inarg);
> > +       args->in_args[0].value = inarg;
> > +       args->out_numargs = 1;
> > +       args->out_args[0].size = sizeof(*outarg);
> > +       args->out_args[0].value = outarg;
> > +}
> > +
> > +/*
> > + * Helper function to initialize fuse_args for GETATTR operations
> > + */
> > +void fuse_getattr_args_fill(struct fuse_args *args, u64 nodeid,
> > +                            struct fuse_getattr_in *inarg,
> > +                            struct fuse_attr_out *outarg)
> > +{
> > +       args->opcode = FUSE_GETATTR;
> > +       args->nodeid = nodeid;
> > +       args->in_numargs = 1;
> > +       args->in_args[0].size = sizeof(*inarg);
> > +       args->in_args[0].value = inarg;
> > +       args->out_numargs = 1;
> > +       args->out_args[0].size = sizeof(*outarg);
> > +       args->out_args[0].value = outarg;
> > +}
> 
> sorry to be so nitpicky but I think we should move this to the
> fuse/dir.c file since that's where the main fuse_do_getattr() function
> lives

That is true. I kept it here since this is the 'second home' and the patch was smaller but that's an easy fix.

> 
> > +
> >  static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
> >                           unsigned int open_flags, int opcode,
> >                           struct fuse_open_out *outargp)
> > @@ -40,14 +73,7 @@ static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
> >                 inarg.open_flags |= FUSE_OPEN_KILL_SUIDGID;
> >         }
> >
> > -       args.opcode = opcode;
> > -       args.nodeid = nodeid;
> > -       args.in_numargs = 1;
> > -       args.in_args[0].size = sizeof(inarg);
> > -       args.in_args[0].value = &inarg;
> > -       args.out_numargs = 1;
> > -       args.out_args[0].size = sizeof(*outargp);
> > -       args.out_args[0].value = outargp;
> > +       fuse_open_args_fill(&args, nodeid, opcode, &inarg, outargp);
> >
> >         return fuse_simple_request(fm, &args);
> >  }
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index 6dddbe2b027b..98ea41f76623 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -1179,6 +1179,14 @@ struct fuse_io_args {
> >  void fuse_read_args_fill(struct fuse_io_args *ia, struct file *file, loff_t pos,
> >                          size_t count, int opcode);
> >
> > +/*
> > + * Helper functions to initialize fuse_args for common operations
> > + */
> > +void fuse_open_args_fill(struct fuse_args *args, u64 nodeid, int opcode,
> > +                        struct fuse_open_in *inarg, struct fuse_open_out *outarg);
> 
> I don't think we need this for fuse_open_args_fill() here since it'll
> be used only in the scope of fuse/file.c
> 

That is true. This was a premature optimization on my part since I have a strong suspicion that we will be using more open calls in other compounds.
I agree, let's keep it clean for the moment.

> Thanks,
> Joanne
> 
> > +void fuse_getattr_args_fill(struct fuse_args *args, u64 nodeid,
> > +                           struct fuse_getattr_in *inarg,
> > +                           struct fuse_attr_out *outarg);
> >
> >  struct fuse_file *fuse_file_alloc(struct fuse_mount *fm, bool release);
> >  void fuse_file_free(struct fuse_file *ff);
> >
> > --
> > 2.51.0
> >

Thanks,
Horst

Re: Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr

[Horst Birthelmer]

On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >
> > From: Horst Birthelmer <hbirthelmer@ddn.com>
> >
> > The discussion about compound commands in fuse was
> > started over an argument to add a new operation that
> > will open a file and return its attributes in the same operation.
> >
> > Here is a demonstration of that use case with compound commands.
> >
> > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > ---
> >  fs/fuse/file.c   | 110 +++++++++++++++++++++++++++++++++++++++++++++++--------
> >  fs/fuse/fuse_i.h |   7 +++-
> >  fs/fuse/inode.c  |   6 +++
> >  fs/fuse/ioctl.c  |   2 +-
> >  4 files changed, 107 insertions(+), 18 deletions(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 53744559455d..c0375b32967d 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -152,8 +152,66 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
> >         }
> >  }
> >
...
> > +
> > +       err = fuse_compound_get_error(compound, 0);
> > +       if (err)
> > +               goto out;
> > +
> > +       err = fuse_compound_get_error(compound, 1);
> > +       if (err)
> > +               goto out;
> 
> Hmm, if the open succeeds but the getattr fails, why not process it
> kernel-side as a success for the open? Especially since on the server
> side, libfuse will disassemble the compound request into separate
> ones, so the server has no idea the open is even part of a compound.
> 
For this specific one (open+getattr) you are completely right. That makes total sense.
But there is the possibility in libfuse and a fuse server to process the compound as such. Especially when it has the (not defined in this patch) 'atomic' flag set.
This actually makes me think that there are multiple error handling scenarios.
1. The one we have here. Have errors for every compound request
2. it failed as a compound but no errors for the request (I have to think of a good example for this ;-) )
3. is not supported with the conditions set (like some atomic flags)

That's why I said in the other comment that we probably need two or three different functions here.
This gets complicated fast ....

Let me come up with something in the next version and see what you and the people here think.

> I haven't looked at the rest of the patch yet but this caught my
> attention when i was looking at how fuse_compound_get_error() gets
> used.
> 
> Thanks,
> Joanne
> 

Thanks,
Horst

Re: Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr

[Horst Birthelmer]

On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >
> > +
> > +       err = fuse_compound_send(compound);
> > +       if (err)
> > +               goto out;
> > +
> > +       err = fuse_compound_get_error(compound, 0);
> > +       if (err)
> > +               goto out;
> > +
> > +       err = fuse_compound_get_error(compound, 1);
> > +       if (err)
> > +               goto out;
> 
> Hmm, if the open succeeds but the getattr fails, why not process it
> kernel-side as a success for the open? Especially since on the server
> side, libfuse will disassemble the compound request into separate
> ones, so the server has no idea the open is even part of a compound.
> 
> I haven't looked at the rest of the patch yet but this caught my
> attention when i was looking at how fuse_compound_get_error() gets
> used.
>
After looking at this again ...
Do you think it would make sense to add an example of lookup+create, or would that just convolute things?
 
> Thanks,
> Joanne
> 

Thanks,
Horst

Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr

[Bernd Schubert]

On 1/15/26 14:38, Horst Birthelmer wrote:
> On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
>> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>>>
>>> +
>>> +       err = fuse_compound_send(compound);
>>> +       if (err)
>>> +               goto out;
>>> +
>>> +       err = fuse_compound_get_error(compound, 0);
>>> +       if (err)
>>> +               goto out;
>>> +
>>> +       err = fuse_compound_get_error(compound, 1);
>>> +       if (err)
>>> +               goto out;
>>
>> Hmm, if the open succeeds but the getattr fails, why not process it
>> kernel-side as a success for the open? Especially since on the server
>> side, libfuse will disassemble the compound request into separate
>> ones, so the server has no idea the open is even part of a compound.
>>
>> I haven't looked at the rest of the patch yet but this caught my
>> attention when i was looking at how fuse_compound_get_error() gets
>> used.
>>
> After looking at this again ...
> Do you think it would make sense to add an example of lookup+create, or would that just convolute things?


I think that will be needed with the LOOKUP_HANDLE from Luis, if we go
the way Miklos proposes. To keep things simple, maybe not right now?


Thanks,
Bernd

Re: Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr

[Horst Birthelmer]

On Thu, Jan 15, 2026 at 02:41:49PM +0100, Bernd Schubert wrote:
> 
> 
> On 1/15/26 14:38, Horst Birthelmer wrote:
> > On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
> >> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >>>
> >>> +
> >>> +       err = fuse_compound_send(compound);
> >>> +       if (err)
> >>> +               goto out;
> >>> +
> >>> +       err = fuse_compound_get_error(compound, 0);
> >>> +       if (err)
> >>> +               goto out;
> >>> +
> >>> +       err = fuse_compound_get_error(compound, 1);
> >>> +       if (err)
> >>> +               goto out;
> >>
> >> Hmm, if the open succeeds but the getattr fails, why not process it
> >> kernel-side as a success for the open? Especially since on the server
> >> side, libfuse will disassemble the compound request into separate
> >> ones, so the server has no idea the open is even part of a compound.
> >>
> >> I haven't looked at the rest of the patch yet but this caught my
> >> attention when i was looking at how fuse_compound_get_error() gets
> >> used.
> >>
> > After looking at this again ...
> > Do you think it would make sense to add an example of lookup+create, or would that just convolute things?
> 
> 
> I think that will be needed with the LOOKUP_HANDLE from Luis, if we go
> the way Miklos proposes. To keep things simple, maybe not right now?

I was thinking more along the lines of ... we would have more than one example especially for the error handling. Otherwise it is easy to miss something because the given example just doesn't need that special case.
Like the case above. There we would be perfectly fine with a function returning the first error, which in the case of lookup+create is the opposite of success and you would need to access every single error to check what actually happened.

> 
> 
> Thanks,
> Bernd

Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr

[Luis Henriques]

On Thu, Jan 15 2026, Horst Birthelmer wrote:

> On Thu, Jan 15, 2026 at 02:41:49PM +0100, Bernd Schubert wrote:
>> 
>> 
>> On 1/15/26 14:38, Horst Birthelmer wrote:
>> > On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
>> >> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>> >>>
>> >>> +
>> >>> +       err = fuse_compound_send(compound);
>> >>> +       if (err)
>> >>> +               goto out;
>> >>> +
>> >>> +       err = fuse_compound_get_error(compound, 0);
>> >>> +       if (err)
>> >>> +               goto out;
>> >>> +
>> >>> +       err = fuse_compound_get_error(compound, 1);
>> >>> +       if (err)
>> >>> +               goto out;
>> >>
>> >> Hmm, if the open succeeds but the getattr fails, why not process it
>> >> kernel-side as a success for the open? Especially since on the server
>> >> side, libfuse will disassemble the compound request into separate
>> >> ones, so the server has no idea the open is even part of a compound.
>> >>
>> >> I haven't looked at the rest of the patch yet but this caught my
>> >> attention when i was looking at how fuse_compound_get_error() gets
>> >> used.
>> >>
>> > After looking at this again ...
>> > Do you think it would make sense to add an example of lookup+create, or would that just convolute things?
>> 
>> 
>> I think that will be needed with the LOOKUP_HANDLE from Luis, if we go
>> the way Miklos proposes. To keep things simple, maybe not right now?
>
> I was thinking more along the lines of ... we would have more than one example
> especially for the error handling. Otherwise it is easy to miss something
> because the given example just doesn't need that special case.
> Like the case above. There we would be perfectly fine with a function returning
> the first error, which in the case of lookup+create is the opposite of success
> and you would need to access every single error to check what actually happened.

Not sure if I can add a lot to this discussion, but I've been playing a
bit with your patchset.

I was trying to understand how to implement the LOOKUP_HANDLE+STATX, and
it doesn't look too difficult at the moment.  But I guess it'll take me some
more time to figure out all the other unknowns (e.g. other operations such
as readdirplus).

Anyway, the interface for compound operations seem to be quite usable in
general.  I'll try to do a proper review soon, but regarding the specific
comment of error handling, I find the interface a bit clumsy.  Have you
thought about using something like an iterator?  Or maybe some sort of
macro such as foreach_compound_error()?

And regarding the error handling in general: it sounds like things can
become really complex when some operations within a compound operation may
succeed and others fail.  Because these examples are using two operations
only, but there's nothing preventing us from having 3 or more in the
future, right?  Wouldn't it be easier to have the compound operation
itself fail or succeed, instead of each op?  (Although that would probably
simply move the complexity into user-space, that would be required to do
more clean-up work when there are failures.)

Cheers,
-- 
Luís

Re: Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr

[Horst Birthelmer]

Hi Luis,

thanks for looking at this.

On Thu, Jan 15, 2026 at 03:11:25PM +0000, Luis Henriques wrote:
> On Thu, Jan 15 2026, Horst Birthelmer wrote:
> 
> > On Thu, Jan 15, 2026 at 02:41:49PM +0100, Bernd Schubert wrote:
> >> 
> >> 
> >> On 1/15/26 14:38, Horst Birthelmer wrote:
> >> > On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
> >> >> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >> >>>
> >> >>> +
> >> >>> +       err = fuse_compound_send(compound);
> >> >>> +       if (err)
> >> >>> +               goto out;
> >> >>> +
> >> >>> +       err = fuse_compound_get_error(compound, 0);
> >> >>> +       if (err)
> >> >>> +               goto out;
> >> >>> +
> >> >>> +       err = fuse_compound_get_error(compound, 1);
> >> >>> +       if (err)
> >> >>> +               goto out;
> >> >>
> >> >> Hmm, if the open succeeds but the getattr fails, why not process it
> >> >> kernel-side as a success for the open? Especially since on the server
> >> >> side, libfuse will disassemble the compound request into separate
> >> >> ones, so the server has no idea the open is even part of a compound.
> >> >>
> >> >> I haven't looked at the rest of the patch yet but this caught my
> >> >> attention when i was looking at how fuse_compound_get_error() gets
> >> >> used.
> >> >>
> >> > After looking at this again ...
> >> > Do you think it would make sense to add an example of lookup+create, or would that just convolute things?
> >> 
> >> 
> >> I think that will be needed with the LOOKUP_HANDLE from Luis, if we go
> >> the way Miklos proposes. To keep things simple, maybe not right now?
> >
> > I was thinking more along the lines of ... we would have more than one example
> > especially for the error handling. Otherwise it is easy to miss something
> > because the given example just doesn't need that special case.
> > Like the case above. There we would be perfectly fine with a function returning
> > the first error, which in the case of lookup+create is the opposite of success
> > and you would need to access every single error to check what actually happened.
> 
> Not sure if I can add a lot to this discussion, but I've been playing a
> bit with your patchset.
You already do ;-)

> 
> I was trying to understand how to implement the LOOKUP_HANDLE+STATX, and
> it doesn't look too difficult at the moment.  But I guess it'll take me some
> more time to figure out all the other unknowns (e.g. other operations such
> as readdirplus).
> 
> Anyway, the interface for compound operations seem to be quite usable in
> general.  I'll try to do a proper review soon, but regarding the specific
> comment of error handling, I find the interface a bit clumsy.  Have you
> thought about using something like an iterator?  Or maybe some sort of
> macro such as foreach_compound_error()?
Not in those terms, no.
But I don't think it would get any better. Do you have an idea you would want implemented in this context?

> 
> And regarding the error handling in general: it sounds like things can
> become really complex when some operations within a compound operation may
> succeed and others fail.  Because these examples are using two operations
> only, but there's nothing preventing us from having 3 or more in the
> future, right?  Wouldn't it be easier to have the compound operation
> itself fail or succeed, instead of each op?  (Although that would probably
> simply move the complexity into user-space, that would be required to do
> more clean-up work when there are failures.)

I think we need all the granularity we can get since different combinations mean different things in different contexts.
Imagine you have a compound as the current example. That is pretty much all or nothing and there is almost no way that one of the operations doesn't succeed, and if it goes wrong you can still fall back to separate operations.
There are certainly cases where the compound itself is the operation because it really has to be atomic, or for arguments sake they have to be started concurrently ... or whatnot.


> 
> Cheers,
> -- 
> Luís

Thanks,
Horst

Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr

[Luis Henriques]

Hi Horst,

On Thu, Jan 15 2026, Horst Birthelmer wrote:

> Hi Luis,
>
> thanks for looking at this.
>
> On Thu, Jan 15, 2026 at 03:11:25PM +0000, Luis Henriques wrote:
>> On Thu, Jan 15 2026, Horst Birthelmer wrote:
>> 
>> > On Thu, Jan 15, 2026 at 02:41:49PM +0100, Bernd Schubert wrote:
>> >> 
>> >> 
>> >> On 1/15/26 14:38, Horst Birthelmer wrote:
>> >> > On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
>> >> >> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>> >> >>>
>> >> >>> +
>> >> >>> +       err = fuse_compound_send(compound);
>> >> >>> +       if (err)
>> >> >>> +               goto out;
>> >> >>> +
>> >> >>> +       err = fuse_compound_get_error(compound, 0);
>> >> >>> +       if (err)
>> >> >>> +               goto out;
>> >> >>> +
>> >> >>> +       err = fuse_compound_get_error(compound, 1);
>> >> >>> +       if (err)
>> >> >>> +               goto out;
>> >> >>
>> >> >> Hmm, if the open succeeds but the getattr fails, why not process it
>> >> >> kernel-side as a success for the open? Especially since on the server
>> >> >> side, libfuse will disassemble the compound request into separate
>> >> >> ones, so the server has no idea the open is even part of a compound.
>> >> >>
>> >> >> I haven't looked at the rest of the patch yet but this caught my
>> >> >> attention when i was looking at how fuse_compound_get_error() gets
>> >> >> used.
>> >> >>
>> >> > After looking at this again ...
>> >> > Do you think it would make sense to add an example of lookup+create, or would that just convolute things?
>> >> 
>> >> 
>> >> I think that will be needed with the LOOKUP_HANDLE from Luis, if we go
>> >> the way Miklos proposes. To keep things simple, maybe not right now?
>> >
>> > I was thinking more along the lines of ... we would have more than one example
>> > especially for the error handling. Otherwise it is easy to miss something
>> > because the given example just doesn't need that special case.
>> > Like the case above. There we would be perfectly fine with a function returning
>> > the first error, which in the case of lookup+create is the opposite of success
>> > and you would need to access every single error to check what actually happened.
>> 
>> Not sure if I can add a lot to this discussion, but I've been playing a
>> bit with your patchset.
> You already do ;-)
>
>> 
>> I was trying to understand how to implement the LOOKUP_HANDLE+STATX, and
>> it doesn't look too difficult at the moment.  But I guess it'll take me some
>> more time to figure out all the other unknowns (e.g. other operations such
>> as readdirplus).
>> 
>> Anyway, the interface for compound operations seem to be quite usable in
>> general.  I'll try to do a proper review soon, but regarding the specific
>> comment of error handling, I find the interface a bit clumsy.  Have you
>> thought about using something like an iterator?  Or maybe some sort of
>> macro such as foreach_compound_error()?
> Not in those terms, no.
> But I don't think it would get any better. Do you have an idea you would want implemented in this context?

Yeah, I'm not really sure it would be any better, to be honest.  And I'm
afraid I'm just bikeshedding...  My suggestion was to have something like:

	err = fuse_compound_send(compound);
	if (err)
		goto out;

	for_each_compound_error(op_arg, compound, list) {
		err = handle_error(op_arg);
		if (err)
			goto out;
	}

But again, this is probably unnecessary.  Or at least not while we're
talking about having compound requests with 2 operations only.

>> And regarding the error handling in general: it sounds like things can
>> become really complex when some operations within a compound operation may
>> succeed and others fail.  Because these examples are using two operations
>> only, but there's nothing preventing us from having 3 or more in the
>> future, right?  Wouldn't it be easier to have the compound operation
>> itself fail or succeed, instead of each op?  (Although that would probably
>> simply move the complexity into user-space, that would be required to do
>> more clean-up work when there are failures.)
>
> I think we need all the granularity we can get since different combinations mean different things in different contexts.
> Imagine you have a compound as the current example. That is pretty much all or
> nothing and there is almost no way that one of the operations doesn't succeed,
> and if it goes wrong you can still fall back to separate operations.
> There are certainly cases where the compound itself is the operation because it
> really has to be atomic, or for arguments sake they have to be started
> concurrently ... or whatnot.

OK, got it.  I guess that errors on atomic compound operations will then
be handled that way (i.e. all or nothing), and it will always depend on
the combination of operations.  Anyway, the devil is always in the details
and I'm sure you've been putting a lot of thought into this ;-)

Cheers,
-- 
Luís