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

[Luis Henriques <luis@igalia.com>]

Hi Horst,

On Tue, Feb 10 2026, Horst Birthelmer 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()

First of all, thanks a lot for this.  It looks solid to me and in general,
I'm happy with this interface.  (Note: I haven't yet tested this v5 rev --
I'll rebase my own code on top of it soon so that I can give it a try).

Please find below a few comments I have, most of them minor.

As for the two other patches in this series, they look mostly alright to
me, though I believe there's some extra clean-up work to be done in the
error paths for the actual open+getattr implementation.  Is this
open+getattr code something you'd like to get merged as well, or is it
just an example on how to implement a compound operation?  If the latter,
maybe it could be moved somewhere else (Documentation/filesystems/fuse/
maybe?).

> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> ---
>  fs/fuse/Makefile          |   2 +-
>  fs/fuse/compound.c        | 224 ++++++++++++++++++++++++++++++++++++++++++++++
>  fs/fuse/fuse_i.h          |  11 +++
>  include/uapi/linux/fuse.h |  40 +++++++++
>  4 files changed, 276 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fuse/Makefile b/fs/fuse/Makefile
> index 22ad9538dfc4b80c6d9b52235bdfead6a6567ae4..4c09038ef995d1b9133c2b6871b97b280a4693b0 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 0000000000000000000000000000000000000000..1a85209f4e99a0e5e54955bcd951eaf395176c12
> --- /dev/null
> +++ b/fs/fuse/compound.c
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * FUSE: Filesystem in Userspace
> + * Copyright (C) 2025

Should this be 2026?  Or 2025-2026, not sure.

> + *
> + * Compound operations for FUSE - batch multiple operations into a single
> + * request to reduce round trips between kernel and userspace.
> + */
> +
> +#include "fuse_i.h"
> +
> +/*
> + * Compound request builder, 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;
> +	int op_errors[FUSE_MAX_COMPOUND_OPS];
> +	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 NULL;
> +
> +	compound->fm = fm;
> +	compound->compound_header.flags = flags;
> +
> +	return 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_resp_data_per_req(const struct fuse_args *args,
> +					  char *resp)
> +{
> +	const struct fuse_arg *arg;
> +	int i;
> +
> +	for (i = 0; i < args->out_numargs; i++) {
> +		arg = &args->out_args[i];
> +		memcpy(arg->value, resp, arg->size);
> +		resp += arg->size;
> +	}
> +
> +	return resp;

fuse_copy_resp_data_per_req() should probably be a void function, as it's
only caller doesn't seem to care about the return.

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

Since this isn't being used in this file, this function could probably be
moved into fuse_i.h as a static inline instead.

> +
> +static void *fuse_compound_parse_one_op(struct fuse_compound_req *compound,
> +					 int op_index, char *response,
> +					 char *response_end)
> +{
> +	struct fuse_out_header *op_hdr = (struct fuse_out_header *)response;
> +	struct fuse_args *args = compound->op_args[op_index];
> +
> +	if (op_hdr->len < sizeof(struct fuse_out_header))
> +		return NULL;
> +
> +	if (response + op_hdr->len > response_end)
> +		return NULL;
> +
> +	if (op_hdr->error)
> +		compound->op_errors[op_index] = op_hdr->error;
> +	else
> +		fuse_copy_resp_data_per_req(args, response +
> +					    sizeof(struct fuse_out_header));
> +	/* in case of error, we still need to advance to the next op */
> +	return response + op_hdr->len;
> +}
> +
> +static int fuse_compound_parse_resp(struct fuse_compound_req *compound,
> +				     char *response, size_t response_size)
> +{
> +	char *response_end = response + response_size;
> +	int req_count;
> +	int i;
> +
> +	req_count = min(compound->compound_header.count,
> +			compound->result_header.count);
> +
> +	for (i = 0; i < req_count; i++) {
> +		response = fuse_compound_parse_one_op(compound, i, response,
> +						      response_end);

So, we allow the possibility of having some requests without out args.
But there seems to be something wrong here, right?  What if the first
request is e.g. a FUSE_UNLINK, which doesn't have an out arg?  In the
function above we'll be copying the arg into the first request.  Or am I
misunderstanding the logic?

> +		if (!response)
> +			return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Build a single operation request in the buffer
> + *
> + * Returns the new buffer position after writing the operation.
> + */
> +static char *fuse_compound_build_one_op(struct fuse_conn *fc,
> +					 struct fuse_args *op_args,
> +					 char *buffer_pos)
> +{
> +	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_pos;
> +	memset(hdr, 0, sizeof(*hdr));

Nit: since the buffer has kmalloc'ed using '__GFP_ZERO', this memset()
could probably be dropped.

> +	hdr->len = needed_size;
> +	hdr->opcode = op_args->opcode;
> +	hdr->nodeid = op_args->nodeid;
> +	hdr->uid = from_kuid(fc->user_ns, current_fsuid());
> +	hdr->gid = from_kgid(fc->user_ns, current_fsgid());
> +	hdr->pid = pid_nr_ns(task_pid(current), fc->pid_ns);
> +	buffer_pos += sizeof(*hdr);
> +
> +	for (j = 0; j < op_args->in_numargs; j++) {
> +		memcpy(buffer_pos, op_args->in_args[j].value,
> +		       op_args->in_args[j].size);
> +		buffer_pos += op_args->in_args[j].size;
> +	}
> +
> +	return buffer_pos;
> +}
> +
> +ssize_t fuse_compound_send(struct fuse_compound_req *compound)
> +{
> +	struct fuse_conn *fc = compound->fm->fc;
> +	struct fuse_args args = {
> +		.opcode = FUSE_COMPOUND,
> +		.in_numargs = 2,
> +		.out_numargs = 2,
> +		.out_argvar = true,
> +	};
> +	unsigned int req_count = compound->compound_header.count;
> +	size_t total_expected_out_size = 0;
> +	size_t actual_response_size;
> +	size_t buffer_size = 0;
> +	void *resp_payload_buffer;
> +	char *buffer_pos;
> +	void *buffer = NULL;
> +	ssize_t ret;
> +	int i, j;
> +
> +	for (i = 0; i < req_count; i++) {
> +		struct fuse_args *op_args = compound->op_args[i];
> +		size_t needed_size = sizeof(struct fuse_in_header);
> +
> +		for (j = 0; j < op_args->in_numargs; j++)
> +			needed_size += op_args->in_args[j].size;
> +
> +		buffer_size += needed_size;
> +
> +		for (j = 0; j < op_args->out_numargs; j++)
> +			total_expected_out_size += op_args->out_args[j].size;
> +	}
> +
> +	buffer = kmalloc(buffer_size, GFP_KERNEL | __GFP_ZERO);

Nit: I find kzalloc() easier to read (and write!).

> +	if (!buffer)
> +		return -ENOMEM;
> +
> +	buffer_pos = buffer;
> +	for (i = 0; i < req_count; i++)
> +		buffer_pos = fuse_compound_build_one_op(fc,
> +							compound->op_args[i],
> +							buffer_pos);
> +
> +	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_size;
> +	args.in_args[1].value = buffer;
> +
> +	buffer_size = total_expected_out_size +
> +		      (req_count * sizeof(struct fuse_out_header));
> +
> +	resp_payload_buffer = kmalloc(buffer_size, GFP_KERNEL | __GFP_ZERO);

Same as above.

> +	if (!resp_payload_buffer) {
> +		ret = -ENOMEM;
> +		goto out_free_buffer;
> +	}
> +
> +	args.out_args[0].size = sizeof(compound->result_header);
> +	args.out_args[0].value = &compound->result_header;
> +	args.out_args[1].size = buffer_size;
> +	args.out_args[1].value = resp_payload_buffer;
> +
> +	ret = fuse_simple_request(compound->fm, &args);
> +	if (ret < 0)
> +		goto out;
> +
> +	actual_response_size = args.out_args[1].size;
> +
> +	ret = fuse_compound_parse_resp(compound, (char *)resp_payload_buffer,
> +				       actual_response_size);
> +out:
> +	kfree(resp_payload_buffer);
> +out_free_buffer:
> +	kfree(buffer);
> +	return ret;
> +}
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 7f16049387d15e869db4be23a93605098588eda9..9ebcd96b6b309d75c86a9c716cbd88aaa55c57ef 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1273,6 +1273,17 @@ 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);
> +
>  /**
>   * Assign a unique id to a fuse request
>   */
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index c13e1f9a2f12bd39f535188cb5466688eba42263..113583c4efb67268174dbe4f68e9ea1c21b45eb6 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,39 @@ struct fuse_supp_groups {
>  	uint32_t	groups[];
>  };
>  
> +#define FUSE_MAX_COMPOUND_OPS   16        /* Maximum operations per compound */
> +
> +#define FUSE_COMPOUND_SEPARABLE (1<<0)
> +#define FUSE_COMPOUND_ATOMIC (1<<1)

Is there a difference between !ATOMIC and SEPARABLE?  I.e can we set both
flags for the same request?  If there is a difference, I think it should
be explicitly spelled out here.

Cheers,
-- 
Luís

> +/*
> + * 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.53.0
>