* [PATCH RFC v2 0/2] fuse: compound commands
@ 2025-12-23 22:13 Horst Birthelmer
2025-12-23 22:13 ` [PATCH RFC v2 1/2] fuse: add compound command to combine multiple requests Horst Birthelmer
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Horst Birthelmer @ 2025-12-23 22:13 UTC (permalink / raw)
To: Miklos Szeredi, Bernd Schubert
Cc: linux-kernel, linux-fsdevel, Horst Birthelmer, syzbot
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).
[1] https://lore.kernel.org/linux-fsdevel/CAJfpegshcrjXJ0USZ8RRdBy=e0MxmBTJSCE0xnxG8LXgXy-xuQ@mail.gmail.com/
Signed-off-by: Horst Birthelmer <hbirthelmer@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 (2):
fuse: add compound command to combine multiple requests
fuse: add an implementation of open+getattr
fs/fuse/Makefile | 2 +-
fs/fuse/compound.c | 368 ++++++++++++++++++++++++++++++++++++++++++++++
fs/fuse/dev.c | 25 ++++
fs/fuse/file.c | 125 ++++++++++++++--
fs/fuse/fuse_i.h | 20 ++-
fs/fuse/inode.c | 6 +
fs/fuse/ioctl.c | 2 +-
include/uapi/linux/fuse.h | 37 +++++
8 files changed, 566 insertions(+), 19 deletions(-)
---
base-commit: 9448598b22c50c8a5bb77a9103e2d49f134c9578
change-id: 20251223-fuse-compounds-upstream-c85b4e39b3d3
Best regards,
--
Horst Birthelmer <hbirthelmer@ddn.com>
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH RFC v2 1/2] fuse: add compound command to combine multiple requests 2025-12-23 22:13 [PATCH RFC v2 0/2] fuse: compound commands Horst Birthelmer @ 2025-12-23 22:13 ` Horst Birthelmer 2026-01-07 1:40 ` Joanne Koong 2025-12-23 22:13 ` [PATCH RFC v2 2/2] fuse: add an implementation of open+getattr Horst Birthelmer 2025-12-29 6:03 ` [PATCH RFC v2 0/2] fuse: compound commands Jingbo Xu 2 siblings, 1 reply; 14+ messages in thread From: Horst Birthelmer @ 2025-12-23 22:13 UTC (permalink / raw) To: Miklos Szeredi, Bernd Schubert Cc: linux-kernel, linux-fsdevel, Horst Birthelmer, syzbot 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_request() fuse_compound_free() Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com> Tested-by: syzbot@syzkaller.appspotmail.com --- fs/fuse/Makefile | 2 +- fs/fuse/compound.c | 368 ++++++++++++++++++++++++++++++++++++++++++++++ fs/fuse/dev.c | 25 ++++ fs/fuse/fuse_i.h | 14 ++ include/uapi/linux/fuse.h | 37 +++++ 5 files changed, 445 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..b15014d61b38 --- /dev/null +++ b/fs/fuse/compound.c @@ -0,0 +1,368 @@ +// 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 + * + * This structure manages the lifecycle of a compound FUSE request, from building + * the request by serializing multiple operations into a single buffer, through + * sending it to userspace, to parsing the compound response back into individual + * operation results. + */ +struct fuse_compound_req { + struct fuse_mount *fm; + struct fuse_compound_in compound_header; + struct fuse_compound_out result_header; + + size_t total_size; /* Total size of serialized operations */ + char *buffer; /* Buffer holding serialized requests */ + size_t buffer_pos; /* Current write position in buffer */ + size_t buffer_size; /* Total allocated buffer size */ + + size_t total_expected_out_size; /* Sum of expected output sizes */ + + /* 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]; + + bool parsed; /* Prevent double-parsing of response */ +}; + +struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm, + uint32_t 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; + compound->buffer_size = PAGE_SIZE; + compound->buffer = kvmalloc(compound->buffer_size, GFP_KERNEL); + if (!compound->buffer) { + kfree(compound); + return ERR_PTR(-ENOMEM); + } + return compound; +} + +void fuse_compound_free(struct fuse_compound_req *compound) +{ + if (!compound) + return; + + kvfree(compound->buffer); + kfree(compound); +} + +static int fuse_compound_validate_header(struct fuse_compound_req *compound) +{ + struct fuse_compound_in *in_header = &compound->compound_header; + size_t offset = 0; + int i; + + if (compound->buffer_pos > compound->buffer_size) + return -EINVAL; + + if (!compound || !compound->buffer) + return -EINVAL; + + if (compound->buffer_pos < sizeof(struct fuse_in_header)) + return -EINVAL; + + if (in_header->count == 0 || in_header->count > FUSE_MAX_COMPOUND_OPS) + return -EINVAL; + + for (i = 0; i < in_header->count; i++) { + const struct fuse_in_header *op_hdr; + + if (offset + sizeof(struct fuse_in_header) > + compound->buffer_pos) { + pr_info_ratelimited("FUSE: compound operation %d header extends beyond buffer (offset %zu + header size %zu > buffer pos %zu)\n", + i, offset, + sizeof(struct fuse_in_header), + compound->buffer_pos); + return -EINVAL; + } + + op_hdr = (const struct fuse_in_header *)(compound->buffer + + offset); + + if (op_hdr->len < sizeof(struct fuse_in_header)) { + pr_info_ratelimited("FUSE: compound operation %d has invalid length %u (minimum %zu bytes)\n", + i, op_hdr->len, + sizeof(struct fuse_in_header)); + return -EINVAL; + } + + if (offset + op_hdr->len > compound->buffer_pos) { + pr_info_ratelimited("FUSE: compound operation %d extends beyond buffer (offset %zu + length %u > buffer pos %zu)\n", + i, offset, op_hdr->len, + compound->buffer_pos); + return -EINVAL; + } + + if (op_hdr->opcode == 0 || op_hdr->opcode == FUSE_COMPOUND) { + pr_info_ratelimited("FUSE: compound operation %d has invalid opcode %u (cannot be 0 or FUSE_COMPOUND)\n", + i, op_hdr->opcode); + return -EINVAL; + } + + if (op_hdr->nodeid == 0) { + pr_info_ratelimited("FUSE: compound operation %d has invalid node ID 0\n", + i); + return -EINVAL; + } + + offset += op_hdr->len; + } + + if (offset != compound->buffer_pos) { + pr_info_ratelimited("FUSE: compound buffer size mismatch (calculated %zu bytes, actual %zu bytes)\n", + offset, compound->buffer_pos); + return -EINVAL; + } + + return 0; +} + +int fuse_compound_add(struct fuse_compound_req *compound, + struct fuse_args *args) +{ + struct fuse_in_header *hdr; + size_t args_size = 0; + size_t needed_size; + size_t expected_out_size = 0; + int i; + + if (!compound || + compound->compound_header.count >= FUSE_MAX_COMPOUND_OPS) + return -EINVAL; + + if (args->in_pages) + return -EINVAL; + + for (i = 0; i < args->in_numargs; i++) + args_size += args->in_args[i].size; + + for (i = 0; i < args->out_numargs; i++) + expected_out_size += args->out_args[i].size; + + needed_size = sizeof(struct fuse_in_header) + args_size; + + if (compound->buffer_pos + needed_size > compound->buffer_size) { + size_t new_size = max(compound->buffer_size * 2, + compound->buffer_pos + needed_size); + char *new_buffer; + + new_size = round_up(new_size, PAGE_SIZE); + new_buffer = kvrealloc(compound->buffer, new_size, + GFP_KERNEL); + if (!new_buffer) + return -ENOMEM; + compound->buffer = new_buffer; + compound->buffer_size = new_size; + } + + /* Build request header */ + hdr = (struct fuse_in_header *)(compound->buffer + + compound->buffer_pos); + memset(hdr, 0, sizeof(*hdr)); + hdr->len = needed_size; + hdr->opcode = args->opcode; + hdr->nodeid = 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); + hdr->unique = fuse_get_unique(&compound->fm->fc->iq); + compound->buffer_pos += sizeof(*hdr); + + for (i = 0; i < args->in_numargs; i++) { + memcpy(compound->buffer + compound->buffer_pos, + args->in_args[i].value, args->in_args[i].size); + compound->buffer_pos += args->in_args[i].size; + } + + compound->total_expected_out_size += expected_out_size; + + /* Store args for response parsing */ + compound->op_args[compound->compound_header.count] = args; + + compound->compound_header.count++; + compound->total_size += needed_size; + + return 0; +} + +static void *fuse_copy_response_data(struct fuse_args *args, + char *response_data) +{ + size_t copied = 0; + int arg_idx; + + for (arg_idx = 0; arg_idx < args->out_numargs; arg_idx++) { + struct fuse_arg current_arg = args->out_args[arg_idx]; + size_t arg_size; + + /* Last argument with out_pages: copy to pages */ + if (arg_idx == args->out_numargs - 1 && args->out_pages) { + /* + * External payload (in the last out arg) + * is not supported at the moment + */ + 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, + uint32_t count, void *response, + size_t response_size) +{ + void *op_out_data = response; + void *response_end = (char *)response + response_size; + int i; + + if (compound->parsed) + return 0; + + 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; + } + + compound->parsed = true; + 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 expected_response_size; + size_t total_buffer_size; + size_t actual_response_size; + void *resp_payload; + ssize_t ret; + + 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; + } + + expected_response_size = compound->total_expected_out_size; + total_buffer_size = expected_response_size + + (compound->compound_header.count * + sizeof(struct fuse_out_header)); + + resp_payload = kvmalloc(total_buffer_size, GFP_KERNEL | __GFP_ZERO); + if (!resp_payload) + return -ENOMEM; + + compound->compound_header.result_size = expected_response_size; + + args.in_args[0].size = sizeof(compound->compound_header); + args.in_args[0].value = &compound->compound_header; + args.in_args[1].size = compound->buffer_pos; + args.in_args[1].value = compound->buffer; + + args.out_args[0].size = sizeof(compound->result_header); + args.out_args[0].value = &compound->result_header; + args.out_args[1].size = total_buffer_size; + args.out_args[1].value = resp_payload; + + ret = fuse_compound_validate_header(compound); + if (ret) + goto out; + + ret = fuse_compound_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); + return ret; +} diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 6d59cbc877c6..2d89ca69308f 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -660,6 +660,31 @@ static void fuse_args_to_req(struct fuse_req *req, struct fuse_args *args) __set_bit(FR_ASYNC, &req->flags); } +ssize_t fuse_compound_request( + struct fuse_mount *fm, struct fuse_args *args) +{ + struct fuse_req *req; + ssize_t ret; + + req = fuse_get_req(&invalid_mnt_idmap, fm, false); + if (IS_ERR(req)) + return PTR_ERR(req); + + fuse_args_to_req(req, args); + + if (!args->noreply) + __set_bit(FR_ISREPLY, &req->flags); + + __fuse_request_send(req); + ret = req->out.h.error; + if (!ret && args->out_argvar) { + BUG_ON(args->out_numargs == 0); + ret = args->out_args[args->out_numargs - 1].size; + } + fuse_put_request(req); + return ret; +} + ssize_t __fuse_simple_request(struct mnt_idmap *idmap, struct fuse_mount *fm, struct fuse_args *args) diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 7f16049387d1..86253517f59b 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -1273,6 +1273,20 @@ 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_request(struct fuse_mount *fm, struct fuse_args *args); +ssize_t fuse_compound_send(struct fuse_compound_req *compound); + +struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm, uint32_t 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..848323acecdc 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 works like multiple simple requests. + * This is a special case for calls that can be combined atomic on the + * fuse server. If the server actually does atomically execute the command is + * left to the fuse server implementation. + */ + FUSE_COMPOUND = 101, + /* 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v2 1/2] fuse: add compound command to combine multiple requests 2025-12-23 22:13 ` [PATCH RFC v2 1/2] fuse: add compound command to combine multiple requests Horst Birthelmer @ 2026-01-07 1:40 ` Joanne Koong 2026-01-07 9:29 ` Horst Birthelmer 2026-01-07 14:03 ` Horst Birthelmer 0 siblings, 2 replies; 14+ messages in thread From: Joanne Koong @ 2026-01-07 1:40 UTC (permalink / raw) To: Horst Birthelmer Cc: Miklos Szeredi, Bernd Schubert, linux-kernel, linux-fsdevel, Horst Birthelmer, syzbot On Tue, Dec 23, 2025 at 2:13 PM Horst Birthelmer <hbirthelmer@googlemail.com> wrote: > > 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_request() > fuse_compound_free() > > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com> > Tested-by: syzbot@syzkaller.appspotmail.com > --- > fs/fuse/Makefile | 2 +- > fs/fuse/compound.c | 368 ++++++++++++++++++++++++++++++++++++++++++++++ > fs/fuse/dev.c | 25 ++++ > fs/fuse/fuse_i.h | 14 ++ > include/uapi/linux/fuse.h | 37 +++++ > 5 files changed, 445 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..b15014d61b38 > --- /dev/null > +++ b/fs/fuse/compound.c > @@ -0,0 +1,368 @@ > +// 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 > + * > + * This structure manages the lifecycle of a compound FUSE request, from building > + * the request by serializing multiple operations into a single buffer, through > + * sending it to userspace, to parsing the compound response back into individual > + * operation results. > + */ > +struct fuse_compound_req { > + struct fuse_mount *fm; > + struct fuse_compound_in compound_header; > + struct fuse_compound_out result_header; > + > + size_t total_size; /* Total size of serialized operations */ > + char *buffer; /* Buffer holding serialized requests */ > + size_t buffer_pos; /* Current write position in buffer */ > + size_t buffer_size; /* Total allocated buffer size */ > + > + size_t total_expected_out_size; /* Sum of expected output sizes */ > + > + /* 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]; > + > + bool parsed; /* Prevent double-parsing of response */ > +}; > + > +struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm, > + uint32_t 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; > + compound->buffer_size = PAGE_SIZE; > + compound->buffer = kvmalloc(compound->buffer_size, GFP_KERNEL); > + if (!compound->buffer) { > + kfree(compound); > + return ERR_PTR(-ENOMEM); > + } > + return compound; > +} > + > +void fuse_compound_free(struct fuse_compound_req *compound) > +{ > + if (!compound) > + return; > + > + kvfree(compound->buffer); > + kfree(compound); > +} > + > +static int fuse_compound_validate_header(struct fuse_compound_req *compound) > +{ > + struct fuse_compound_in *in_header = &compound->compound_header; > + size_t offset = 0; > + int i; > + > + if (compound->buffer_pos > compound->buffer_size) > + return -EINVAL; > + > + if (!compound || !compound->buffer) > + return -EINVAL; > + > + if (compound->buffer_pos < sizeof(struct fuse_in_header)) > + return -EINVAL; > + > + if (in_header->count == 0 || in_header->count > FUSE_MAX_COMPOUND_OPS) > + return -EINVAL; > + > + for (i = 0; i < in_header->count; i++) { > + const struct fuse_in_header *op_hdr; > + > + if (offset + sizeof(struct fuse_in_header) > > + compound->buffer_pos) { > + pr_info_ratelimited("FUSE: compound operation %d header extends beyond buffer (offset %zu + header size %zu > buffer pos %zu)\n", > + i, offset, > + sizeof(struct fuse_in_header), > + compound->buffer_pos); > + return -EINVAL; > + } > + > + op_hdr = (const struct fuse_in_header *)(compound->buffer + > + offset); > + > + if (op_hdr->len < sizeof(struct fuse_in_header)) { > + pr_info_ratelimited("FUSE: compound operation %d has invalid length %u (minimum %zu bytes)\n", > + i, op_hdr->len, > + sizeof(struct fuse_in_header)); > + return -EINVAL; > + } > + > + if (offset + op_hdr->len > compound->buffer_pos) { > + pr_info_ratelimited("FUSE: compound operation %d extends beyond buffer (offset %zu + length %u > buffer pos %zu)\n", > + i, offset, op_hdr->len, > + compound->buffer_pos); > + return -EINVAL; > + } > + > + if (op_hdr->opcode == 0 || op_hdr->opcode == FUSE_COMPOUND) { > + pr_info_ratelimited("FUSE: compound operation %d has invalid opcode %u (cannot be 0 or FUSE_COMPOUND)\n", > + i, op_hdr->opcode); > + return -EINVAL; > + } > + > + if (op_hdr->nodeid == 0) { > + pr_info_ratelimited("FUSE: compound operation %d has invalid node ID 0\n", > + i); > + return -EINVAL; > + } > + > + offset += op_hdr->len; > + } > + > + if (offset != compound->buffer_pos) { > + pr_info_ratelimited("FUSE: compound buffer size mismatch (calculated %zu bytes, actual %zu bytes)\n", > + offset, compound->buffer_pos); > + return -EINVAL; > + } > + > + return 0; I wonder if this is overkill to have given that the kernel is in charge of setting up the compound request and if the code is right, has done it correctly. imo it adds extra overhead but doesn't offer much given that the kernel code should aalready be correct. > +} > + > +int fuse_compound_add(struct fuse_compound_req *compound, > + struct fuse_args *args) > +{ > + struct fuse_in_header *hdr; > + size_t args_size = 0; > + size_t needed_size; > + size_t expected_out_size = 0; > + int i; > + > + if (!compound || > + compound->compound_header.count >= FUSE_MAX_COMPOUND_OPS) > + return -EINVAL; > + > + if (args->in_pages) > + return -EINVAL; > + > + for (i = 0; i < args->in_numargs; i++) > + args_size += args->in_args[i].size; > + > + for (i = 0; i < args->out_numargs; i++) > + expected_out_size += args->out_args[i].size; > + > + needed_size = sizeof(struct fuse_in_header) + args_size; > + > + if (compound->buffer_pos + needed_size > compound->buffer_size) { > + size_t new_size = max(compound->buffer_size * 2, > + compound->buffer_pos + needed_size); > + char *new_buffer; > + > + new_size = round_up(new_size, PAGE_SIZE); > + new_buffer = kvrealloc(compound->buffer, new_size, > + GFP_KERNEL); > + if (!new_buffer) > + return -ENOMEM; > + compound->buffer = new_buffer; > + compound->buffer_size = new_size; Hmm... when we're setting up a compound request, we already know the size that will be needed to hold all the requests, right? Do you think it makes sense to allocate that from the get-go in fuse_compound_alloc() and then not have to do any buffer reallocation? I think that also gets rid of fuse_compound_req->total_size, as that would just be the same as fuse_compound_req->buffer_size. > + } > + > + /* Build request header */ > + hdr = (struct fuse_in_header *)(compound->buffer + > + compound->buffer_pos); > + memset(hdr, 0, sizeof(*hdr)); > + hdr->len = needed_size; > + hdr->opcode = args->opcode; > + hdr->nodeid = 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); > + hdr->unique = fuse_get_unique(&compound->fm->fc->iq); > + compound->buffer_pos += sizeof(*hdr); > + > + for (i = 0; i < args->in_numargs; i++) { > + memcpy(compound->buffer + compound->buffer_pos, > + args->in_args[i].value, args->in_args[i].size); > + compound->buffer_pos += args->in_args[i].size; > + } > + > + compound->total_expected_out_size += expected_out_size; > + > + /* Store args for response parsing */ > + compound->op_args[compound->compound_header.count] = args; > + > + compound->compound_header.count++; > + compound->total_size += needed_size; > + > + return 0; Does fc->max_pages need to be accounted for as well? iirc, that's the upper limit on how many pages can be forwarded to the server in a request. > +} > + > +static void *fuse_copy_response_data(struct fuse_args *args, > + char *response_data) > +{ > + size_t copied = 0; > + int arg_idx; > + > + for (arg_idx = 0; arg_idx < args->out_numargs; arg_idx++) { > + struct fuse_arg current_arg = args->out_args[arg_idx]; > + size_t arg_size; > + > + /* Last argument with out_pages: copy to pages */ > + if (arg_idx == args->out_numargs - 1 && args->out_pages) { > + /* > + * External payload (in the last out arg) > + * is not supported at the moment > + */ > + 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, > + uint32_t count, void *response, > + size_t response_size) > +{ > + void *op_out_data = response; > + void *response_end = (char *)response + response_size; > + int i; > + > + if (compound->parsed) > + return 0; > + > + 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; > + } > + > + compound->parsed = true; > + 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 expected_response_size; > + size_t total_buffer_size; > + size_t actual_response_size; > + void *resp_payload; > + ssize_t ret; > + > + 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; > + } > + > + expected_response_size = compound->total_expected_out_size; > + total_buffer_size = expected_response_size + > + (compound->compound_header.count * > + sizeof(struct fuse_out_header)); > + > + resp_payload = kvmalloc(total_buffer_size, GFP_KERNEL | __GFP_ZERO); > + if (!resp_payload) > + return -ENOMEM; > + > + compound->compound_header.result_size = expected_response_size; > + > + args.in_args[0].size = sizeof(compound->compound_header); > + args.in_args[0].value = &compound->compound_header; > + args.in_args[1].size = compound->buffer_pos; > + args.in_args[1].value = compound->buffer; > + > + args.out_args[0].size = sizeof(compound->result_header); > + args.out_args[0].value = &compound->result_header; > + args.out_args[1].size = total_buffer_size; > + args.out_args[1].value = resp_payload; > + > + ret = fuse_compound_validate_header(compound); > + if (ret) > + goto out; > + > + ret = fuse_compound_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); > + return ret; > +} > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 6d59cbc877c6..2d89ca69308f 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -660,6 +660,31 @@ static void fuse_args_to_req(struct fuse_req *req, struct fuse_args *args) > __set_bit(FR_ASYNC, &req->flags); > } > > +ssize_t fuse_compound_request( > + struct fuse_mount *fm, struct fuse_args *args) > +{ > + struct fuse_req *req; > + ssize_t ret; > + > + req = fuse_get_req(&invalid_mnt_idmap, fm, false); > + if (IS_ERR(req)) > + return PTR_ERR(req); > + > + fuse_args_to_req(req, args); > + > + if (!args->noreply) > + __set_bit(FR_ISREPLY, &req->flags); > + > + __fuse_request_send(req); > + ret = req->out.h.error; > + if (!ret && args->out_argvar) { > + BUG_ON(args->out_numargs == 0); > + ret = args->out_args[args->out_numargs - 1].size; > + } > + fuse_put_request(req); > + return ret; > +} This logic looks almost identical to the logic in __fuse_simple_request(). Do you think it makes sense to just call into __fuse_simple_request() here? > + > ssize_t __fuse_simple_request(struct mnt_idmap *idmap, > struct fuse_mount *fm, > struct fuse_args *args) > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > index 7f16049387d1..86253517f59b 100644 > --- a/fs/fuse/fuse_i.h > +++ b/fs/fuse/fuse_i.h > @@ -1273,6 +1273,20 @@ 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_request(struct fuse_mount *fm, struct fuse_args *args); > +ssize_t fuse_compound_send(struct fuse_compound_req *compound); > + > +struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm, uint32_t flags); > +int fuse_compound_add(struct fuse_compound_req *compound, > + struct fuse_args *args); Looking at how this is used in the 2nd patch (open+getattr), I see that the args are first defined / filled out and then fuse_compound_add() copies them into the compound request's buffer. Instead of having a fuse_compound_add() api that takkes in filled out args, what are your thoughts on having some api that returns back a pointer to the compound buffer's current position, and then the caller can just directly fill that out? I think that ends up saving a memcpy. > +int fuse_compound_get_error(struct fuse_compound_req *compound, > + int op_idx); > +void fuse_compound_free(struct fuse_compound_req *compound); > + I think an alternative way of doing compound requests, though it would only work on io-uring, is leveraging the io-uring multishot functionality. This allows multiple cqes (requests) to be forwarded together in one system call. Instead of batching X requests into one fuse compound request, I think there would just be X cqes and the normal flow of execution would remain pretty much the same (though on the kernel side there would have to be a head "master request" that request_wait_answer() operates on). Not sure in actuality if this would be more complicated or simpler, but I think your approach here probably makes more sense since it also works for /dev/fuse. Thanks, Joanne > /** > * Assign a unique id to a fuse request > */ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH RFC v2 1/2] fuse: add compound command to combine multiple requests 2026-01-07 1:40 ` Joanne Koong @ 2026-01-07 9:29 ` Horst Birthelmer 2026-01-08 1:41 ` Joanne Koong 2026-01-07 14:03 ` Horst Birthelmer 1 sibling, 1 reply; 14+ messages in thread From: Horst Birthelmer @ 2026-01-07 9:29 UTC (permalink / raw) To: Joanne Koong Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, linux-kernel, linux-fsdevel, Horst Birthelmer, syzbot Hi Joanne, first ... thank you so much for taking the time. On Tue, Jan 06, 2026 at 05:40:52PM -0800, Joanne Koong wrote: > On Tue, Dec 23, 2025 at 2:13 PM Horst Birthelmer > <hbirthelmer@googlemail.com> wrote: > > > > 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_request() > > fuse_compound_free() > > ... > > + > > + if (offset != compound->buffer_pos) { > > + pr_info_ratelimited("FUSE: compound buffer size mismatch (calculated %zu bytes, actual %zu bytes)\n", > > + offset, compound->buffer_pos); > > + return -EINVAL; > > + } > > + > > + return 0; > > I wonder if this is overkill to have given that the kernel is in > charge of setting up the compound request and if the code is right, > has done it correctly. imo it adds extra overhead but doesn't offer > much given that the kernel code should aalready be correct. You are completely right. It was just a big help during development and has to be taken out eventually. I don't really like #ifdefs very much. Do you think we should throw it out completely or just not use it in the usual code path? > > +} > > + > > +int fuse_compound_add(struct fuse_compound_req *compound, > > + struct fuse_args *args) > > +{ > > + struct fuse_in_header *hdr; > > + size_t args_size = 0; > > + size_t needed_size; > > + size_t expected_out_size = 0; > > + int i; > > + > > + if (!compound || > > + compound->compound_header.count >= FUSE_MAX_COMPOUND_OPS) > > + return -EINVAL; > > + > > + if (args->in_pages) > > + return -EINVAL; > > + > > + for (i = 0; i < args->in_numargs; i++) > > + args_size += args->in_args[i].size; > > + > > + for (i = 0; i < args->out_numargs; i++) > > + expected_out_size += args->out_args[i].size; > > + > > + needed_size = sizeof(struct fuse_in_header) + args_size; > > + > > + if (compound->buffer_pos + needed_size > compound->buffer_size) { > > + size_t new_size = max(compound->buffer_size * 2, > > + compound->buffer_pos + needed_size); > > + char *new_buffer; > > + > > + new_size = round_up(new_size, PAGE_SIZE); > > + new_buffer = kvrealloc(compound->buffer, new_size, > > + GFP_KERNEL); > > + if (!new_buffer) > > + return -ENOMEM; > > + compound->buffer = new_buffer; > > + compound->buffer_size = new_size; > > Hmm... when we're setting up a compound request, we already know the > size that will be needed to hold all the requests, right? Do you think > it makes sense to allocate that from the get-go in > fuse_compound_alloc() and then not have to do any buffer reallocation? > I think that also gets rid of fuse_compound_req->total_size, as that > would just be the same as fuse_compound_req->buffer_size. We could do that. But then we would have to have the whole picture from the start. What I'm trying to say is (and at the moment there is no example to the contrary) that we could not create a compound without exactly knowing what will be in it. So by the time we do the allocation the calls should all be there. I have no counter argument for that at the moment which doesn't mean there isn't one. The API felt more inuitive to me doing alloc, then add, then send, but it would be faster the other way around. > > > + } > > + > > + /* Build request header */ > > + hdr = (struct fuse_in_header *)(compound->buffer + > > + compound->buffer_pos); > > + memset(hdr, 0, sizeof(*hdr)); > > + hdr->len = needed_size; > > + hdr->opcode = args->opcode; > > + hdr->nodeid = 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); > > + hdr->unique = fuse_get_unique(&compound->fm->fc->iq); > > + compound->buffer_pos += sizeof(*hdr); > > + > > + for (i = 0; i < args->in_numargs; i++) { > > + memcpy(compound->buffer + compound->buffer_pos, > > + args->in_args[i].value, args->in_args[i].size); > > + compound->buffer_pos += args->in_args[i].size; > > + } > > + > > + compound->total_expected_out_size += expected_out_size; > > + > > + /* Store args for response parsing */ > > + compound->op_args[compound->compound_header.count] = args; > > + > > + compound->compound_header.count++; > > + compound->total_size += needed_size; > > + > > + return 0; > > Does fc->max_pages need to be accounted for as well? iirc, that's the > upper limit on how many pages can be forwarded to the server in a > request. > Eventually yes. For open+getattr and lookup+create I didn't think it was relevant. Am I missing something obvious? ... > > + > > + fuse_args_to_req(req, args); > > + > > + if (!args->noreply) > > + __set_bit(FR_ISREPLY, &req->flags); > > + > > + __fuse_request_send(req); > > + ret = req->out.h.error; > > + if (!ret && args->out_argvar) { > > + BUG_ON(args->out_numargs == 0); > > + ret = args->out_args[args->out_numargs - 1].size; > > + } > > + fuse_put_request(req); > > + return ret; > > +} > > This logic looks almost identical to the logic in > __fuse_simple_request(). Do you think it makes sense to just call into > __fuse_simple_request() here? When I wrote it, I thought I had to do a different error handling. After testing I realized, that I could actually treat the compound as a normal request. So, yes I think so. Will be changed in v3. > > > + > > ssize_t __fuse_simple_request(struct mnt_idmap *idmap, > > struct fuse_mount *fm, > > struct fuse_args *args) > > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > > index 7f16049387d1..86253517f59b 100644 > > --- a/fs/fuse/fuse_i.h > > +++ b/fs/fuse/fuse_i.h > > @@ -1273,6 +1273,20 @@ 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_request(struct fuse_mount *fm, struct fuse_args *args); > > +ssize_t fuse_compound_send(struct fuse_compound_req *compound); > > + > > +struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm, uint32_t flags); > > +int fuse_compound_add(struct fuse_compound_req *compound, > > + struct fuse_args *args); > > Looking at how this is used in the 2nd patch (open+getattr), I see > that the args are first defined / filled out and then > fuse_compound_add() copies them into the compound request's buffer. > Instead of having a fuse_compound_add() api that takkes in filled out > args, what are your thoughts on having some api that returns back a > pointer to the compound buffer's current position, and then the caller > can just directly fill that out? I think that ends up saving a memcpy. > I have to think about that. Sounds like a good point, but makes the api slightly more complicated. > > +int fuse_compound_get_error(struct fuse_compound_req *compound, > > + int op_idx); > > +void fuse_compound_free(struct fuse_compound_req *compound); > > + > > I think an alternative way of doing compound requests, though it would > only work on io-uring, is leveraging the io-uring multishot > functionality. This allows multiple cqes (requests) to be forwarded > together in one system call. Instead of batching X requests into one > fuse compound request, I think there would just be X cqes and the > normal flow of execution would remain pretty much the same (though on > the kernel side there would have to be a head "master request" that > request_wait_answer() operates on). Not sure in actuality if this > would be more complicated or simpler, but I think your approach here > probably makes more sense since it also works for /dev/fuse. > I had a similar discussion with Bernd for libfuse. I completely get your point, but I think it would really complicate things. I would really want to keep this as simple as I can, since the handling gets complicated very fast the moment we get more calls and have to handle pages. Bernd already had some idea about doing writes while using compounds making the handling different form /dev/fuse to io-uring was too scary for me. > Thanks, > Joanne > Thanks, Horst ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH RFC v2 1/2] fuse: add compound command to combine multiple requests 2026-01-07 9:29 ` Horst Birthelmer @ 2026-01-08 1:41 ` Joanne Koong 0 siblings, 0 replies; 14+ messages in thread From: Joanne Koong @ 2026-01-08 1:41 UTC (permalink / raw) To: Horst Birthelmer Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, linux-kernel, linux-fsdevel, Horst Birthelmer, syzbot On Wed, Jan 7, 2026 at 1:29 AM Horst Birthelmer <horst@birthelmer.de> wrote: > Hi Horst, > > Hi Joanne, > > first ... thank you so much for taking the time. > > On Tue, Jan 06, 2026 at 05:40:52PM -0800, Joanne Koong wrote: > > On Tue, Dec 23, 2025 at 2:13 PM Horst Birthelmer > > <hbirthelmer@googlemail.com> wrote: > > > > > > 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_request() > > > fuse_compound_free() > > > > ... > > > + > > > + if (offset != compound->buffer_pos) { > > > + pr_info_ratelimited("FUSE: compound buffer size mismatch (calculated %zu bytes, actual %zu bytes)\n", > > > + offset, compound->buffer_pos); > > > + return -EINVAL; > > > + } > > > + > > > + return 0; > > > > I wonder if this is overkill to have given that the kernel is in > > charge of setting up the compound request and if the code is right, > > has done it correctly. imo it adds extra overhead but doesn't offer > > much given that the kernel code should aalready be correct. > > You are completely right. It was just a big help during development and has to be taken out eventually. > I don't really like #ifdefs very much. Do you think we should throw it out completely or just not use it in the usual code path? > imo I think we should just get rid of it entirely. > > > +} > > > + > > > +int fuse_compound_add(struct fuse_compound_req *compound, > > > + struct fuse_args *args) > > > +{ > > > + struct fuse_in_header *hdr; > > > + size_t args_size = 0; > > > + size_t needed_size; > > > + size_t expected_out_size = 0; > > > + int i; > > > + > > > + if (!compound || > > > + compound->compound_header.count >= FUSE_MAX_COMPOUND_OPS) > > > + return -EINVAL; > > > + > > > + if (args->in_pages) > > > + return -EINVAL; > > > + > > > + for (i = 0; i < args->in_numargs; i++) > > > + args_size += args->in_args[i].size; > > > + > > > + for (i = 0; i < args->out_numargs; i++) > > > + expected_out_size += args->out_args[i].size; > > > + > > > + needed_size = sizeof(struct fuse_in_header) + args_size; > > > + > > > + if (compound->buffer_pos + needed_size > compound->buffer_size) { > > > + size_t new_size = max(compound->buffer_size * 2, > > > + compound->buffer_pos + needed_size); > > > + char *new_buffer; > > > + > > > + new_size = round_up(new_size, PAGE_SIZE); > > > + new_buffer = kvrealloc(compound->buffer, new_size, > > > + GFP_KERNEL); > > > + if (!new_buffer) > > > + return -ENOMEM; > > > + compound->buffer = new_buffer; > > > + compound->buffer_size = new_size; > > > > Hmm... when we're setting up a compound request, we already know the > > size that will be needed to hold all the requests, right? Do you think > > it makes sense to allocate that from the get-go in > > fuse_compound_alloc() and then not have to do any buffer reallocation? > > I think that also gets rid of fuse_compound_req->total_size, as that > > would just be the same as fuse_compound_req->buffer_size. > > We could do that. But then we would have to have the whole picture from the start. > What I'm trying to say is (and at the moment there is no example to the contrary) > that we could not create a compound without exactly knowing what will be in it. > > So by the time we do the allocation the calls should all be there. > I have no counter argument for that at the moment which doesn't mean there isn't one. > The API felt more inuitive to me doing alloc, then add, then send, but it would be faster > the other way around. > > > > > > + } > > > + > > > + /* Build request header */ > > > + hdr = (struct fuse_in_header *)(compound->buffer + > > > + compound->buffer_pos); > > > + memset(hdr, 0, sizeof(*hdr)); > > > + hdr->len = needed_size; > > > + hdr->opcode = args->opcode; > > > + hdr->nodeid = 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); > > > + hdr->unique = fuse_get_unique(&compound->fm->fc->iq); > > > + compound->buffer_pos += sizeof(*hdr); > > > + > > > + for (i = 0; i < args->in_numargs; i++) { > > > + memcpy(compound->buffer + compound->buffer_pos, > > > + args->in_args[i].value, args->in_args[i].size); > > > + compound->buffer_pos += args->in_args[i].size; > > > + } > > > + > > > + compound->total_expected_out_size += expected_out_size; > > > + > > > + /* Store args for response parsing */ > > > + compound->op_args[compound->compound_header.count] = args; > > > + > > > + compound->compound_header.count++; > > > + compound->total_size += needed_size; > > > + > > > + return 0; > > > > Does fc->max_pages need to be accounted for as well? iirc, that's the > > upper limit on how many pages can be forwarded to the server in a > > request. > > > Eventually yes. For open+getattr and lookup+create I didn't think it was relevant. > Am I missing something obvious? I don't think it's relevant for open+getattr and lookup+create either since they won't exceed PAGE_SIZE, but imo I think it'd be good to have the fc->max_pages logic added here since the api is a general api. > > ... > > > + > > > + fuse_args_to_req(req, args); > > > + > > > + if (!args->noreply) > > > + __set_bit(FR_ISREPLY, &req->flags); > > > + > > > + __fuse_request_send(req); > > > + ret = req->out.h.error; > > > + if (!ret && args->out_argvar) { > > > + BUG_ON(args->out_numargs == 0); > > > + ret = args->out_args[args->out_numargs - 1].size; > > > + } > > > + fuse_put_request(req); > > > + return ret; > > > +} > > > > This logic looks almost identical to the logic in > > __fuse_simple_request(). Do you think it makes sense to just call into > > __fuse_simple_request() here? > > When I wrote it, I thought I had to do a different error handling. > After testing I realized, that I could actually treat the compound > as a normal request. > So, yes I think so. > Will be changed in v3. > > > > > > + > > > ssize_t __fuse_simple_request(struct mnt_idmap *idmap, > > > struct fuse_mount *fm, > > > struct fuse_args *args) > > > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > > > index 7f16049387d1..86253517f59b 100644 > > > --- a/fs/fuse/fuse_i.h > > > +++ b/fs/fuse/fuse_i.h > > > @@ -1273,6 +1273,20 @@ 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_request(struct fuse_mount *fm, struct fuse_args *args); > > > +ssize_t fuse_compound_send(struct fuse_compound_req *compound); > > > + > > > +struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm, uint32_t flags); > > > +int fuse_compound_add(struct fuse_compound_req *compound, > > > + struct fuse_args *args); > > > > Looking at how this is used in the 2nd patch (open+getattr), I see > > that the args are first defined / filled out and then > > fuse_compound_add() copies them into the compound request's buffer. > > Instead of having a fuse_compound_add() api that takkes in filled out > > args, what are your thoughts on having some api that returns back a > > pointer to the compound buffer's current position, and then the caller > > can just directly fill that out? I think that ends up saving a memcpy. > > > I have to think about that. > Sounds like a good point, but makes the api slightly more complicated. > > > > +int fuse_compound_get_error(struct fuse_compound_req *compound, > > > + int op_idx); > > > +void fuse_compound_free(struct fuse_compound_req *compound); > > > + > > > > I think an alternative way of doing compound requests, though it would > > only work on io-uring, is leveraging the io-uring multishot > > functionality. This allows multiple cqes (requests) to be forwarded > > together in one system call. Instead of batching X requests into one > > fuse compound request, I think there would just be X cqes and the > > normal flow of execution would remain pretty much the same (though on > > the kernel side there would have to be a head "master request" that > > request_wait_answer() operates on). Not sure in actuality if this > > would be more complicated or simpler, but I think your approach here > > probably makes more sense since it also works for /dev/fuse. > > > I had a similar discussion with Bernd for libfuse. > I completely get your point, but I think it would really complicate things. > > I would really want to keep this as simple as I can, since the handling > gets complicated very fast the moment we get more calls and have to handle > pages. > > Bernd already had some idea about doing writes while using compounds making the > handling different form /dev/fuse to io-uring was too scary for me. That makes sense, I think this approach overall is simpler than the io-uring approach and I agree that simpler is better :) I think for some compound requests though, it would only suffice with the io-uring approach due to the inherent limitation of /dev/fuse being bounded by the size of the lone buffer that's used for reads/writes to the /dev/fuse fd, whereas with fuse io-uring, the queue is set up with the capacity to service multiple big buffers in parallel per 1 system call. This only comes into play, I think, for compounding readahead and writeback requests. Eg the buffer used for /dev/fuse is usually 1 MB, which means the upper limit is writing back 1 MB per system call, but with io-uring, the default queue depth is 8 which means we're potentially able to send 8 MB of writeback data per 1 system call. I think it'd be very nice if we could reduce the number of system calls needed for readahead/writeback. I think maybe this could be orthogonal to the compound requests implementation you have in this patchset (eg for readahead/writeback, we use io-uring multishot to compound those requests together to be sent in 1 system call, and for other requests like open+getattr, we use the compound request infrastructure you've added here), since some servers may not be using io-uring and overall, I think implementing io-uring multishot for fuse requests is a lot simpler if those requests are all background requests (which they are for readahead/writeback requests). Thanks, Joanne > > > Thanks, > > Joanne > > > > Thanks, > Horst ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH RFC v2 1/2] fuse: add compound command to combine multiple requests 2026-01-07 1:40 ` Joanne Koong 2026-01-07 9:29 ` Horst Birthelmer @ 2026-01-07 14:03 ` Horst Birthelmer 2026-01-08 1:42 ` Joanne Koong 1 sibling, 1 reply; 14+ messages in thread From: Horst Birthelmer @ 2026-01-07 14:03 UTC (permalink / raw) To: Joanne Koong Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, linux-kernel, linux-fsdevel, Horst Birthelmer, syzbot On Tue, Jan 06, 2026 at 05:40:52PM -0800, Joanne Koong wrote: > On Tue, Dec 23, 2025 at 2:13 PM Horst Birthelmer > <hbirthelmer@googlemail.com> wrote: > > > > 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_request() > > fuse_compound_free() > > ... > > + > > + if (compound->buffer_pos + needed_size > compound->buffer_size) { > > + size_t new_size = max(compound->buffer_size * 2, > > + compound->buffer_pos + needed_size); > > + char *new_buffer; > > + > > + new_size = round_up(new_size, PAGE_SIZE); > > + new_buffer = kvrealloc(compound->buffer, new_size, > > + GFP_KERNEL); > > + if (!new_buffer) > > + return -ENOMEM; > > + compound->buffer = new_buffer; > > + compound->buffer_size = new_size; > > Hmm... when we're setting up a compound request, we already know the > size that will be needed to hold all the requests, right? Do you think > it makes sense to allocate that from the get-go in > fuse_compound_alloc() and then not have to do any buffer reallocation? > I think that also gets rid of fuse_compound_req->total_size, as that > would just be the same as fuse_compound_req->buffer_size. > After looking at this again, I realized it would be more efficient to not do any allocation in fuse_compound_alloc() at all except for the fuse_compound_req, of course, and then do all the work in fuse_compound_send(). We keep pointers to the fuse_args given to the compound command anyway since we need to fill out the result, so why not keep just the fuse args and don't copy anything except when actually sending it out? I will test this version a bit and make a simplified v3. ... > > Thanks, > Joanne Thanks, Horst ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH RFC v2 1/2] fuse: add compound command to combine multiple requests 2026-01-07 14:03 ` Horst Birthelmer @ 2026-01-08 1:42 ` Joanne Koong 0 siblings, 0 replies; 14+ messages in thread From: Joanne Koong @ 2026-01-08 1:42 UTC (permalink / raw) To: Horst Birthelmer Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, linux-kernel, linux-fsdevel, Horst Birthelmer, syzbot On Wed, Jan 7, 2026 at 6:03 AM Horst Birthelmer <horst@birthelmer.de> wrote: > > On Tue, Jan 06, 2026 at 05:40:52PM -0800, Joanne Koong wrote: > > On Tue, Dec 23, 2025 at 2:13 PM Horst Birthelmer > > <hbirthelmer@googlemail.com> wrote: > > > > > > 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_request() > > > fuse_compound_free() > > > > ... > > > + > > > + if (compound->buffer_pos + needed_size > compound->buffer_size) { > > > + size_t new_size = max(compound->buffer_size * 2, > > > + compound->buffer_pos + needed_size); > > > + char *new_buffer; > > > + > > > + new_size = round_up(new_size, PAGE_SIZE); > > > + new_buffer = kvrealloc(compound->buffer, new_size, > > > + GFP_KERNEL); > > > + if (!new_buffer) > > > + return -ENOMEM; > > > + compound->buffer = new_buffer; > > > + compound->buffer_size = new_size; > > > > Hmm... when we're setting up a compound request, we already know the > > size that will be needed to hold all the requests, right? Do you think > > it makes sense to allocate that from the get-go in > > fuse_compound_alloc() and then not have to do any buffer reallocation? > > I think that also gets rid of fuse_compound_req->total_size, as that > > would just be the same as fuse_compound_req->buffer_size. > > > After looking at this again, I realized it would be more efficient to not do any allocation > in fuse_compound_alloc() at all except for the fuse_compound_req, of course, and then > do all the work in fuse_compound_send(). > We keep pointers to the fuse_args given to the compound command anyway since we need > to fill out the result, so why not keep just the fuse args and don't copy anything > except when actually sending it out? > > I will test this version a bit and make a simplified v3. Awesome, looking forward to seeing v3. Thanks, Joanne > > ... > > > > Thanks, > > Joanne > > Thanks, > Horst ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH RFC v2 2/2] fuse: add an implementation of open+getattr 2025-12-23 22:13 [PATCH RFC v2 0/2] fuse: compound commands Horst Birthelmer 2025-12-23 22:13 ` [PATCH RFC v2 1/2] fuse: add compound command to combine multiple requests Horst Birthelmer @ 2025-12-23 22:13 ` Horst Birthelmer 2026-01-07 1:46 ` Joanne Koong 2025-12-29 6:03 ` [PATCH RFC v2 0/2] fuse: compound commands Jingbo Xu 2 siblings, 1 reply; 14+ messages in thread From: Horst Birthelmer @ 2025-12-23 22:13 UTC (permalink / raw) To: Miklos Szeredi, Bernd Schubert Cc: linux-kernel, linux-fsdevel, Horst Birthelmer 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 | 125 ++++++++++++++++++++++++++++++++++++++++++++++++------- fs/fuse/fuse_i.h | 6 ++- fs/fuse/inode.c | 6 +++ fs/fuse/ioctl.c | 2 +- 4 files changed, 121 insertions(+), 18 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 01bc894e9c2b..507b4c4ba257 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -126,8 +126,84 @@ 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 = {}, getattr_args = {}; + struct fuse_open_in open_in = {}; + struct fuse_getattr_in getattr_in = {}; + int err; + + /* Build compound request with flag to execute in the given order */ + compound = fuse_compound_alloc(fm, 0); + if (IS_ERR(compound)) + return PTR_ERR(compound); + + /* Add OPEN */ + 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; + } + open_args.opcode = opcode; + open_args.nodeid = nodeid; + open_args.in_numargs = 1; + open_args.in_args[0].size = sizeof(open_in); + open_args.in_args[0].value = &open_in; + open_args.out_numargs = 1; + open_args.out_args[0].size = sizeof(struct fuse_open_out); + open_args.out_args[0].value = outopenp; + + err = fuse_compound_add(compound, &open_args); + if (err) + goto out; + + /* Add GETATTR */ + getattr_args.opcode = FUSE_GETATTR; + getattr_args.nodeid = nodeid; + getattr_args.in_numargs = 1; + getattr_args.in_args[0].size = sizeof(getattr_in); + getattr_args.in_args[0].value = &getattr_in; + getattr_args.out_numargs = 1; + getattr_args.out_args[0].size = sizeof(struct fuse_attr_out); + getattr_args.out_args[0].value = outattrp; + + err = fuse_compound_add(compound, &getattr_args); + if (err) + goto out; + + err = fuse_compound_send(compound); + if (err) + goto out; + + /* Check if the OPEN operation succeeded */ + err = fuse_compound_get_error(compound, 0); + if (err) + goto out; + + /* Check if the GETATTR operation succeeded */ + 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; @@ -153,23 +229,41 @@ 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); - 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) { + ff->fh = outargp->fh; + ff->open_flags = outargp->open_flags; + } + } + + 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; } } } @@ -185,11 +279,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 86253517f59b..98af019037c3 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; @@ -1557,7 +1560,8 @@ 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v2 2/2] fuse: add an implementation of open+getattr 2025-12-23 22:13 ` [PATCH RFC v2 2/2] fuse: add an implementation of open+getattr Horst Birthelmer @ 2026-01-07 1:46 ` Joanne Koong 2026-01-07 9:03 ` Horst Birthelmer 0 siblings, 1 reply; 14+ messages in thread From: Joanne Koong @ 2026-01-07 1:46 UTC (permalink / raw) To: Horst Birthelmer Cc: Miklos Szeredi, Bernd Schubert, linux-kernel, linux-fsdevel, Horst Birthelmer On Tue, Dec 23, 2025 at 2:13 PM Horst Birthelmer <hbirthelmer@googlemail.com> wrote: > > 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 | 125 ++++++++++++++++++++++++++++++++++++++++++++++++------- > fs/fuse/fuse_i.h | 6 ++- > fs/fuse/inode.c | 6 +++ > fs/fuse/ioctl.c | 2 +- > 4 files changed, 121 insertions(+), 18 deletions(-) > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index 01bc894e9c2b..507b4c4ba257 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -126,8 +126,84 @@ 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 = {}, getattr_args = {}; > + struct fuse_open_in open_in = {}; > + struct fuse_getattr_in getattr_in = {}; > + int err; > + > + /* Build compound request with flag to execute in the given order */ > + compound = fuse_compound_alloc(fm, 0); > + if (IS_ERR(compound)) > + return PTR_ERR(compound); > + > + /* Add OPEN */ > + 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; > + } > + open_args.opcode = opcode; > + open_args.nodeid = nodeid; > + open_args.in_numargs = 1; > + open_args.in_args[0].size = sizeof(open_in); > + open_args.in_args[0].value = &open_in; > + open_args.out_numargs = 1; > + open_args.out_args[0].size = sizeof(struct fuse_open_out); > + open_args.out_args[0].value = outopenp; > + > + err = fuse_compound_add(compound, &open_args); > + if (err) > + goto out; > + > + /* Add GETATTR */ > + getattr_args.opcode = FUSE_GETATTR; > + getattr_args.nodeid = nodeid; > + getattr_args.in_numargs = 1; > + getattr_args.in_args[0].size = sizeof(getattr_in); > + getattr_args.in_args[0].value = &getattr_in; > + getattr_args.out_numargs = 1; > + getattr_args.out_args[0].size = sizeof(struct fuse_attr_out); > + getattr_args.out_args[0].value = outattrp; I think things end up looking cleaner here (and above for the open args) if the arg initialization logic gets abstracted into helper functions, as fuse_do_getattr() and fuse_send_open() have pretty much the exact same logic. Thanks, Joanne > + > + err = fuse_compound_add(compound, &getattr_args); > + if (err) > + goto out; > + > + err = fuse_compound_send(compound); > + if (err) > + goto out; > + > + /* Check if the OPEN operation succeeded */ > + err = fuse_compound_get_error(compound, 0); > + if (err) > + goto out; > + > + /* Check if the GETATTR operation succeeded */ > + 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; > @@ -153,23 +229,41 @@ 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); > > - 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) { > + ff->fh = outargp->fh; > + ff->open_flags = outargp->open_flags; > + } > + } > + > + 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; > } > } > } > @@ -185,11 +279,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 86253517f59b..98af019037c3 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; > > @@ -1557,7 +1560,8 @@ 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 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH RFC v2 2/2] fuse: add an implementation of open+getattr 2026-01-07 1:46 ` Joanne Koong @ 2026-01-07 9:03 ` Horst Birthelmer 0 siblings, 0 replies; 14+ messages in thread From: Horst Birthelmer @ 2026-01-07 9:03 UTC (permalink / raw) To: Joanne Koong Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, linux-kernel, linux-fsdevel, Horst Birthelmer On Tue, Jan 06, 2026 at 05:46:09PM -0800, Joanne Koong wrote: > On Tue, Dec 23, 2025 at 2:13 PM Horst Birthelmer > <hbirthelmer@googlemail.com> wrote: > > > > + open_args.opcode = opcode; > > + open_args.nodeid = nodeid; > > + open_args.in_numargs = 1; > > + open_args.in_args[0].size = sizeof(open_in); > > + open_args.in_args[0].value = &open_in; > > + open_args.out_numargs = 1; > > + open_args.out_args[0].size = sizeof(struct fuse_open_out); > > + open_args.out_args[0].value = outopenp; > > + > > + err = fuse_compound_add(compound, &open_args); > > + if (err) > > + goto out; > > + > > + /* Add GETATTR */ > > + getattr_args.opcode = FUSE_GETATTR; > > + getattr_args.nodeid = nodeid; > > + getattr_args.in_numargs = 1; > > + getattr_args.in_args[0].size = sizeof(getattr_in); > > + getattr_args.in_args[0].value = &getattr_in; > > + getattr_args.out_numargs = 1; > > + getattr_args.out_args[0].size = sizeof(struct fuse_attr_out); > > + getattr_args.out_args[0].value = outattrp; > > I think things end up looking cleaner here (and above for the open > args) if the arg initialization logic gets abstracted into helper > functions, as fuse_do_getattr() and fuse_send_open() have pretty much > the exact same logic. > > Thanks, > Joanne > You are completely right. Will change that in the next version, Thanks, Horst ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v2 0/2] fuse: compound commands 2025-12-23 22:13 [PATCH RFC v2 0/2] fuse: compound commands Horst Birthelmer 2025-12-23 22:13 ` [PATCH RFC v2 1/2] fuse: add compound command to combine multiple requests Horst Birthelmer 2025-12-23 22:13 ` [PATCH RFC v2 2/2] fuse: add an implementation of open+getattr Horst Birthelmer @ 2025-12-29 6:03 ` Jingbo Xu 2025-12-29 12:27 ` Bernd Schubert 2025-12-30 8:36 ` Horst Birthelmer 2 siblings, 2 replies; 14+ messages in thread From: Jingbo Xu @ 2025-12-29 6:03 UTC (permalink / raw) To: Horst Birthelmer, Miklos Szeredi, Bernd Schubert Cc: linux-kernel, linux-fsdevel, Horst Birthelmer, syzbot Hi Horst & Bernd, On 12/24/25 6:13 AM, Horst Birthelmer wrote: > 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). > > [1] https://lore.kernel.org/linux-fsdevel/CAJfpegshcrjXJ0USZ8RRdBy=e0MxmBTJSCE0xnxG8LXgXy-xuQ@mail.gmail.com/ > To achieve close-to-open, why not just invalidate the cached attr on open so that the following access to the attr would trigger a new FUSE_GETATTR request to fetch the latest attr from server? -- Thanks, Jingbo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v2 0/2] fuse: compound commands 2025-12-29 6:03 ` [PATCH RFC v2 0/2] fuse: compound commands Jingbo Xu @ 2025-12-29 12:27 ` Bernd Schubert 2025-12-30 8:36 ` Horst Birthelmer 1 sibling, 0 replies; 14+ messages in thread From: Bernd Schubert @ 2025-12-29 12:27 UTC (permalink / raw) To: Jingbo Xu, Horst Birthelmer, Miklos Szeredi, Joanne Koong Cc: linux-kernel, linux-fsdevel, Horst Birthelmer, syzbot On 12/29/25 07:03, Jingbo Xu wrote: > Hi Horst & Bernd, > > On 12/24/25 6:13 AM, Horst Birthelmer wrote: >> 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). >> >> [1] https://lore.kernel.org/linux-fsdevel/CAJfpegshcrjXJ0USZ8RRdBy=e0MxmBTJSCE0xnxG8LXgXy-xuQ@mail.gmail.com/ >> > > To achieve close-to-open, why not just invalidate the cached attr on > open so that the following access to the attr would trigger a new > FUSE_GETATTR request to fetch the latest attr from server? > > Hi Jingbo, because that slows down operation. Doing in 1 step is faster. https://lore.kernel.org/r/20240820211735.2098951-1-bschubert@ddn.com https://lore.kernel.org/all/20240813212149.1909627-1-joannelkoong@gmail.com/ Initially Joanne had created the 1st patch and I had asked to do that in one step and created open+getattr v2 with a new op code. Miklos then objected and asked for a compound version to avoid too many new op codes. Horst is in the mean time also working on atomic open using compounds and I have another use case for attribute updates on writes. I.e. this is the 3rd attempt. Thanks, Bernd ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH RFC v2 0/2] fuse: compound commands 2025-12-29 6:03 ` [PATCH RFC v2 0/2] fuse: compound commands Jingbo Xu 2025-12-29 12:27 ` Bernd Schubert @ 2025-12-30 8:36 ` Horst Birthelmer 2025-12-30 11:58 ` Jingbo Xu 1 sibling, 1 reply; 14+ messages in thread From: Horst Birthelmer @ 2025-12-30 8:36 UTC (permalink / raw) To: Jingbo Xu Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, linux-kernel, linux-fsdevel, Horst Birthelmer, syzbot On Mon, Dec 29, 2025 at 02:03:02PM +0800, Jingbo Xu wrote: > Hi Horst & Bernd, > > On 12/24/25 6:13 AM, Horst Birthelmer wrote: > > 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). > > > > [1] https://lore.kernel.org/linux-fsdevel/CAJfpegshcrjXJ0USZ8RRdBy=e0MxmBTJSCE0xnxG8LXgXy-xuQ@mail.gmail.com/ > > > > To achieve close-to-open, why not just invalidate the cached attr on > open so that the following access to the attr would trigger a new > FUSE_GETATTR request to fetch the latest attr from server? > Hi Jingbo, you are probably right, that it can be achieved that way. I thas some consequences that can be avoided, like having to wait at a later moment for the attributes to be fetched. The main goal of this patch however was not close-to-open, even though it was discussed in that context. We can do a lot more with the compounds than just fix close-to-open consistency. As Bernd mentioned, I am very close to havin implemented the fuse_atomic_open() call with compounds, namely the atomic combination of lookup+create. And there are some more ideas out there. open+getattr was just the low hanging fruit in this case. Cheers, Horst ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v2 0/2] fuse: compound commands 2025-12-30 8:36 ` Horst Birthelmer @ 2025-12-30 11:58 ` Jingbo Xu 0 siblings, 0 replies; 14+ messages in thread From: Jingbo Xu @ 2025-12-30 11:58 UTC (permalink / raw) To: Horst Birthelmer Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, linux-kernel, linux-fsdevel, Horst Birthelmer, syzbot On 12/30/25 4:36 PM, Horst Birthelmer wrote: > On Mon, Dec 29, 2025 at 02:03:02PM +0800, Jingbo Xu wrote: >> Hi Horst & Bernd, >> >> On 12/24/25 6:13 AM, Horst Birthelmer wrote: >>> 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). >>> >>> [1] https://lore.kernel.org/linux-fsdevel/CAJfpegshcrjXJ0USZ8RRdBy=e0MxmBTJSCE0xnxG8LXgXy-xuQ@mail.gmail.com/ >>> >> >> To achieve close-to-open, why not just invalidate the cached attr on >> open so that the following access to the attr would trigger a new >> FUSE_GETATTR request to fetch the latest attr from server? >> > > Hi Jingbo, > > you are probably right, that it can be achieved that way. I thas some consequences that can be avoided, like having to wait at a later moment for the attributes to be fetched. > > The main goal of this patch however was not close-to-open, even though it was discussed in that context. > > We can do a lot more with the compounds than just fix close-to-open consistency. As Bernd mentioned, I am very close to havin implemented the fuse_atomic_open() call with compounds, namely the atomic combination of lookup+create. > And there are some more ideas out there. > > open+getattr was just the low hanging fruit in this case. Got it. Thanks. -- Thanks, Jingbo ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-01-08 1:42 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-12-23 22:13 [PATCH RFC v2 0/2] fuse: compound commands Horst Birthelmer 2025-12-23 22:13 ` [PATCH RFC v2 1/2] fuse: add compound command to combine multiple requests Horst Birthelmer 2026-01-07 1:40 ` Joanne Koong 2026-01-07 9:29 ` Horst Birthelmer 2026-01-08 1:41 ` Joanne Koong 2026-01-07 14:03 ` Horst Birthelmer 2026-01-08 1:42 ` Joanne Koong 2025-12-23 22:13 ` [PATCH RFC v2 2/2] fuse: add an implementation of open+getattr Horst Birthelmer 2026-01-07 1:46 ` Joanne Koong 2026-01-07 9:03 ` Horst Birthelmer 2025-12-29 6:03 ` [PATCH RFC v2 0/2] fuse: compound commands Jingbo Xu 2025-12-29 12:27 ` Bernd Schubert 2025-12-30 8:36 ` Horst Birthelmer 2025-12-30 11:58 ` Jingbo Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®