* [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-02-26 16:43 [PATCH v6 0/3] fuse: compound commands Horst Birthelmer
@ 2026-02-26 16:43 ` Horst Birthelmer
2026-02-26 23:05 ` Joanne Koong
2026-02-27 9:45 ` Miklos Szeredi
2026-02-26 16:43 ` [PATCH v6 2/3] fuse: create helper functions for filling in fuse args for open and getattr Horst Birthelmer
2026-02-26 16:43 ` [PATCH v6 3/3] fuse: add an implementation of open+getattr Horst Birthelmer
2 siblings, 2 replies; 31+ messages in thread
From: Horst Birthelmer @ 2026-02-26 16:43 UTC (permalink / raw)
To: Miklos Szeredi, Bernd Schubert, Joanne Koong, Luis Henriques
Cc: linux-kernel, linux-fsdevel, Horst Birthelmer
From: Horst Birthelmer <hbirthelmer@ddn.com>
For a FUSE_COMPOUND we add a small header that informs the
fuse server how much buffer memory the kernel has for the result.
This will make the interpretation in libfuse easier,
since we can preallocate the whole result and work on the return
buffer.
Then we append the requests that belong to this compound.
The API for the compound command has:
fuse_compound_alloc()
fuse_compound_add()
fuse_compound_send()
fuse_compound_free()
Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
---
fs/fuse/Makefile | 2 +-
fs/fuse/compound.c | 308 ++++++++++++++++++++++++++++++++++++++++++++++
fs/fuse/fuse_i.h | 39 ++++++
include/uapi/linux/fuse.h | 52 ++++++++
4 files changed, 400 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..68f30123f39b244dd82b835717077cc271518e14
--- /dev/null
+++ b/fs/fuse/compound.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * FUSE: Filesystem in Userspace
+ * Copyright (C) 2025-2026
+ *
+ * Compound operations for FUSE - batch multiple operations into a single
+ * request to reduce round trips between kernel and userspace.
+ */
+
+#include "fuse_i.h"
+
+struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm,
+ u32 max_count, u32 flags)
+{
+ struct fuse_compound_req *compound;
+
+ if (max_count == 0)
+ return NULL;
+
+ compound = kzalloc(sizeof(*compound), GFP_KERNEL);
+ if (!compound)
+ return NULL;
+
+ compound->max_count = max_count;
+ compound->count = 0;
+ compound->fm = fm;
+ compound->compound_header.flags = flags;
+
+ compound->op_errors = kcalloc(max_count, sizeof(int), GFP_KERNEL);
+ if (!compound->op_errors)
+ goto out_free_compound;
+
+ compound->op_args = kcalloc(max_count, sizeof(struct fuse_args *),
+ GFP_KERNEL);
+ if (!compound->op_args)
+ goto out_free_op_errors;
+
+ compound->op_converters = kcalloc(max_count,
+ sizeof(int (*)(struct fuse_compound_req *, unsigned int)),
+ GFP_KERNEL);
+ if (!compound->op_converters)
+ goto out_free_op_args;
+
+ return compound;
+
+out_free_op_args:
+ kfree(compound->op_args);
+out_free_op_errors:
+ kfree(compound->op_errors);
+out_free_compound:
+ kfree(compound);
+ return NULL;
+}
+
+void fuse_compound_free(struct fuse_compound_req *compound)
+{
+ kfree(compound->op_errors);
+ kfree(compound->op_args);
+ kfree(compound->op_converters);
+ kfree(compound);
+}
+
+int fuse_compound_add(struct fuse_compound_req *compound,
+ struct fuse_args *args,
+ int (*converter)(struct fuse_compound_req *compound,
+ unsigned int index))
+{
+ if (!compound || compound->count >= compound->max_count)
+ return -EINVAL;
+
+ if (args->in_pages)
+ return -EINVAL;
+
+ compound->op_args[compound->count] = args;
+ compound->op_converters[compound->count] = converter;
+ compound->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;
+ }
+}
+
+static char *fuse_compound_parse_one_op(struct fuse_compound_req *compound,
+ char *response,
+ char *response_end,
+ int op_count)
+{
+ struct fuse_out_header *op_hdr = (struct fuse_out_header *)response;
+ struct fuse_args *args;
+
+ if (op_hdr->len < sizeof(struct fuse_out_header))
+ return NULL;
+
+ if (response + op_hdr->len > response_end)
+ return NULL;
+
+ if (op_count >= compound->max_count)
+ return NULL;
+
+ if (op_hdr->error) {
+ compound->op_errors[op_count] = op_hdr->error;
+ } else {
+ args = compound->op_args[op_count];
+ 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, char *response_end)
+{
+ int op_count = 0;
+
+ while (response < response_end) {
+ response = fuse_compound_parse_one_op(compound, response,
+ response_end, op_count);
+ if (!response)
+ return -EIO;
+ op_count++;
+ }
+
+ return 0;
+}
+
+static int fuse_handle_compound_results(struct fuse_compound_req *compound,
+ struct fuse_args *args)
+{
+ size_t actual_response_size;
+ size_t buffer_size;
+ char *resp_payload_buffer;
+ int ret;
+
+ buffer_size = compound->compound_header.result_size +
+ compound->count * sizeof(struct fuse_out_header);
+
+ resp_payload_buffer = args->out_args[1].value;
+ actual_response_size = args->out_args[1].size;
+
+ if (actual_response_size <= buffer_size) {
+ ret = fuse_compound_parse_resp(compound,
+ (char *)resp_payload_buffer,
+ resp_payload_buffer +
+ actual_response_size);
+ } else {
+ /* FUSE server sent more data than expected */
+ ret = -EIO;
+ }
+
+ return ret;
+}
+
+/*
+ * 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,
+ unsigned int index)
+{
+ 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;
+ hdr->unique = index;
+ hdr->len = needed_size;
+ hdr->opcode = op_args->opcode;
+ hdr->nodeid = op_args->nodeid;
+ 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;
+}
+
+static ssize_t fuse_compound_fallback_separate(struct fuse_compound_req *compound)
+{
+ unsigned int req_count = compound->count;
+ ssize_t ret = 0;
+ unsigned int i;
+
+ /* Try separate requests */
+ for (i = 0; i < req_count; i++) {
+ /* fill the current args from the already received responses */
+ if (compound->op_converters[i])
+ ret = compound->op_converters[i](compound, i);
+
+ ret = fuse_simple_request(compound->fm, compound->op_args[i]);
+ if (ret < 0) {
+ compound->op_errors[i] = ret;
+ if (!(compound->compound_header.flags & FUSE_COMPOUND_CONTINUE))
+ break;
+ }
+ }
+
+ return ret;
+}
+
+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->count;
+ size_t total_expected_out_size = 0;
+ size_t buffer_size = 0;
+ void *resp_payload_buffer;
+ char *buffer_pos;
+ void *buffer = NULL;
+ ssize_t ret;
+ unsigned 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 = kzalloc(buffer_size, GFP_KERNEL);
+ if (!buffer)
+ return -ENOMEM;
+
+ buffer_pos = buffer;
+ for (i = 0; i < req_count; i++) {
+ if (compound->op_converters[i]) {
+ ret = compound->op_converters[i](compound, i);
+ if (ret < 0)
+ goto out_free_buffer;
+ }
+
+ buffer_pos = fuse_compound_build_one_op(fc,
+ compound->op_args[i],
+ buffer_pos, i);
+ }
+
+ 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 = kzalloc(buffer_size, GFP_KERNEL);
+ 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 fallback_separate;
+
+ ret = fuse_handle_compound_results(compound, &args);
+ if (ret == 0)
+ goto out;
+
+fallback_separate:
+ /* Kernel tries to fallback to separate requests */
+ if (!(compound->compound_header.flags & FUSE_COMPOUND_ATOMIC))
+ ret = fuse_compound_fallback_separate(compound);
+
+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..e46315aa428c9d0e704c62a0b80811172c5ec9c1 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1273,6 +1273,45 @@ 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 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;
+
+ struct fuse_args **op_args;
+
+ /*
+ * Every op can add a converter function to construct the ops args from
+ * the already received responses.
+ */
+ int (**op_converters)(struct fuse_compound_req *compound,
+ unsigned int index);
+ int *op_errors;
+
+ unsigned int max_count;
+ unsigned int count;
+};
+
+/*
+ * Compound request API
+ */
+ssize_t fuse_compound_send(struct fuse_compound_req *compound);
+
+struct fuse_compound_req *fuse_compound_alloc(struct fuse_mount *fm,
+ u32 max_count, u32 flags);
+int fuse_compound_add(struct fuse_compound_req *compound,
+ struct fuse_args *args,
+ int (*converter)(struct fuse_compound_req *compound,
+ unsigned int index));
+void fuse_compound_free(struct fuse_compound_req *compound);
+static inline int fuse_compound_get_error(struct fuse_compound_req *compound, int op_idx)
+{
+ return compound->op_errors[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..d43bffd1ccbe2b3d144864407d60ff7a48db53ed 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,51 @@ struct fuse_supp_groups {
uint32_t groups[];
};
+/*
+ * This is a hint to the fuse server that all requests are complete and it can
+ * use automatic decoding and sequential processing from libfuse.
+ */
+#define FUSE_COMPOUND_SEPARABLE (1 << 0)
+/*
+ * This will be used by the kernel to continue on
+ * even after one of the requests fail.
+ */
+#define FUSE_COMPOUND_CONTINUE (1 << 1)
+/*
+ * This flags the compound as atomic, which
+ * means that the operation has to be interpreted
+ * atomically and be directly supported by the fuse server
+ * itself.
+ */
+#define FUSE_COMPOUND_ATOMIC (1 << 2)
+
+/*
+ * Compound request header
+ *
+ * This header is followed by the fuse requests
+ */
+struct fuse_compound_in {
+ uint32_t flags; /* Compound flags */
+
+ /* Total size of all results expected from the fuse server.
+ * This is needed for preallocating the whole result for all
+ * commands in the fuse server.
+ */
+ uint32_t result_size;
+ uint64_t reserved;
+};
+
+/*
+ * Compound response header
+ *
+ * This header is followed by complete fuse responses
+ */
+struct fuse_compound_out {
+ uint32_t flags; /* Result flags */
+ uint32_t padding;
+ uint64_t reserved;
+};
+
/**
* Size of the ring buffer header
*/
--
2.53.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-02-26 16:43 ` [PATCH v6 1/3] fuse: add compound command to combine multiple requests Horst Birthelmer
@ 2026-02-26 23:05 ` Joanne Koong
2026-02-27 9:45 ` Miklos Szeredi
1 sibling, 0 replies; 31+ messages in thread
From: Joanne Koong @ 2026-02-26 23:05 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Miklos Szeredi, Bernd Schubert, Luis Henriques, linux-kernel,
linux-fsdevel, Horst Birthelmer
On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>
> From: Horst Birthelmer <hbirthelmer@ddn.com>
>
> +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->count;
> + size_t total_expected_out_size = 0;
> + size_t buffer_size = 0;
> + void *resp_payload_buffer;
> + char *buffer_pos;
> + void *buffer = NULL;
> + ssize_t ret;
> + unsigned 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 = kzalloc(buffer_size, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + buffer_pos = buffer;
> + for (i = 0; i < req_count; i++) {
> + if (compound->op_converters[i]) {
> + ret = compound->op_converters[i](compound, i);
Can you explain why this is needed? The caller has all the information
up front, so why can't it just set this information before calling
fuse_compoudn_send() instead of needing this to be done in the
->op_converters callback?
> + if (ret < 0)
> + goto out_free_buffer;
> + }
> +
> + buffer_pos = fuse_compound_build_one_op(fc,
> + compound->op_args[i],
> + buffer_pos, i);
> + }
> +
> + 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 = kzalloc(buffer_size, GFP_KERNEL);
> + 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 fallback_separate;
> +
> + ret = fuse_handle_compound_results(compound, &args);
> + if (ret == 0)
> + goto out;
> +
> +fallback_separate:
> + /* Kernel tries to fallback to separate requests */
> + if (!(compound->compound_header.flags & FUSE_COMPOUND_ATOMIC))
> + ret = fuse_compound_fallback_separate(compound);
imo it's libfuse's responsibility to handle everything correctly and
if the compound request cannot be handled by libfuse for whatever
reason, the kernel should just fail it instead of retrying each
request separately. I don't see it being likely that if the compound
request fails, then sending each request separately helps. This would
also let us get rid of the FUSE_COMPOUND_CONTINUE flag which imo is a
bit confusing.
Thanks,
Joanne
> +
> +out:
> + kfree(resp_payload_buffer);
> +out_free_buffer:
> + kfree(buffer);
> + return ret;
> +}
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-02-26 16:43 ` [PATCH v6 1/3] fuse: add compound command to combine multiple requests Horst Birthelmer
2026-02-26 23:05 ` Joanne Koong
@ 2026-02-27 9:45 ` Miklos Szeredi
2026-02-27 10:48 ` Horst Birthelmer
` (2 more replies)
1 sibling, 3 replies; 31+ messages in thread
From: Miklos Szeredi @ 2026-02-27 9:45 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Bernd Schubert, Joanne Koong, Luis Henriques, linux-kernel,
linux-fsdevel, Horst Birthelmer
On Thu, 26 Feb 2026 at 17:43, Horst Birthelmer <horst@birthelmer.com> wrote:
> +int fuse_compound_add(struct fuse_compound_req *compound,
> + struct fuse_args *args,
> + int (*converter)(struct fuse_compound_req *compound,
> + unsigned int index))
> +{
> + if (!compound || compound->count >= compound->max_count)
> + return -EINVAL;
> +
> + if (args->in_pages)
> + return -EINVAL;
WARN_ON()
> +
> + compound->op_args[compound->count] = args;
This could be done *much* simpler with lists. Just add a 'struct
list_head list' member to struct fuse_args and pass a 'struct
list_head *compound' to fuse_compound_add(). No need for
fuse_compound_alloc/free().
Alternatively pass a 'void **' to fuse_compound_add(), where the input
args could be copied directly. This has the advantage of not having to
keep the args around, so they could be local to the fill function.
After the request is done the responses would similarly be decoded
into the outargs.
Both approaches have advantages and disadvantages, I don't see a clear winner.
> + compound->op_converters[compound->count] = converter;
What are these converters?
> +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->count;
> + size_t total_expected_out_size = 0;
> + size_t buffer_size = 0;
> + void *resp_payload_buffer;
> + char *buffer_pos;
> + void *buffer = NULL;
> + ssize_t ret;
> + unsigned 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 = kzalloc(buffer_size, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + buffer_pos = buffer;
> + for (i = 0; i < req_count; i++) {
> + if (compound->op_converters[i]) {
> + ret = compound->op_converters[i](compound, i);
> + if (ret < 0)
> + goto out_free_buffer;
> + }
> +
> + buffer_pos = fuse_compound_build_one_op(fc,
> + compound->op_args[i],
> + buffer_pos, i);
> + }
> +
> + 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 = kzalloc(buffer_size, GFP_KERNEL);
> + 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 fallback_separate;
> +
> + ret = fuse_handle_compound_results(compound, &args);
> + if (ret == 0)
> + goto out;
> +
> +fallback_separate:
> + /* Kernel tries to fallback to separate requests */
> + if (!(compound->compound_header.flags & FUSE_COMPOUND_ATOMIC))
> + ret = fuse_compound_fallback_separate(compound);
> +
> +out:
> + kfree(resp_payload_buffer);
> +out_free_buffer:
> + kfree(buffer);
> + return ret;
> +}
If we go with the list of fuse_args, then all the above logic could go
into the lower layer (dev.c) which already handles fuse_args ->
request -> fuse_args conversion. What's needed is mostly just a loop
that repeats this for all the sub requests.
> +struct fuse_compound_req {
> + struct fuse_mount *fm;
> + struct fuse_compound_in compound_header;
> + struct fuse_compound_out result_header;
> +
> + struct fuse_args **op_args;
> +
> + /*
> + * Every op can add a converter function to construct the ops args from
> + * the already received responses.
> + */
> + int (**op_converters)(struct fuse_compound_req *compound,
> + unsigned int index);
> + int *op_errors;
Can go into fuse_args.
> +
> + unsigned int max_count;
> + unsigned int count;
> +};
> +/*
> + * This is a hint to the fuse server that all requests are complete and it can
> + * use automatic decoding and sequential processing from libfuse.
> + */
> +#define FUSE_COMPOUND_SEPARABLE (1 << 0)
We really need per sub-request flags, not per-compound flags.
I.e:
FUSE_SUB_IS_ENTRY - this sub request will return a new entry on
success (nodeid, filehandle)
FUSE_SUB_DEP_ENTRY - this sub request depends on the result of a previous lookup
> +/*
> + * This will be used by the kernel to continue on
> + * even after one of the requests fail.
> + */
> +#define FUSE_COMPOUND_CONTINUE (1 << 1)
Again, I think it makes no sense to have compound-global flags, since
it might be possible that there are several sub-requests and there are
different dependencies between for each of them.
Also if there are no examples of a certain flag in this patchset, then
it's better to just leave it out and add it together with the actual
user.
> +/*
> + * This flags the compound as atomic, which
> + * means that the operation has to be interpreted
> + * atomically and be directly supported by the fuse server
> + * itself.
> + */
> +#define FUSE_COMPOUND_ATOMIC (1 << 2)
Why would this be needed? The VFS provides the locking that ensures
atomicity, even if the implementation is not atomic. At least for
local filesystems that is always the case.
> +
> +/*
> + * Compound request header
> + *
> + * This header is followed by the fuse requests
> + */
> +struct fuse_compound_in {
> + uint32_t flags; /* Compound flags */
Not needed.
> +
> + /* Total size of all results expected from the fuse server.
> + * This is needed for preallocating the whole result for all
> + * commands in the fuse server.
> + */
> + uint32_t result_size;
Please drop this. I think libfuse can allocate separate buffers for
each sub-request's out arg and hand a vector of these to the transport
layer.
> + uint64_t reserved;
So it turns out the compound header is empty. Not a problem, just
make it contain 'uint64_t reserved[2]' for future use.
> +};
> +
> +/*
> + * Compound response header
> + *
> + * This header is followed by complete fuse responses
> + */
> +struct fuse_compound_out {
> + uint32_t flags; /* Result flags */
What is this for?
> + uint32_t padding;
> + uint64_t reserved;
> +};
Thanks,
Miklos
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: Re: [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-02-27 9:45 ` Miklos Szeredi
@ 2026-02-27 10:48 ` Horst Birthelmer
2026-02-27 11:29 ` Miklos Szeredi
2026-03-02 9:56 ` Horst Birthelmer
2026-03-06 14:27 ` Horst Birthelmer
2 siblings, 1 reply; 31+ messages in thread
From: Horst Birthelmer @ 2026-02-27 10:48 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Horst Birthelmer, Bernd Schubert, Joanne Koong, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Fri, Feb 27, 2026 at 10:45:36AM +0100, Miklos Szeredi wrote:
> On Thu, 26 Feb 2026 at 17:43, Horst Birthelmer <horst@birthelmer.com> wrote:
>
> > +int fuse_compound_add(struct fuse_compound_req *compound,
> > + struct fuse_args *args,
> > + int (*converter)(struct fuse_compound_req *compound,
> > + unsigned int index))
> > +{
> > + if (!compound || compound->count >= compound->max_count)
> > + return -EINVAL;
> > +
> > + if (args->in_pages)
> > + return -EINVAL;
>
> WARN_ON()
>
> > +
> > + compound->op_args[compound->count] = args;
>
> This could be done *much* simpler with lists. Just add a 'struct
> list_head list' member to struct fuse_args and pass a 'struct
> list_head *compound' to fuse_compound_add(). No need for
> fuse_compound_alloc/free().
>
> Alternatively pass a 'void **' to fuse_compound_add(), where the input
> args could be copied directly. This has the advantage of not having to
> keep the args around, so they could be local to the fill function.
> After the request is done the responses would similarly be decoded
> into the outargs.
>
> Both approaches have advantages and disadvantages, I don't see a clear winner.
Will have another go at this.
> > + compound->op_converters[compound->count] = converter;
>
> What are these converters?
This was my way of dealing with the interdependencies.
The automatic sequencialization will call this for every request.
So we can copy and manipulate the args for the next request.
No need for any other flags then. We can provide one or more
of this callback functions and be done.
>
> > +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->count;
> > + size_t total_expected_out_size = 0;
> > + size_t buffer_size = 0;
> > + void *resp_payload_buffer;
> > + char *buffer_pos;
> > + void *buffer = NULL;
> > + ssize_t ret;
> > + unsigned 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 = kzalloc(buffer_size, GFP_KERNEL);
> > + if (!buffer)
> > + return -ENOMEM;
> > +
> > + buffer_pos = buffer;
> > + for (i = 0; i < req_count; i++) {
> > + if (compound->op_converters[i]) {
> > + ret = compound->op_converters[i](compound, i);
> > + if (ret < 0)
> > + goto out_free_buffer;
> > + }
> > +
> > + buffer_pos = fuse_compound_build_one_op(fc,
> > + compound->op_args[i],
> > + buffer_pos, i);
> > + }
> > +
> > + 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 = kzalloc(buffer_size, GFP_KERNEL);
> > + 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 fallback_separate;
> > +
> > + ret = fuse_handle_compound_results(compound, &args);
> > + if (ret == 0)
> > + goto out;
> > +
> > +fallback_separate:
> > + /* Kernel tries to fallback to separate requests */
> > + if (!(compound->compound_header.flags & FUSE_COMPOUND_ATOMIC))
> > + ret = fuse_compound_fallback_separate(compound);
> > +
> > +out:
> > + kfree(resp_payload_buffer);
> > +out_free_buffer:
> > + kfree(buffer);
> > + return ret;
> > +}
>
> If we go with the list of fuse_args, then all the above logic could go
> into the lower layer (dev.c) which already handles fuse_args ->
> request -> fuse_args conversion. What's needed is mostly just a loop
> that repeats this for all the sub requests.
>
>
> > +struct fuse_compound_req {
> > + struct fuse_mount *fm;
> > + struct fuse_compound_in compound_header;
> > + struct fuse_compound_out result_header;
> > +
> > + struct fuse_args **op_args;
> > +
> > + /*
> > + * Every op can add a converter function to construct the ops args from
> > + * the already received responses.
> > + */
> > + int (**op_converters)(struct fuse_compound_req *compound,
> > + unsigned int index);
> > + int *op_errors;
>
> Can go into fuse_args.
>
> > +
> > + unsigned int max_count;
> > + unsigned int count;
> > +};
> > +/*
> > + * This is a hint to the fuse server that all requests are complete and it can
> > + * use automatic decoding and sequential processing from libfuse.
> > + */
> > +#define FUSE_COMPOUND_SEPARABLE (1 << 0)
>
> We really need per sub-request flags, not per-compound flags.
>
> I.e:
>
> FUSE_SUB_IS_ENTRY - this sub request will return a new entry on
> success (nodeid, filehandle)
> FUSE_SUB_DEP_ENTRY - this sub request depends on the result of a previous lookup
>
we don't need this if we use my converters from above.
> > +/*
> > + * This will be used by the kernel to continue on
> > + * even after one of the requests fail.
> > + */
> > +#define FUSE_COMPOUND_CONTINUE (1 << 1)
>
> Again, I think it makes no sense to have compound-global flags, since
> it might be possible that there are several sub-requests and there are
> different dependencies between for each of them.
>
> Also if there are no examples of a certain flag in this patchset, then
> it's better to just leave it out and add it together with the actual
> user.
actually there is in compound.c
>
> > +/*
> > + * This flags the compound as atomic, which
> > + * means that the operation has to be interpreted
> > + * atomically and be directly supported by the fuse server
> > + * itself.
> > + */
> > +#define FUSE_COMPOUND_ATOMIC (1 << 2)
>
> Why would this be needed? The VFS provides the locking that ensures
> atomicity, even if the implementation is not atomic. At least for
> local filesystems that is always the case.
>
we (by we I mean the fuse server I work on) could use the information that
a certain combination of requests should be atomic.
> > +
> > +/*
> > + * Compound request header
> > + *
> > + * This header is followed by the fuse requests
> > + */
> > +struct fuse_compound_in {
> > + uint32_t flags; /* Compound flags */
>
> Not needed.
>
> > +
> > + /* Total size of all results expected from the fuse server.
> > + * This is needed for preallocating the whole result for all
> > + * commands in the fuse server.
> > + */
> > + uint32_t result_size;
>
> Please drop this. I think libfuse can allocate separate buffers for
> each sub-request's out arg and hand a vector of these to the transport
> layer.
>
> > + uint64_t reserved;
>
> So it turns out the compound header is empty. Not a problem, just
> make it contain 'uint64_t reserved[2]' for future use.
>
OK, will do.
> > +};
> > +
> > +/*
> > + * Compound response header
> > + *
> > + * This header is followed by complete fuse responses
> > + */
> > +struct fuse_compound_out {
> > + uint32_t flags; /* Result flags */
>
> What is this for?
>
This was used for signalling stuff from the fuse server, like e.g.
did we actually create something etc.
On second glance ... in the spirit of your minimalization, probably
not needed any more.
> > + uint32_t padding;
> > + uint64_t reserved;
> > +};
>
> Thanks,
> Miklos
Overall I like your idea to make compounds really minimal.
There is only the part with the interdependencies that I struggle with, since
almost all examples I tried did not have a very simple methodology.
(LOOKUP+MKNOD+OPEN or Luis CREATE_HANDLE+OPEN)
Could you maybe provide some examples of usecases, that I should try to drill the
new logic?
It feels like you have other compounds in mind than I do.
I have used compounds to send groups of semantically linked requests to the fuse server
signalling to it if the kernel expects it to be one atomic operation or a preferred
'group' of requests (like open+getattr, nothing happens if those are not processed atomic
in a distributed file system)
Thanks for taking the time!
Horst
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: Re: [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-02-27 10:48 ` Horst Birthelmer
@ 2026-02-27 11:29 ` Miklos Szeredi
2026-02-27 11:37 ` Horst Birthelmer
0 siblings, 1 reply; 31+ messages in thread
From: Miklos Szeredi @ 2026-02-27 11:29 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Horst Birthelmer, Bernd Schubert, Joanne Koong, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Fri, 27 Feb 2026 at 11:48, Horst Birthelmer <horst@birthelmer.de> wrote:
> > FUSE_SUB_IS_ENTRY - this sub request will return a new entry on
> > success (nodeid, filehandle)
> > FUSE_SUB_DEP_ENTRY - this sub request depends on the result of a previous lookup
> >
>
> we don't need this if we use my converters from above.
Dependencies need to be handled by the kernel and libfuse as well.
Makes no sense to have two separate mechanisms for handling
dependencies, so the kernel should use the same flags.
> Could you maybe provide some examples of usecases, that I should try to drill the
> new logic?
- LOOKUP + GETATTR[L]
- MKOBJ + (SETXATTR[L] (only for posix_acl inheritance)) + GETATTR[L]
+ (OPEN[L] (optional)
- SETATTR + SETXATTR (setting posix_acl that modifies mode or setting
mode on file with posix_acl)
- INIT + LOOKUP_ROOT + GETATTR[L]
- OPEN + IOCTL[O] + RELEASE[O] (fileattr_get/set)
Only two dependencies here: lookup or open. Both are simple in terms
of just needing to copy a field from a previous request to the current
one with fixed positions in all of the above cases.
The LOOKUP + MKNOD one *is* more complicated, because it makes
execution of the MKNOD dependent on the result of the LOOKUP, so the
dependency handler needs to look inside the result and decide how to
proceed based on that. Some pros and cons of both approaches, so I'm
curious to see how yours looks like.
> I have used compounds to send groups of semantically linked requests to the fuse server
> signalling to it if the kernel expects it to be one atomic operation or a preferred
> 'group' of requests (like open+getattr, nothing happens if those are not processed atomic
> in a distributed file system)
Which is the case where the kernel expects them to be atomic?
Thanks,
Miklos
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: Re: [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-02-27 11:29 ` Miklos Szeredi
@ 2026-02-27 11:37 ` Horst Birthelmer
2026-02-27 11:58 ` Miklos Szeredi
0 siblings, 1 reply; 31+ messages in thread
From: Horst Birthelmer @ 2026-02-27 11:37 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Horst Birthelmer, Bernd Schubert, Joanne Koong, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Fri, Feb 27, 2026 at 12:29:00PM +0100, Miklos Szeredi wrote:
> On Fri, 27 Feb 2026 at 11:48, Horst Birthelmer <horst@birthelmer.de> wrote:
>
> > > FUSE_SUB_IS_ENTRY - this sub request will return a new entry on
> > > success (nodeid, filehandle)
> > > FUSE_SUB_DEP_ENTRY - this sub request depends on the result of a previous lookup
> > >
> >
> > we don't need this if we use my converters from above.
>
> Dependencies need to be handled by the kernel and libfuse as well.
> Makes no sense to have two separate mechanisms for handling
> dependencies, so the kernel should use the same flags.
>
OK, got it.
> > Could you maybe provide some examples of usecases, that I should try to drill the
> > new logic?
>
> - LOOKUP + GETATTR[L]
> - MKOBJ + (SETXATTR[L] (only for posix_acl inheritance)) + GETATTR[L]
> + (OPEN[L] (optional)
> - SETATTR + SETXATTR (setting posix_acl that modifies mode or setting
> mode on file with posix_acl)
> - INIT + LOOKUP_ROOT + GETATTR[L]
> - OPEN + IOCTL[O] + RELEASE[O] (fileattr_get/set)
>
> Only two dependencies here: lookup or open. Both are simple in terms
> of just needing to copy a field from a previous request to the current
> one with fixed positions in all of the above cases.
>
> The LOOKUP + MKNOD one *is* more complicated, because it makes
> execution of the MKNOD dependent on the result of the LOOKUP, so the
> dependency handler needs to look inside the result and decide how to
> proceed based on that. Some pros and cons of both approaches, so I'm
> curious to see how yours looks like.
>
I really am greateful for this list. Helps me a lot, since I was looking at this from the
perspective of the fuse server, which truns out to be different.
> > I have used compounds to send groups of semantically linked requests to the fuse server
> > signalling to it if the kernel expects it to be one atomic operation or a preferred
> > 'group' of requests (like open+getattr, nothing happens if those are not processed atomic
> > in a distributed file system)
>
> Which is the case where the kernel expects them to be atomic?
>
I naively thought that fuse_atomic_open() was actually there to do an atomic open ... ;-)
> Thanks,
> Miklos
>
Thanks,
Horst
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: Re: [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-02-27 11:37 ` Horst Birthelmer
@ 2026-02-27 11:58 ` Miklos Szeredi
0 siblings, 0 replies; 31+ messages in thread
From: Miklos Szeredi @ 2026-02-27 11:58 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Horst Birthelmer, Bernd Schubert, Joanne Koong, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Fri, 27 Feb 2026 at 12:37, Horst Birthelmer <horst@birthelmer.de> wrote:
> I naively thought that fuse_atomic_open() was actually there to do an atomic open ... ;-)
Yes, it helps with atomicity relative to operations on other clients
in a distributed fs.
For a local fs it does not make a difference in terms of correctness,
but with compounds it could improve performance compared to separate
lookup + mknod + open.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-02-27 9:45 ` Miklos Szeredi
2026-02-27 10:48 ` Horst Birthelmer
@ 2026-03-02 9:56 ` Horst Birthelmer
2026-03-02 11:03 ` Miklos Szeredi
2026-03-06 14:27 ` Horst Birthelmer
2 siblings, 1 reply; 31+ messages in thread
From: Horst Birthelmer @ 2026-03-02 9:56 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Horst Birthelmer, Bernd Schubert, Joanne Koong, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Fri, Feb 27, 2026 at 10:45:36AM +0100, Miklos Szeredi wrote:
> On Thu, 26 Feb 2026 at 17:43, Horst Birthelmer <horst@birthelmer.com> wrote:
> > +
> > + unsigned int max_count;
> > + unsigned int count;
> > +};
> > +/*
> > + * This is a hint to the fuse server that all requests are complete and it can
> > + * use automatic decoding and sequential processing from libfuse.
> > + */
> > +#define FUSE_COMPOUND_SEPARABLE (1 << 0)
>
> We really need per sub-request flags, not per-compound flags.
>
> I.e:
>
> FUSE_SUB_IS_ENTRY - this sub request will return a new entry on
> success (nodeid, filehandle)
> FUSE_SUB_DEP_ENTRY - this sub request depends on the result of a previous lookup
>
Couldn't we just save boolean flags in the fuse_args?
Something like 'bool is_sub_entry:1' and so on?
If we have the automatic separation and call of requests in the kernel
when the fuse server returns ENOSYS, I don't see the point in adding this
to libfuse as well, since there will never be the case, that kernel
doesn't support compounds but libfuse does.
It's either the fuse server handles the whole compound, or the kernel does.
My point is, we don't need to send that information anywhere.
> Thanks,
> Miklos
Thanks for taking the time,
Horst
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-03-02 9:56 ` Horst Birthelmer
@ 2026-03-02 11:03 ` Miklos Szeredi
2026-03-02 13:19 ` Horst Birthelmer
0 siblings, 1 reply; 31+ messages in thread
From: Miklos Szeredi @ 2026-03-02 11:03 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Horst Birthelmer, Bernd Schubert, Joanne Koong, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Mon, 2 Mar 2026 at 10:56, Horst Birthelmer <horst@birthelmer.de> wrote:
>
> On Fri, Feb 27, 2026 at 10:45:36AM +0100, Miklos Szeredi wrote:
> > On Thu, 26 Feb 2026 at 17:43, Horst Birthelmer <horst@birthelmer.com> wrote:
> > > +
> > > + unsigned int max_count;
> > > + unsigned int count;
> > > +};
> > > +/*
> > > + * This is a hint to the fuse server that all requests are complete and it can
> > > + * use automatic decoding and sequential processing from libfuse.
> > > + */
> > > +#define FUSE_COMPOUND_SEPARABLE (1 << 0)
> >
> > We really need per sub-request flags, not per-compound flags.
> >
> > I.e:
> >
> > FUSE_SUB_IS_ENTRY - this sub request will return a new entry on
> > success (nodeid, filehandle)
> > FUSE_SUB_DEP_ENTRY - this sub request depends on the result of a previous lookup
> >
>
> Couldn't we just save boolean flags in the fuse_args?
> Something like 'bool is_sub_entry:1' and so on?
Sure, that's fine.
> If we have the automatic separation and call of requests in the kernel
> when the fuse server returns ENOSYS, I don't see the point in adding this
> to libfuse as well, since there will never be the case, that kernel
> doesn't support compounds but libfuse does.
> It's either the fuse server handles the whole compound, or the kernel does.
No, I think the library is in a good position to handle compounds,
because that can reduce the complexity in the server while keeping
most of the performance benefits.
> My point is, we don't need to send that information anywhere.
We need to send that information in any case. It needs to be part of
the matching done by the server to "recognize" a certain compound,
because the same sequence of operations could have different meaning
if the dependencies are different.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: Re: [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-03-02 11:03 ` Miklos Szeredi
@ 2026-03-02 13:19 ` Horst Birthelmer
2026-03-02 13:30 ` Miklos Szeredi
0 siblings, 1 reply; 31+ messages in thread
From: Horst Birthelmer @ 2026-03-02 13:19 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Horst Birthelmer, Bernd Schubert, Joanne Koong, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Mon, Mar 02, 2026 at 12:03:35PM +0100, Miklos Szeredi wrote:
> On Mon, 2 Mar 2026 at 10:56, Horst Birthelmer <horst@birthelmer.de> wrote:
> >
> > On Fri, Feb 27, 2026 at 10:45:36AM +0100, Miklos Szeredi wrote:
> > > On Thu, 26 Feb 2026 at 17:43, Horst Birthelmer <horst@birthelmer.com> wrote:
> > > > +
> > > > + unsigned int max_count;
> > > > + unsigned int count;
> > > > +};
> > > > +/*
> > > > + * This is a hint to the fuse server that all requests are complete and it can
> > > > + * use automatic decoding and sequential processing from libfuse.
> > > > + */
> > > > +#define FUSE_COMPOUND_SEPARABLE (1 << 0)
> > >
> > > We really need per sub-request flags, not per-compound flags.
> > >
> > > I.e:
> > >
> > > FUSE_SUB_IS_ENTRY - this sub request will return a new entry on
> > > success (nodeid, filehandle)
> > > FUSE_SUB_DEP_ENTRY - this sub request depends on the result of a previous lookup
> > >
> >
> > Couldn't we just save boolean flags in the fuse_args?
> > Something like 'bool is_sub_entry:1' and so on?
>
> Sure, that's fine.
>
> > If we have the automatic separation and call of requests in the kernel
> > when the fuse server returns ENOSYS, I don't see the point in adding this
> > to libfuse as well, since there will never be the case, that kernel
> > doesn't support compounds but libfuse does.
> > It's either the fuse server handles the whole compound, or the kernel does.
>
> No, I think the library is in a good position to handle compounds,
> because that can reduce the complexity in the server while keeping
> most of the performance benefits.
>
> > My point is, we don't need to send that information anywhere.
>
> We need to send that information in any case. It needs to be part of
> the matching done by the server to "recognize" a certain compound,
> because the same sequence of operations could have different meaning
> if the dependencies are different.
OK, if I have to send flags, that are only present if the fuse request
is inside a compound then I would suggest that we preface the fuse request
with a small compound header, where we store that information.
I would not want to change the fuse request, especially not define the same
flags for every type of fuse requests.
Would that be acceptable?
>
> Thanks,
> Miklos
>
Thanks,
Horst
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: Re: [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-03-02 13:19 ` Horst Birthelmer
@ 2026-03-02 13:30 ` Miklos Szeredi
0 siblings, 0 replies; 31+ messages in thread
From: Miklos Szeredi @ 2026-03-02 13:30 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Horst Birthelmer, Bernd Schubert, Joanne Koong, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Mon, 2 Mar 2026 at 14:19, Horst Birthelmer <horst@birthelmer.de> wrote:
> OK, if I have to send flags, that are only present if the fuse request
> is inside a compound then I would suggest that we preface the fuse request
> with a small compound header, where we store that information.
>
> I would not want to change the fuse request, especially not define the same
> flags for every type of fuse requests.
>
> Would that be acceptable?
Yes, a separate header is the cleanest approach,
Thanks,
Miklos
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: [PATCH v6 1/3] fuse: add compound command to combine multiple requests
2026-02-27 9:45 ` Miklos Szeredi
2026-02-27 10:48 ` Horst Birthelmer
2026-03-02 9:56 ` Horst Birthelmer
@ 2026-03-06 14:27 ` Horst Birthelmer
2 siblings, 0 replies; 31+ messages in thread
From: Horst Birthelmer @ 2026-03-06 14:27 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Horst Birthelmer, Bernd Schubert, Joanne Koong, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Fri, Feb 27, 2026 at 10:45:36AM +0100, Miklos Szeredi wrote:
> On Thu, 26 Feb 2026 at 17:43, Horst Birthelmer <horst@birthelmer.com> wrote:
>
> > +
> > +fallback_separate:
> > + /* Kernel tries to fallback to separate requests */
> > + if (!(compound->compound_header.flags & FUSE_COMPOUND_ATOMIC))
> > + ret = fuse_compound_fallback_separate(compound);
> > +
> > +out:
> > + kfree(resp_payload_buffer);
> > +out_free_buffer:
> > + kfree(buffer);
> > + return ret;
> > +}
>
> If we go with the list of fuse_args, then all the above logic could go
> into the lower layer (dev.c) which already handles fuse_args ->
> request -> fuse_args conversion. What's needed is mostly just a loop
> that repeats this for all the sub requests.
>
>
I have actually implemented this idea and avoided any memory allocation.
So the short version is, it can be done.
But to me this looks kinda ugly and a bit wrong. I have to check in the
lower layer for an opcode from the upper layer and 'stream' the args.
(in fuse_dev_do_read() or somewhere in that region there has to be
a check for FUSE_COMPOUND and then call into different code)
When handled on that level it has to be handled for io-uring
slightly differently as well.
I will test this a bit more and provide a new version unless someone
tells me that this is not the right direction.
>
> Thanks,
> Miklos
Thanks,
Horst
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v6 2/3] fuse: create helper functions for filling in fuse args for open and getattr
2026-02-26 16:43 [PATCH v6 0/3] fuse: compound commands Horst Birthelmer
2026-02-26 16:43 ` [PATCH v6 1/3] fuse: add compound command to combine multiple requests Horst Birthelmer
@ 2026-02-26 16:43 ` Horst Birthelmer
2026-02-26 16:43 ` [PATCH v6 3/3] fuse: add an implementation of open+getattr Horst Birthelmer
2 siblings, 0 replies; 31+ messages in thread
From: Horst Birthelmer @ 2026-02-26 16:43 UTC (permalink / raw)
To: Miklos Szeredi, Bernd Schubert, Joanne Koong, Luis Henriques
Cc: linux-kernel, linux-fsdevel, Horst Birthelmer
From: Horst Birthelmer <hbirthelmer@ddn.com>
create fuse_getattr_args_fill() and fuse_open_args_fill() to fill in
the parameters for the open and getattr calls.
This is in preparation for implementing open+getattr and does not
represent any functional change.
Suggested-by: Joanne Koong <joannelkoong@gmail.com>
Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
---
fs/fuse/dir.c | 26 ++++++++++++++++++--------
fs/fuse/file.c | 26 ++++++++++++++++++--------
fs/fuse/fuse_i.h | 6 ++++++
3 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 3927cb069236e9c52674301831c6d655397f24c5..e5ae033a15e85757a10a38b5e7d03dac86067c2a 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1471,6 +1471,23 @@ static int fuse_do_statx(struct mnt_idmap *idmap, struct inode *inode,
return 0;
}
+/*
+ * Helper function to initialize fuse_args for GETATTR operations
+ */
+void fuse_getattr_args_fill(struct fuse_args *args, u64 nodeid,
+ struct fuse_getattr_in *inarg,
+ struct fuse_attr_out *outarg)
+{
+ args->opcode = FUSE_GETATTR;
+ args->nodeid = nodeid;
+ args->in_numargs = 1;
+ args->in_args[0].size = sizeof(*inarg);
+ args->in_args[0].value = inarg;
+ args->out_numargs = 1;
+ args->out_args[0].size = sizeof(*outarg);
+ args->out_args[0].value = outarg;
+}
+
static int fuse_do_getattr(struct mnt_idmap *idmap, struct inode *inode,
struct kstat *stat, struct file *file)
{
@@ -1492,14 +1509,7 @@ static int fuse_do_getattr(struct mnt_idmap *idmap, struct inode *inode,
inarg.getattr_flags |= FUSE_GETATTR_FH;
inarg.fh = ff->fh;
}
- args.opcode = FUSE_GETATTR;
- args.nodeid = get_node_id(inode);
- args.in_numargs = 1;
- args.in_args[0].size = sizeof(inarg);
- args.in_args[0].value = &inarg;
- args.out_numargs = 1;
- args.out_args[0].size = sizeof(outarg);
- args.out_args[0].value = &outarg;
+ fuse_getattr_args_fill(&args, get_node_id(inode), &inarg, &outarg);
err = fuse_simple_request(fm, &args);
if (!err) {
if (fuse_invalid_attr(&outarg.attr) ||
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 3b2a171e652f0c9dd1c9e37253d3d3e88caab148..a408a9668abbb361e2c1e386ebab9dfcb0a7a573 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -23,6 +23,23 @@
#include <linux/task_io_accounting_ops.h>
#include <linux/iomap.h>
+/*
+ * Helper function to initialize fuse_args for OPEN/OPENDIR operations
+ */
+static void fuse_open_args_fill(struct fuse_args *args, u64 nodeid, int opcode,
+ struct fuse_open_in *inarg, struct fuse_open_out *outarg)
+{
+ args->opcode = opcode;
+ args->nodeid = nodeid;
+ args->in_numargs = 1;
+ args->in_args[0].size = sizeof(*inarg);
+ args->in_args[0].value = inarg;
+ args->out_numargs = 1;
+ args->out_args[0].size = sizeof(*outarg);
+ args->out_args[0].value = outarg;
+}
+
+
static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
unsigned int open_flags, int opcode,
struct fuse_open_out *outargp)
@@ -40,14 +57,7 @@ static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
inarg.open_flags |= FUSE_OPEN_KILL_SUIDGID;
}
- args.opcode = opcode;
- args.nodeid = nodeid;
- args.in_numargs = 1;
- args.in_args[0].size = sizeof(inarg);
- args.in_args[0].value = &inarg;
- args.out_numargs = 1;
- args.out_args[0].size = sizeof(*outargp);
- args.out_args[0].value = outargp;
+ fuse_open_args_fill(&args, nodeid, opcode, &inarg, outargp);
return fuse_simple_request(fm, &args);
}
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index e46315aa428c9d0e704c62a0b80811172c5ec9c1..ff8222b66c4f7b04c0671a980237a43871affd0a 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1179,6 +1179,12 @@ struct fuse_io_args {
void fuse_read_args_fill(struct fuse_io_args *ia, struct file *file, loff_t pos,
size_t count, int opcode);
+/*
+ * Helper functions to initialize fuse_args for common operations
+ */
+void fuse_getattr_args_fill(struct fuse_args *args, u64 nodeid,
+ struct fuse_getattr_in *inarg,
+ struct fuse_attr_out *outarg);
struct fuse_file *fuse_file_alloc(struct fuse_mount *fm, bool release);
void fuse_file_free(struct fuse_file *ff);
--
2.53.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-02-26 16:43 [PATCH v6 0/3] fuse: compound commands Horst Birthelmer
2026-02-26 16:43 ` [PATCH v6 1/3] fuse: add compound command to combine multiple requests Horst Birthelmer
2026-02-26 16:43 ` [PATCH v6 2/3] fuse: create helper functions for filling in fuse args for open and getattr Horst Birthelmer
@ 2026-02-26 16:43 ` Horst Birthelmer
2026-02-26 19:12 ` Joanne Koong
2 siblings, 1 reply; 31+ messages in thread
From: Horst Birthelmer @ 2026-02-26 16:43 UTC (permalink / raw)
To: Miklos Szeredi, Bernd Schubert, Joanne Koong, Luis Henriques
Cc: linux-kernel, linux-fsdevel, Horst Birthelmer
From: Horst Birthelmer <hbirthelmer@ddn.com>
The discussion about compound commands in fuse was
started over an argument to add a new operation that
will open a file and return its attributes in the same operation.
Here is a demonstration of that use case with compound commands.
Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
---
fs/fuse/file.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
fs/fuse/fuse_i.h | 4 +-
fs/fuse/ioctl.c | 2 +-
3 files changed, 99 insertions(+), 18 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -136,8 +136,71 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
}
}
+static int fuse_compound_open_getattr(struct fuse_mount *fm, u64 nodeid,
+ struct inode *inode, int flags, int opcode,
+ struct fuse_file *ff,
+ struct fuse_attr_out *outattrp,
+ struct fuse_open_out *outopenp)
+{
+ struct fuse_conn *fc = fm->fc;
+ struct fuse_compound_req *compound;
+ struct fuse_args open_args = {};
+ struct fuse_args getattr_args = {};
+ struct fuse_open_in open_in = {};
+ struct fuse_getattr_in getattr_in = {};
+ int err;
+
+ compound = fuse_compound_alloc(fm, 2, FUSE_COMPOUND_SEPARABLE);
+ if (!compound)
+ return -ENOMEM;
+
+ open_in.flags = flags & ~(O_CREAT | O_EXCL | O_NOCTTY);
+ if (!fm->fc->atomic_o_trunc)
+ open_in.flags &= ~O_TRUNC;
+
+ if (fm->fc->handle_killpriv_v2 &&
+ (open_in.flags & O_TRUNC) && !capable(CAP_FSETID))
+ open_in.open_flags |= FUSE_OPEN_KILL_SUIDGID;
+
+ fuse_open_args_fill(&open_args, nodeid, opcode, &open_in, outopenp);
+
+ err = fuse_compound_add(compound, &open_args, NULL);
+ if (err)
+ goto out;
+
+ fuse_getattr_args_fill(&getattr_args, nodeid, &getattr_in, outattrp);
+
+ err = fuse_compound_add(compound, &getattr_args, NULL);
+ if (err)
+ goto out;
+
+ err = fuse_compound_send(compound);
+ if (err)
+ goto out;
+
+ err = fuse_compound_get_error(compound, 0);
+ if (err)
+ goto out;
+
+ ff->fh = outopenp->fh;
+ ff->open_flags = outopenp->open_flags;
+
+ err = fuse_compound_get_error(compound, 1);
+ if (err)
+ goto out;
+
+ fuse_change_attributes(inode, &outattrp->attr, NULL,
+ ATTR_TIMEOUT(outattrp),
+ fuse_get_attr_version(fc));
+
+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;
@@ -163,23 +226,40 @@ 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;
- 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 (inode) {
+ struct fuse_attr_out attr_outarg;
+
+ err = fuse_compound_open_getattr(fm, nodeid, inode,
+ open_flags, opcode, ff,
+ &attr_outarg, outargp);
+ }
+
+ if (err == -ENOSYS) {
+ err = fuse_send_open(fm, nodeid, open_flags, opcode,
+ outargp);
+ if (!err) {
+ ff->fh = outargp->fh;
+ ff->open_flags = outargp->open_flags;
+ }
+ }
+
+ 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;
}
}
}
@@ -195,11 +275,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 ff8222b66c4f7b04c0671a980237a43871affd0a..40409a4ab016a061eea20afee76c8a7fe9c15adb 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1588,7 +1588,9 @@ void fuse_file_io_release(struct fuse_file *ff, struct inode *inode);
/* file.c */
struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
- unsigned int open_flags, bool isdir);
+ struct inode *inode,
+ unsigned int open_flags,
+ bool isdir);
void fuse_file_release(struct inode *inode, struct fuse_file *ff,
unsigned int open_flags, fl_owner_t id, bool isdir);
diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
index fdc175e93f74743eb4d2e5a4bc688df1c62e64c4..07a02e47b2c3a68633d213675a8cc380a0cf31d8 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.53.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-02-26 16:43 ` [PATCH v6 3/3] fuse: add an implementation of open+getattr Horst Birthelmer
@ 2026-02-26 19:12 ` Joanne Koong
2026-02-27 7:48 ` Horst Birthelmer
0 siblings, 1 reply; 31+ messages in thread
From: Joanne Koong @ 2026-02-26 19:12 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Miklos Szeredi, Bernd Schubert, Luis Henriques, linux-kernel,
linux-fsdevel, Horst Birthelmer
On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>
> From: Horst Birthelmer <hbirthelmer@ddn.com>
>
> The discussion about compound commands in fuse was
> started over an argument to add a new operation that
> will open a file and return its attributes in the same operation.
>
> Here is a demonstration of that use case with compound commands.
>
> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> ---
> fs/fuse/file.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> fs/fuse/fuse_i.h | 4 +-
> fs/fuse/ioctl.c | 2 +-
> 3 files changed, 99 insertions(+), 18 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -136,8 +136,71 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
> }
> }
>
> +static int fuse_compound_open_getattr(struct fuse_mount *fm, u64 nodeid,
> + struct inode *inode, int flags, int opcode,
> + struct fuse_file *ff,
> + struct fuse_attr_out *outattrp,
> + struct fuse_open_out *outopenp)
> +{
> + struct fuse_conn *fc = fm->fc;
> + struct fuse_compound_req *compound;
> + struct fuse_args open_args = {};
> + struct fuse_args getattr_args = {};
> + struct fuse_open_in open_in = {};
> + struct fuse_getattr_in getattr_in = {};
> + int err;
> +
> + compound = fuse_compound_alloc(fm, 2, FUSE_COMPOUND_SEPARABLE);
> + if (!compound)
> + return -ENOMEM;
> +
> + 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;
Do you think it makes sense to move this chunk of logic into
fuse_open_args_fill() since this logic has to be done in
fuse_send_open() as well?
> +
> + fuse_open_args_fill(&open_args, nodeid, opcode, &open_in, outopenp);
> +
> + err = fuse_compound_add(compound, &open_args, NULL);
> + if (err)
> + goto out;
> +
> + fuse_getattr_args_fill(&getattr_args, nodeid, &getattr_in, outattrp);
> +
> + err = fuse_compound_add(compound, &getattr_args, NULL);
> + if (err)
> + goto out;
> +
> + err = fuse_compound_send(compound);
> + if (err)
> + goto out;
> +
> + err = fuse_compound_get_error(compound, 0);
> + if (err)
> + goto out;
> +
> + ff->fh = outopenp->fh;
> + ff->open_flags = outopenp->open_flags;
It looks like this logic is shared between here and the non-compound
open path, maybe a bit better to just do this in fuse_file_open()
instead? That way we also don't need to pass the struct fuse_file *ff
as an arg either.
> +
> + err = fuse_compound_get_error(compound, 1);
> + if (err)
> + goto out;
For this open+getattr case, if getattr fails but the open succeeds,
should this still succeed the open since they're separable requests? I
think we had a conversation about it in v4, but imo this case should.
> +
> + fuse_change_attributes(inode, &outattrp->attr, NULL,
> + ATTR_TIMEOUT(outattrp),
> + fuse_get_attr_version(fc));
> +
> +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,
As I understand it, now every open() is a opengetattr() (except for
the ioctl path) but is this the desired behavior? for example if there
was a previous FUSE_LOOKUP that was just done, doesn't this mean
there's no getattr that's needed since the lookup refreshed the attrs?
or if the server has reasonable entry_valid and attr_valid timeouts,
multiple opens() of the same file would only need to send FUSE_OPEN
and not the FUSE_GETATTR, no?
> + unsigned int open_flags, bool isdir)
> {
> struct fuse_conn *fc = fm->fc;
> struct fuse_file *ff;
> @@ -163,23 +226,40 @@ 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;
>
> - 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 (inode) {
> + struct fuse_attr_out attr_outarg;
> +
> + err = fuse_compound_open_getattr(fm, nodeid, inode,
> + open_flags, opcode, ff,
> + &attr_outarg, outargp);
instead of passing in &attr_outarg, what about just having that moved
to fuse_compound_open_getattr()?
> + }
> +
> + if (err == -ENOSYS) {
> + err = fuse_send_open(fm, nodeid, open_flags, opcode,
> + outargp);
> + if (!err) {
> + ff->fh = outargp->fh;
> + ff->open_flags = outargp->open_flags;
> + }
> + }
> +
> + 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;
kfree(ff->args) and ff->args = NULL should not be called for the
!isdir case or it leads to the deadlock that was fixed in
https://lore.kernel.org/linux-fsdevel/20251010220738.3674538-2-joannelkoong@gmail.com/
I think if you have the "ff->fh = outargp..." and "ff->open_flags =
..." logic shared between fuse_compound_open_getattr() and
fuse_send_open() then the original errorr handling for this could just
be left as-is.
Thanks,
Joanne
> }
> }
> }
> @@ -195,11 +275,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 ff8222b66c4f7b04c0671a980237a43871affd0a..40409a4ab016a061eea20afee76c8a7fe9c15adb 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1588,7 +1588,9 @@ void fuse_file_io_release(struct fuse_file *ff, struct inode *inode);
>
> /* file.c */
> struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> - unsigned int open_flags, bool isdir);
> + struct inode *inode,
> + unsigned int open_flags,
> + bool isdir);
> void fuse_file_release(struct inode *inode, struct fuse_file *ff,
> unsigned int open_flags, fl_owner_t id, bool isdir);
>
> diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
> index fdc175e93f74743eb4d2e5a4bc688df1c62e64c4..07a02e47b2c3a68633d213675a8cc380a0cf31d8 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.53.0
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-02-26 19:12 ` Joanne Koong
@ 2026-02-27 7:48 ` Horst Birthelmer
2026-02-27 17:51 ` Joanne Koong
0 siblings, 1 reply; 31+ messages in thread
From: Horst Birthelmer @ 2026-02-27 7:48 UTC (permalink / raw)
To: Joanne Koong
Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >
> > From: Horst Birthelmer <hbirthelmer@ddn.com>
> >
> > The discussion about compound commands in fuse was
> > started over an argument to add a new operation that
> > will open a file and return its attributes in the same operation.
> >
> > Here is a demonstration of that use case with compound commands.
> >
> > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > ---
> > fs/fuse/file.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > fs/fuse/fuse_i.h | 4 +-
> > fs/fuse/ioctl.c | 2 +-
> > 3 files changed, 99 insertions(+), 18 deletions(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -136,8 +136,71 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
> > }
> > }
> >
> > +static int fuse_compound_open_getattr(struct fuse_mount *fm, u64 nodeid,
> > + struct inode *inode, int flags, int opcode,
> > + struct fuse_file *ff,
> > + struct fuse_attr_out *outattrp,
> > + struct fuse_open_out *outopenp)
> > +{
> > + struct fuse_conn *fc = fm->fc;
> > + struct fuse_compound_req *compound;
> > + struct fuse_args open_args = {};
> > + struct fuse_args getattr_args = {};
> > + struct fuse_open_in open_in = {};
> > + struct fuse_getattr_in getattr_in = {};
> > + int err;
> > +
> > + compound = fuse_compound_alloc(fm, 2, FUSE_COMPOUND_SEPARABLE);
> > + if (!compound)
> > + return -ENOMEM;
> > +
> > + 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;
>
> Do you think it makes sense to move this chunk of logic into
> fuse_open_args_fill() since this logic has to be done in
> fuse_send_open() as well?
>
Yes, I think that makes sense and would be beneficial to other requests in
other compounds that will be constructed with that function.
> > +
> > + fuse_open_args_fill(&open_args, nodeid, opcode, &open_in, outopenp);
> > +
> > + err = fuse_compound_add(compound, &open_args, NULL);
> > + if (err)
> > + goto out;
> > +
> > + fuse_getattr_args_fill(&getattr_args, nodeid, &getattr_in, outattrp);
> > +
> > + err = fuse_compound_add(compound, &getattr_args, NULL);
> > + if (err)
> > + goto out;
> > +
> > + err = fuse_compound_send(compound);
> > + if (err)
> > + goto out;
> > +
> > + err = fuse_compound_get_error(compound, 0);
> > + if (err)
> > + goto out;
> > +
> > + ff->fh = outopenp->fh;
> > + ff->open_flags = outopenp->open_flags;
>
> It looks like this logic is shared between here and the non-compound
> open path, maybe a bit better to just do this in fuse_file_open()
> instead? That way we also don't need to pass the struct fuse_file *ff
> as an arg either.
>
Will do that.
> > +
> > + err = fuse_compound_get_error(compound, 1);
> > + if (err)
> > + goto out;
>
> For this open+getattr case, if getattr fails but the open succeeds,
> should this still succeed the open since they're separable requests? I
> think we had a conversation about it in v4, but imo this case should.
>
You are right, we had the conversation and other people joined, so I
changed this code but to something else. Sorry about that.
I think your idea will work, since the behavior then is exactly what happens
at the moment with exactly the same drawback.
> > +
> > + fuse_change_attributes(inode, &outattrp->attr, NULL,
> > + ATTR_TIMEOUT(outattrp),
> > + fuse_get_attr_version(fc));
> > +
> > +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,
>
> As I understand it, now every open() is a opengetattr() (except for
> the ioctl path) but is this the desired behavior? for example if there
> was a previous FUSE_LOOKUP that was just done, doesn't this mean
> there's no getattr that's needed since the lookup refreshed the attrs?
> or if the server has reasonable entry_valid and attr_valid timeouts,
> multiple opens() of the same file would only need to send FUSE_OPEN
> and not the FUSE_GETATTR, no?
So your concern is, that we send too many requests?
If the fuse server implwments the compound that is not the case.
>
>
> > + unsigned int open_flags, bool isdir)
> > {
> > struct fuse_conn *fc = fm->fc;
> > struct fuse_file *ff;
> > @@ -163,23 +226,40 @@ 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;
> >
> > - 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 (inode) {
> > + struct fuse_attr_out attr_outarg;
> > +
> > + err = fuse_compound_open_getattr(fm, nodeid, inode,
> > + open_flags, opcode, ff,
> > + &attr_outarg, outargp);
>
> instead of passing in &attr_outarg, what about just having that moved
> to fuse_compound_open_getattr()?
>
This is a victim of 'code move' already.
I had the code to handle the outarg here before and did not change the functions
signature, which now looks stupid.
> > + }
> > +
> > + if (err == -ENOSYS) {
> > + err = fuse_send_open(fm, nodeid, open_flags, opcode,
> > + outargp);
> > + if (!err) {
> > + ff->fh = outargp->fh;
> > + ff->open_flags = outargp->open_flags;
> > + }
> > + }
> > +
> > + 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;
>
> kfree(ff->args) and ff->args = NULL should not be called for the
> !isdir case or it leads to the deadlock that was fixed in
> https://lore.kernel.org/linux-fsdevel/20251010220738.3674538-2-joannelkoong@gmail.com/
>
> I think if you have the "ff->fh = outargp..." and "ff->open_flags =
> ..." logic shared between fuse_compound_open_getattr() and
> fuse_send_open() then the original errorr handling for this could just
> be left as-is.
Very good argument to share the error handling then ...
>
> Thanks,
> Joanne
>
Thanks for taking the time,
Horst
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-02-27 7:48 ` Horst Birthelmer
@ 2026-02-27 17:51 ` Joanne Koong
2026-02-27 18:07 ` Joanne Koong
0 siblings, 1 reply; 31+ messages in thread
From: Joanne Koong @ 2026-02-27 17:51 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
>
> On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> > On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> > >
> > > From: Horst Birthelmer <hbirthelmer@ddn.com>
> > >
> > > The discussion about compound commands in fuse was
> > > started over an argument to add a new operation that
> > > will open a file and return its attributes in the same operation.
> > >
> > > Here is a demonstration of that use case with compound commands.
> > >
> > > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > > ---
> > > fs/fuse/file.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > > fs/fuse/fuse_i.h | 4 +-
> > > fs/fuse/ioctl.c | 2 +-
> > > 3 files changed, 99 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > > index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> > > --- a/fs/fuse/file.c
> > > +++ b/fs/fuse/file.c
> > > struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> > > - unsigned int open_flags, bool isdir)
> > > + struct inode *inode,
> >
> > As I understand it, now every open() is a opengetattr() (except for
> > the ioctl path) but is this the desired behavior? for example if there
> > was a previous FUSE_LOOKUP that was just done, doesn't this mean
> > there's no getattr that's needed since the lookup refreshed the attrs?
> > or if the server has reasonable entry_valid and attr_valid timeouts,
> > multiple opens() of the same file would only need to send FUSE_OPEN
> > and not the FUSE_GETATTR, no?
>
> So your concern is, that we send too many requests?
> If the fuse server implwments the compound that is not the case.
>
My concern is that we're adding unnecessary overhead for every open
when in most cases, the attributes are already uptodate. I don't think
we can assume that the server always has attributes locally cached, so
imo the extra getattr is nontrivial (eg might require having to
stat()).
Thanks,
Joanne
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-02-27 17:51 ` Joanne Koong
@ 2026-02-27 18:07 ` Joanne Koong
2026-02-28 8:14 ` Horst Birthelmer
0 siblings, 1 reply; 31+ messages in thread
From: Joanne Koong @ 2026-02-27 18:07 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
> >
> > On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> > > On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> > > >
> > > > From: Horst Birthelmer <hbirthelmer@ddn.com>
> > > >
> > > > The discussion about compound commands in fuse was
> > > > started over an argument to add a new operation that
> > > > will open a file and return its attributes in the same operation.
> > > >
> > > > Here is a demonstration of that use case with compound commands.
> > > >
> > > > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > > > ---
> > > > fs/fuse/file.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > > > fs/fuse/fuse_i.h | 4 +-
> > > > fs/fuse/ioctl.c | 2 +-
> > > > 3 files changed, 99 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > > > index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> > > > --- a/fs/fuse/file.c
> > > > +++ b/fs/fuse/file.c
> > > > struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> > > > - unsigned int open_flags, bool isdir)
> > > > + struct inode *inode,
> > >
> > > As I understand it, now every open() is a opengetattr() (except for
> > > the ioctl path) but is this the desired behavior? for example if there
> > > was a previous FUSE_LOOKUP that was just done, doesn't this mean
> > > there's no getattr that's needed since the lookup refreshed the attrs?
> > > or if the server has reasonable entry_valid and attr_valid timeouts,
> > > multiple opens() of the same file would only need to send FUSE_OPEN
> > > and not the FUSE_GETATTR, no?
> >
> > So your concern is, that we send too many requests?
> > If the fuse server implwments the compound that is not the case.
> >
>
> My concern is that we're adding unnecessary overhead for every open
> when in most cases, the attributes are already uptodate. I don't think
> we can assume that the server always has attributes locally cached, so
> imo the extra getattr is nontrivial (eg might require having to
> stat()).
Looking at where the attribute valid time gets set... it looks like
this gets stored in fi->i_time (as per
fuse_change_attributes_common()), so maybe it's better to only send
the compound open+getattr if time_before64(fi->i_time,
get_jiffies_64()) is true, otherwise only the open is needed. This
doesn't solve the O_APPEND data corruption bug seen in [1] but imo
this would be a more preferable way of doing it.
Thanks,
Joanne
[1] https://lore.kernel.org/linux-fsdevel/20240813212149.1909627-1-joannelkoong@gmail.com/
>
> Thanks,
> Joanne
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-02-27 18:07 ` Joanne Koong
@ 2026-02-28 8:14 ` Horst Birthelmer
2026-03-02 18:56 ` Joanne Koong
0 siblings, 1 reply; 31+ messages in thread
From: Horst Birthelmer @ 2026-02-28 8:14 UTC (permalink / raw)
To: Joanne Koong
Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Fri, Feb 27, 2026 at 10:07:20AM -0800, Joanne Koong wrote:
> On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
> >
> > On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
> > >
> > > On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> > > > On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> > > > >
> > > > > From: Horst Birthelmer <hbirthelmer@ddn.com>
> > > > >
> > > > > The discussion about compound commands in fuse was
> > > > > started over an argument to add a new operation that
> > > > > will open a file and return its attributes in the same operation.
> > > > >
> > > > > Here is a demonstration of that use case with compound commands.
> > > > >
> > > > > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > > > > ---
> > > > > fs/fuse/file.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > > > > fs/fuse/fuse_i.h | 4 +-
> > > > > fs/fuse/ioctl.c | 2 +-
> > > > > 3 files changed, 99 insertions(+), 18 deletions(-)
> > > > >
> > > > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > > > > index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> > > > > --- a/fs/fuse/file.c
> > > > > +++ b/fs/fuse/file.c
> > > > > struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> > > > > - unsigned int open_flags, bool isdir)
> > > > > + struct inode *inode,
> > > >
> > > > As I understand it, now every open() is a opengetattr() (except for
> > > > the ioctl path) but is this the desired behavior? for example if there
> > > > was a previous FUSE_LOOKUP that was just done, doesn't this mean
> > > > there's no getattr that's needed since the lookup refreshed the attrs?
> > > > or if the server has reasonable entry_valid and attr_valid timeouts,
> > > > multiple opens() of the same file would only need to send FUSE_OPEN
> > > > and not the FUSE_GETATTR, no?
> > >
> > > So your concern is, that we send too many requests?
> > > If the fuse server implwments the compound that is not the case.
> > >
> >
> > My concern is that we're adding unnecessary overhead for every open
> > when in most cases, the attributes are already uptodate. I don't think
> > we can assume that the server always has attributes locally cached, so
> > imo the extra getattr is nontrivial (eg might require having to
> > stat()).
>
> Looking at where the attribute valid time gets set... it looks like
> this gets stored in fi->i_time (as per
> fuse_change_attributes_common()), so maybe it's better to only send
> the compound open+getattr if time_before64(fi->i_time,
> get_jiffies_64()) is true, otherwise only the open is needed. This
> doesn't solve the O_APPEND data corruption bug seen in [1] but imo
> this would be a more preferable way of doing it.
>
Don't take this as an objection. I'm looking for arguments, since my defense
was always the line I used above (if the fuse server implements the compound,
it's one call).
What made you change your mind from avoiding the data corruption to worry
about the number of stat calls done? Since you were the one reporting the
issue and even providing a fix.
ATM I would rather have data consistency than fewer requests during open.
> Thanks,
> Joanne
>
> [1] https://lore.kernel.org/linux-fsdevel/20240813212149.1909627-1-joannelkoong@gmail.com/
>
> >
> > Thanks,
> > Joanne
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-02-28 8:14 ` Horst Birthelmer
@ 2026-03-02 18:56 ` Joanne Koong
2026-03-02 20:03 ` Bernd Schubert
0 siblings, 1 reply; 31+ messages in thread
From: Joanne Koong @ 2026-03-02 18:56 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Horst Birthelmer, Miklos Szeredi, Bernd Schubert, Luis Henriques,
linux-kernel, linux-fsdevel, Horst Birthelmer
On Sat, Feb 28, 2026 at 12:14 AM Horst Birthelmer <horst@birthelmer.de> wrote:
>
> On Fri, Feb 27, 2026 at 10:07:20AM -0800, Joanne Koong wrote:
> > On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
> > >
> > > On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
> > > >
> > > > On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> > > > > On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> > > > > >
> > > > > > From: Horst Birthelmer <hbirthelmer@ddn.com>
> > > > > >
> > > > > > The discussion about compound commands in fuse was
> > > > > > started over an argument to add a new operation that
> > > > > > will open a file and return its attributes in the same operation.
> > > > > >
> > > > > > Here is a demonstration of that use case with compound commands.
> > > > > >
> > > > > > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > > > > > ---
> > > > > > fs/fuse/file.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > > > > > fs/fuse/fuse_i.h | 4 +-
> > > > > > fs/fuse/ioctl.c | 2 +-
> > > > > > 3 files changed, 99 insertions(+), 18 deletions(-)
> > > > > >
> > > > > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > > > > > index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> > > > > > --- a/fs/fuse/file.c
> > > > > > +++ b/fs/fuse/file.c
> > > > > > struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> > > > > > - unsigned int open_flags, bool isdir)
> > > > > > + struct inode *inode,
> > > > >
> > > > > As I understand it, now every open() is a opengetattr() (except for
> > > > > the ioctl path) but is this the desired behavior? for example if there
> > > > > was a previous FUSE_LOOKUP that was just done, doesn't this mean
> > > > > there's no getattr that's needed since the lookup refreshed the attrs?
> > > > > or if the server has reasonable entry_valid and attr_valid timeouts,
> > > > > multiple opens() of the same file would only need to send FUSE_OPEN
> > > > > and not the FUSE_GETATTR, no?
> > > >
> > > > So your concern is, that we send too many requests?
> > > > If the fuse server implwments the compound that is not the case.
> > > >
> > >
> > > My concern is that we're adding unnecessary overhead for every open
> > > when in most cases, the attributes are already uptodate. I don't think
> > > we can assume that the server always has attributes locally cached, so
> > > imo the extra getattr is nontrivial (eg might require having to
> > > stat()).
> >
> > Looking at where the attribute valid time gets set... it looks like
> > this gets stored in fi->i_time (as per
> > fuse_change_attributes_common()), so maybe it's better to only send
> > the compound open+getattr if time_before64(fi->i_time,
> > get_jiffies_64()) is true, otherwise only the open is needed. This
> > doesn't solve the O_APPEND data corruption bug seen in [1] but imo
> > this would be a more preferable way of doing it.
> >
> Don't take this as an objection. I'm looking for arguments, since my defense
> was always the line I used above (if the fuse server implements the compound,
> it's one call).
The overhead for the server to fetch the attributes may be nontrivial
(eg may require stat()). I really don't think we can assume the data
is locally cached somewhere. Why always compound the getattr to the
open instead of only compounding the getattr when the attributes are
actually invalid?
But maybe I'm wrong here and this is the preferable way of doing it.
Miklos, could you provide your input on this?
>
> What made you change your mind from avoiding the data corruption to worry
> about the number of stat calls done? Since you were the one reporting the
> issue and even providing a fix.
imo I think the O_APPEND fix should be something like:
if ((open_flags & O_APPEND) || time_before64(fi->i_time, get_jiffies_64()))
send FUSE_OPEN + FUSE_GETATTR compound
else
send FUSE_OPEN only
since the O_APPEND case specifically needs accurate file size for the
append offset.
Thanks,
Joanne
>
> ATM I would rather have data consistency than fewer requests during open.
>
> > Thanks,
> > Joanne
> >
> > [1] https://lore.kernel.org/linux-fsdevel/20240813212149.1909627-1-joannelkoong@gmail.com/
> >
> > >
> > > Thanks,
> > > Joanne
> >
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-03-02 18:56 ` Joanne Koong
@ 2026-03-02 20:03 ` Bernd Schubert
2026-03-03 5:06 ` Darrick J. Wong
0 siblings, 1 reply; 31+ messages in thread
From: Bernd Schubert @ 2026-03-02 20:03 UTC (permalink / raw)
To: Joanne Koong, Horst Birthelmer
Cc: Horst Birthelmer, Miklos Szeredi, Luis Henriques, linux-kernel,
linux-fsdevel, Horst Birthelmer
On 3/2/26 19:56, Joanne Koong wrote:
> On Sat, Feb 28, 2026 at 12:14 AM Horst Birthelmer <horst@birthelmer.de> wrote:
>>
>> On Fri, Feb 27, 2026 at 10:07:20AM -0800, Joanne Koong wrote:
>>> On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
>>>>
>>>> On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
>>>>>
>>>>> On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
>>>>>> On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>>>>>>>
>>>>>>> From: Horst Birthelmer <hbirthelmer@ddn.com>
>>>>>>>
>>>>>>> The discussion about compound commands in fuse was
>>>>>>> started over an argument to add a new operation that
>>>>>>> will open a file and return its attributes in the same operation.
>>>>>>>
>>>>>>> Here is a demonstration of that use case with compound commands.
>>>>>>>
>>>>>>> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
>>>>>>> ---
>>>>>>> fs/fuse/file.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
>>>>>>> fs/fuse/fuse_i.h | 4 +-
>>>>>>> fs/fuse/ioctl.c | 2 +-
>>>>>>> 3 files changed, 99 insertions(+), 18 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>>>>>>> index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
>>>>>>> --- a/fs/fuse/file.c
>>>>>>> +++ b/fs/fuse/file.c
>>>>>>> struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
>>>>>>> - unsigned int open_flags, bool isdir)
>>>>>>> + struct inode *inode,
>>>>>>
>>>>>> As I understand it, now every open() is a opengetattr() (except for
>>>>>> the ioctl path) but is this the desired behavior? for example if there
>>>>>> was a previous FUSE_LOOKUP that was just done, doesn't this mean
>>>>>> there's no getattr that's needed since the lookup refreshed the attrs?
>>>>>> or if the server has reasonable entry_valid and attr_valid timeouts,
>>>>>> multiple opens() of the same file would only need to send FUSE_OPEN
>>>>>> and not the FUSE_GETATTR, no?
>>>>>
>>>>> So your concern is, that we send too many requests?
>>>>> If the fuse server implwments the compound that is not the case.
>>>>>
>>>>
>>>> My concern is that we're adding unnecessary overhead for every open
>>>> when in most cases, the attributes are already uptodate. I don't think
>>>> we can assume that the server always has attributes locally cached, so
>>>> imo the extra getattr is nontrivial (eg might require having to
>>>> stat()).
>>>
>>> Looking at where the attribute valid time gets set... it looks like
>>> this gets stored in fi->i_time (as per
>>> fuse_change_attributes_common()), so maybe it's better to only send
>>> the compound open+getattr if time_before64(fi->i_time,
>>> get_jiffies_64()) is true, otherwise only the open is needed. This
>>> doesn't solve the O_APPEND data corruption bug seen in [1] but imo
>>> this would be a more preferable way of doing it.
>>>
>> Don't take this as an objection. I'm looking for arguments, since my defense
>> was always the line I used above (if the fuse server implements the compound,
>> it's one call).
>
> The overhead for the server to fetch the attributes may be nontrivial
> (eg may require stat()). I really don't think we can assume the data
> is locally cached somewhere. Why always compound the getattr to the
> open instead of only compounding the getattr when the attributes are
> actually invalid?
>
> But maybe I'm wrong here and this is the preferable way of doing it.
> Miklos, could you provide your input on this?
Personally I would see it as change of behavior if out of the sudden
open is followed by getattr. In my opinion fuse server needs to make a
decision that it wants that. Let's take my favorite sshfs example with a
1s latency - it be very noticeable if open would get slowed down by
factor 2.
Thanks,
Bernd
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-03-02 20:03 ` Bernd Schubert
@ 2026-03-03 5:06 ` Darrick J. Wong
2026-03-03 7:26 ` Horst Birthelmer
2026-03-03 10:03 ` Miklos Szeredi
0 siblings, 2 replies; 31+ messages in thread
From: Darrick J. Wong @ 2026-03-03 5:06 UTC (permalink / raw)
To: Bernd Schubert
Cc: Joanne Koong, Horst Birthelmer, Horst Birthelmer, Miklos Szeredi,
Luis Henriques, linux-kernel, linux-fsdevel, Horst Birthelmer
On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
>
>
> On 3/2/26 19:56, Joanne Koong wrote:
> > On Sat, Feb 28, 2026 at 12:14 AM Horst Birthelmer <horst@birthelmer.de> wrote:
> >>
> >> On Fri, Feb 27, 2026 at 10:07:20AM -0800, Joanne Koong wrote:
> >>> On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
> >>>>
> >>>> On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
> >>>>>
> >>>>> On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> >>>>>> On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >>>>>>>
> >>>>>>> From: Horst Birthelmer <hbirthelmer@ddn.com>
> >>>>>>>
> >>>>>>> The discussion about compound commands in fuse was
> >>>>>>> started over an argument to add a new operation that
> >>>>>>> will open a file and return its attributes in the same operation.
> >>>>>>>
> >>>>>>> Here is a demonstration of that use case with compound commands.
> >>>>>>>
> >>>>>>> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> >>>>>>> ---
> >>>>>>> fs/fuse/file.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> >>>>>>> fs/fuse/fuse_i.h | 4 +-
> >>>>>>> fs/fuse/ioctl.c | 2 +-
> >>>>>>> 3 files changed, 99 insertions(+), 18 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> >>>>>>> index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> >>>>>>> --- a/fs/fuse/file.c
> >>>>>>> +++ b/fs/fuse/file.c
> >>>>>>> struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> >>>>>>> - unsigned int open_flags, bool isdir)
> >>>>>>> + struct inode *inode,
> >>>>>>
> >>>>>> As I understand it, now every open() is a opengetattr() (except for
> >>>>>> the ioctl path) but is this the desired behavior? for example if there
> >>>>>> was a previous FUSE_LOOKUP that was just done, doesn't this mean
> >>>>>> there's no getattr that's needed since the lookup refreshed the attrs?
> >>>>>> or if the server has reasonable entry_valid and attr_valid timeouts,
> >>>>>> multiple opens() of the same file would only need to send FUSE_OPEN
> >>>>>> and not the FUSE_GETATTR, no?
> >>>>>
> >>>>> So your concern is, that we send too many requests?
> >>>>> If the fuse server implwments the compound that is not the case.
> >>>>>
> >>>>
> >>>> My concern is that we're adding unnecessary overhead for every open
> >>>> when in most cases, the attributes are already uptodate. I don't think
> >>>> we can assume that the server always has attributes locally cached, so
> >>>> imo the extra getattr is nontrivial (eg might require having to
> >>>> stat()).
> >>>
> >>> Looking at where the attribute valid time gets set... it looks like
> >>> this gets stored in fi->i_time (as per
> >>> fuse_change_attributes_common()), so maybe it's better to only send
> >>> the compound open+getattr if time_before64(fi->i_time,
> >>> get_jiffies_64()) is true, otherwise only the open is needed. This
> >>> doesn't solve the O_APPEND data corruption bug seen in [1] but imo
> >>> this would be a more preferable way of doing it.
/me notes that NFS can corrupt O_APPEND writes if you're not careful to
synchronize writers at the application level...
> >> Don't take this as an objection. I'm looking for arguments, since my defense
> >> was always the line I used above (if the fuse server implements the compound,
> >> it's one call).
> >
> > The overhead for the server to fetch the attributes may be nontrivial
> > (eg may require stat()). I really don't think we can assume the data
> > is locally cached somewhere. Why always compound the getattr to the
> > open instead of only compounding the getattr when the attributes are
> > actually invalid?
> >
> > But maybe I'm wrong here and this is the preferable way of doing it.
> > Miklos, could you provide your input on this?
>
> Personally I would see it as change of behavior if out of the sudden
> open is followed by getattr. In my opinion fuse server needs to make a
> decision that it wants that. Let's take my favorite sshfs example with a
> 1s latency - it be very noticeable if open would get slowed down by
> factor 2.
I wonder, since O_APPEND writes supposedly reposition the file position
to i_size before every write, can we enlarge the write reply so that the
fuse server could tell the client what i_size is supposed to be after
every write? Or perhaps add a notification so a network filesystem
could try to keep the kernel uptodate after another node appends to a
file?
Just my unqualified opinion ;)
--D
> Thanks,
> Bernd
>
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-03-03 5:06 ` Darrick J. Wong
@ 2026-03-03 7:26 ` Horst Birthelmer
2026-03-03 10:03 ` Miklos Szeredi
1 sibling, 0 replies; 31+ messages in thread
From: Horst Birthelmer @ 2026-03-03 7:26 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Bernd Schubert, Joanne Koong, Horst Birthelmer, Miklos Szeredi,
Luis Henriques, linux-kernel, linux-fsdevel, Horst Birthelmer
On Mon, Mar 02, 2026 at 09:06:14PM -0800, Darrick J. Wong wrote:
> On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > On 3/2/26 19:56, Joanne Koong wrote:
> > > On Sat, Feb 28, 2026 at 12:14 AM Horst Birthelmer <horst@birthelmer.de> wrote:
> > >>
> > >> On Fri, Feb 27, 2026 at 10:07:20AM -0800, Joanne Koong wrote:
> > >>> On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
> > >>>>
> > >>>> On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
> > >>>>>
> > >>>>> On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> > >>>>>> On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> > >>>>>>>
> > >>>>>>> From: Horst Birthelmer <hbirthelmer@ddn.com>
> > >>>>>>>
> > >>>>>>> The discussion about compound commands in fuse was
> > >>>>>>> started over an argument to add a new operation that
> > >>>>>>> will open a file and return its attributes in the same operation.
> > >>>>>>>
> > >>>>>>> Here is a demonstration of that use case with compound commands.
> > >>>>>>>
> > >>>>>>> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > >>>>>>> ---
> > >>>>>>> fs/fuse/file.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > >>>>>>> fs/fuse/fuse_i.h | 4 +-
> > >>>>>>> fs/fuse/ioctl.c | 2 +-
> > >>>>>>> 3 files changed, 99 insertions(+), 18 deletions(-)
> > >>>>>>>
...
> > >
> > > The overhead for the server to fetch the attributes may be nontrivial
> > > (eg may require stat()). I really don't think we can assume the data
> > > is locally cached somewhere. Why always compound the getattr to the
> > > open instead of only compounding the getattr when the attributes are
> > > actually invalid?
> > >
> > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > Miklos, could you provide your input on this?
> >
> > Personally I would see it as change of behavior if out of the sudden
> > open is followed by getattr. In my opinion fuse server needs to make a
> > decision that it wants that. Let's take my favorite sshfs example with a
> > 1s latency - it be very noticeable if open would get slowed down by
> > factor 2.
>
> I wonder, since O_APPEND writes supposedly reposition the file position
> to i_size before every write, can we enlarge the write reply so that the
> fuse server could tell the client what i_size is supposed to be after
> every write? Or perhaps add a notification so a network filesystem
> could try to keep the kernel uptodate after another node appends to a
> file?
>
Bernd already had that idea, that we add an optional getattr to a write
in a compound.
If the fuse server supports it, you would get that data.
I'm really not happy about that idea, since we would have to support 'pages'
then for the compound.
Regarding the notification. Isn't that doable with the current code already?
> Just my unqualified opinion ;)
>
> --D
>
> > Thanks,
> > Bernd
Thanks,
Horst
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-03-03 5:06 ` Darrick J. Wong
2026-03-03 7:26 ` Horst Birthelmer
@ 2026-03-03 10:03 ` Miklos Szeredi
2026-03-03 10:38 ` Horst Birthelmer
2026-03-03 23:13 ` Joanne Koong
1 sibling, 2 replies; 31+ messages in thread
From: Miklos Szeredi @ 2026-03-03 10:03 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Bernd Schubert, Joanne Koong, Horst Birthelmer, Horst Birthelmer,
Luis Henriques, linux-kernel, linux-fsdevel, Horst Birthelmer
On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> >
> > On 3/2/26 19:56, Joanne Koong wrote:
> > > The overhead for the server to fetch the attributes may be nontrivial
> > > (eg may require stat()). I really don't think we can assume the data
> > > is locally cached somewhere. Why always compound the getattr to the
> > > open instead of only compounding the getattr when the attributes are
> > > actually invalid?
> > >
> > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > Miklos, could you provide your input on this?
Yes, it makes sense to refresh attributes only when necessary.
> I wonder, since O_APPEND writes supposedly reposition the file position
> to i_size before every write, can we enlarge the write reply so that the
> fuse server could tell the client what i_size is supposed to be after
> every write? Or perhaps add a notification so a network filesystem
> could try to keep the kernel uptodate after another node appends to a
> file?
This can be done with FUSE_NOTIFY_INVAL_INODE.
Still racy. If need to have perfect O_APPEND semantics,
FOPEN_DIRECT_IO is currently the only option.
It would be nice to have some sort of delegation/lease mechanism in
the fuse protocol to allow caching when remote is not modifying (which
is the common case usually) but force uncached I/O when there's
concurrent modification.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-03-03 10:03 ` Miklos Szeredi
@ 2026-03-03 10:38 ` Horst Birthelmer
2026-03-03 21:19 ` Joanne Koong
2026-03-03 23:13 ` Joanne Koong
1 sibling, 1 reply; 31+ messages in thread
From: Horst Birthelmer @ 2026-03-03 10:38 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Darrick J. Wong, Bernd Schubert, Joanne Koong, Horst Birthelmer,
Luis Henriques, linux-kernel, linux-fsdevel, Horst Birthelmer
On Tue, Mar 03, 2026 at 11:03:14AM +0100, Miklos Szeredi wrote:
> On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > >
> > > On 3/2/26 19:56, Joanne Koong wrote:
>
> > > > The overhead for the server to fetch the attributes may be nontrivial
> > > > (eg may require stat()). I really don't think we can assume the data
> > > > is locally cached somewhere. Why always compound the getattr to the
> > > > open instead of only compounding the getattr when the attributes are
> > > > actually invalid?
> > > >
> > > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > > Miklos, could you provide your input on this?
>
> Yes, it makes sense to refresh attributes only when necessary.
>
OK, I will add a 'compound flag' for optional operations and don't
execute those, when the fuse server does not support compounds.
This way we can always send the open+getattr, but if the fuse server
does not process this as a compound (aka. it would be costly to do it),
we only resend the FUSE_OPEN. Thus the current behavior does not change
and we can profit from fuse servers that actually have the possibility to
give us the attributes.
We can take a look at when to fetch the attributes in another context for
the other cases.
>
> Thanks,
> Miklos
>
Thanks,
Horst
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-03-03 10:38 ` Horst Birthelmer
@ 2026-03-03 21:19 ` Joanne Koong
2026-03-04 9:11 ` Horst Birthelmer
0 siblings, 1 reply; 31+ messages in thread
From: Joanne Koong @ 2026-03-03 21:19 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Miklos Szeredi, Darrick J. Wong, Bernd Schubert, Horst Birthelmer,
Luis Henriques, linux-kernel, linux-fsdevel, Horst Birthelmer
On Tue, Mar 3, 2026 at 2:39 AM Horst Birthelmer <horst@birthelmer.de> wrote:
>
> On Tue, Mar 03, 2026 at 11:03:14AM +0100, Miklos Szeredi wrote:
> > On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
> > >
> > > On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > > >
> > > > On 3/2/26 19:56, Joanne Koong wrote:
> >
> > > > > The overhead for the server to fetch the attributes may be nontrivial
> > > > > (eg may require stat()). I really don't think we can assume the data
> > > > > is locally cached somewhere. Why always compound the getattr to the
> > > > > open instead of only compounding the getattr when the attributes are
> > > > > actually invalid?
> > > > >
> > > > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > > > Miklos, could you provide your input on this?
> >
> > Yes, it makes sense to refresh attributes only when necessary.
> >
>
> OK, I will add a 'compound flag' for optional operations and don't
> execute those, when the fuse server does not support compounds.
>
> This way we can always send the open+getattr, but if the fuse server
> does not process this as a compound (aka. it would be costly to do it),
> we only resend the FUSE_OPEN. Thus the current behavior does not change
> and we can profit from fuse servers that actually have the possibility to
> give us the attributes.
in my opinion, this adds additional complexity for no real benefit. I
think we'll rarely hit a case where it'll be useful to speculatively
prefetch attributes that are already valid that is not already
accounted for by the attr_timeout the server set.
Thanks,
Joanne
>
> We can take a look at when to fetch the attributes in another context for
> the other cases.
>
> >
> > Thanks,
> > Miklos
> >
>
> Thanks,
> Horst
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-03-03 21:19 ` Joanne Koong
@ 2026-03-04 9:11 ` Horst Birthelmer
2026-03-04 21:42 ` Joanne Koong
0 siblings, 1 reply; 31+ messages in thread
From: Horst Birthelmer @ 2026-03-04 9:11 UTC (permalink / raw)
To: Joanne Koong
Cc: Miklos Szeredi, Darrick J. Wong, Bernd Schubert, Horst Birthelmer,
Luis Henriques, linux-kernel, linux-fsdevel, Horst Birthelmer
On Tue, Mar 03, 2026 at 01:19:43PM -0800, Joanne Koong wrote:
> On Tue, Mar 3, 2026 at 2:39 AM Horst Birthelmer <horst@birthelmer.de> wrote:
> >
> > On Tue, Mar 03, 2026 at 11:03:14AM +0100, Miklos Szeredi wrote:
> > > On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
> > > >
> > > > On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > > > >
> > > > > On 3/2/26 19:56, Joanne Koong wrote:
> > >
> > > > > > The overhead for the server to fetch the attributes may be nontrivial
> > > > > > (eg may require stat()). I really don't think we can assume the data
> > > > > > is locally cached somewhere. Why always compound the getattr to the
> > > > > > open instead of only compounding the getattr when the attributes are
> > > > > > actually invalid?
> > > > > >
> > > > > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > > > > Miklos, could you provide your input on this?
> > >
> > > Yes, it makes sense to refresh attributes only when necessary.
> > >
> >
> > OK, I will add a 'compound flag' for optional operations and don't
> > execute those, when the fuse server does not support compounds.
> >
> > This way we can always send the open+getattr, but if the fuse server
> > does not process this as a compound (aka. it would be costly to do it),
> > we only resend the FUSE_OPEN. Thus the current behavior does not change
> > and we can profit from fuse servers that actually have the possibility to
> > give us the attributes.
>
> in my opinion, this adds additional complexity for no real benefit. I
> think we'll rarely hit a case where it'll be useful to speculatively
> prefetch attributes that are already valid that is not already
> accounted for by the attr_timeout the server set.
>
So the current consensus would be to use the compound when we
either don't have the data, or it has become invalid,
and use the current behavior
(just do an open and don't worry about the stale attributes)
when we have unexpired attributes?
Thanks,
Horst
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-03-04 9:11 ` Horst Birthelmer
@ 2026-03-04 21:42 ` Joanne Koong
0 siblings, 0 replies; 31+ messages in thread
From: Joanne Koong @ 2026-03-04 21:42 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Miklos Szeredi, Darrick J. Wong, Bernd Schubert, Horst Birthelmer,
Luis Henriques, linux-kernel, linux-fsdevel, Horst Birthelmer
On Wed, Mar 4, 2026 at 1:11 AM Horst Birthelmer <horst@birthelmer.de> wrote:
>
> On Tue, Mar 03, 2026 at 01:19:43PM -0800, Joanne Koong wrote:
> > On Tue, Mar 3, 2026 at 2:39 AM Horst Birthelmer <horst@birthelmer.de> wrote:
> > >
> > > On Tue, Mar 03, 2026 at 11:03:14AM +0100, Miklos Szeredi wrote:
> > > > On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
> > > > >
> > > > > On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > > > > >
> > > > > > On 3/2/26 19:56, Joanne Koong wrote:
> > > >
> > > > > > > The overhead for the server to fetch the attributes may be nontrivial
> > > > > > > (eg may require stat()). I really don't think we can assume the data
> > > > > > > is locally cached somewhere. Why always compound the getattr to the
> > > > > > > open instead of only compounding the getattr when the attributes are
> > > > > > > actually invalid?
> > > > > > >
> > > > > > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > > > > > Miklos, could you provide your input on this?
> > > >
> > > > Yes, it makes sense to refresh attributes only when necessary.
> > > >
> > >
> > > OK, I will add a 'compound flag' for optional operations and don't
> > > execute those, when the fuse server does not support compounds.
> > >
> > > This way we can always send the open+getattr, but if the fuse server
> > > does not process this as a compound (aka. it would be costly to do it),
> > > we only resend the FUSE_OPEN. Thus the current behavior does not change
> > > and we can profit from fuse servers that actually have the possibility to
> > > give us the attributes.
> >
> > in my opinion, this adds additional complexity for no real benefit. I
> > think we'll rarely hit a case where it'll be useful to speculatively
> > prefetch attributes that are already valid that is not already
> > accounted for by the attr_timeout the server set.
> >
>
> So the current consensus would be to use the compound when we
> either don't have the data, or it has become invalid,
> and use the current behavior
> (just do an open and don't worry about the stale attributes)
> when we have unexpired attributes?
In my opinion yes that would be the best path forward. I don't think
we should be optimizing for the distributed backend stale attributes
case as it introduces additional complexity and overhead and the case
is rarely encountered. I think it's better handled by the lease idea
Miklos mentioned.
Thanks,
Joanne
>
> Thanks,
> Horst
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-03-03 10:03 ` Miklos Szeredi
2026-03-03 10:38 ` Horst Birthelmer
@ 2026-03-03 23:13 ` Joanne Koong
2026-03-04 9:37 ` Miklos Szeredi
1 sibling, 1 reply; 31+ messages in thread
From: Joanne Koong @ 2026-03-03 23:13 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Darrick J. Wong, Bernd Schubert, Horst Birthelmer,
Horst Birthelmer, Luis Henriques, linux-kernel, linux-fsdevel,
Horst Birthelmer
On Tue, Mar 3, 2026 at 2:03 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > >
> > > On 3/2/26 19:56, Joanne Koong wrote:
>
> > > > The overhead for the server to fetch the attributes may be nontrivial
> > > > (eg may require stat()). I really don't think we can assume the data
> > > > is locally cached somewhere. Why always compound the getattr to the
> > > > open instead of only compounding the getattr when the attributes are
> > > > actually invalid?
> > > >
> > > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > > Miklos, could you provide your input on this?
>
> Yes, it makes sense to refresh attributes only when necessary.
>
> > I wonder, since O_APPEND writes supposedly reposition the file position
> > to i_size before every write, can we enlarge the write reply so that the
> > fuse server could tell the client what i_size is supposed to be after
> > every write? Or perhaps add a notification so a network filesystem
> > could try to keep the kernel uptodate after another node appends to a
> > file?
>
> This can be done with FUSE_NOTIFY_INVAL_INODE.
>
> Still racy. If need to have perfect O_APPEND semantics,
> FOPEN_DIRECT_IO is currently the only option.
I think FOPEN_DIRECT_IO also uses the stale file size. eg
write_iter()
fuse_file_write_iter()
fuse_direct_write_iter()
generic_write_checks()
generic_write_checks_count()
which does
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
and uses the locally cached (and stale) i_size.
I think right now the only option is for the fuse server to just
handle append semantics itself if it detects the O_APPEND flag in the
write request and just ignore the kernel-provided offset (assuming the
distributed backend synchronizes access amongst multiple clients). Or
return -ESTALE to open() if it detects the file size is stale, which
will trigger a lookup to fetch the latest attributes.
Thanks,
Joanne
>
> It would be nice to have some sort of delegation/lease mechanism in
> the fuse protocol to allow caching when remote is not modifying (which
> is the common case usually) but force uncached I/O when there's
> concurrent modification.
>
> Thanks,
> Miklos
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
2026-03-03 23:13 ` Joanne Koong
@ 2026-03-04 9:37 ` Miklos Szeredi
0 siblings, 0 replies; 31+ messages in thread
From: Miklos Szeredi @ 2026-03-04 9:37 UTC (permalink / raw)
To: Joanne Koong
Cc: Darrick J. Wong, Bernd Schubert, Horst Birthelmer,
Horst Birthelmer, Luis Henriques, linux-kernel, linux-fsdevel,
Horst Birthelmer
On Wed, 4 Mar 2026 at 00:13, Joanne Koong <joannelkoong@gmail.com> wrote:
> I think right now the only option is for the fuse server to just
> handle append semantics itself if it detects the O_APPEND flag in the
> write request and just ignore the kernel-provided offset (assuming the
> distributed backend synchronizes access amongst multiple clients).
Right. It can also send a NOTIFY_INVAL_INODE in case it detects that
the offset was wrong. This will fix the file size, but not the file
position.
fuse_write_out could be extended with a file offset, which would fix
both issues.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 31+ messages in thread