[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJfpegvt0HwHOmOTzkCoOqdmvU6pf-wM228QQSauDsbcL+mmUA@mail.gmail.com>
Date: Wed, 11 Feb 2026 17:13:21 +0100
From: Miklos Szeredi <miklos@...redi.hu>
To: Horst Birthelmer <horst@...thelmer.com>
Cc: Bernd Schubert <bschubert@....com>, Joanne Koong <joannelkoong@...il.com>,
Luis Henriques <luis@...lia.com>, linux-kernel@...r.kernel.org,
linux-fsdevel@...r.kernel.org, Horst Birthelmer <hbirthelmer@....com>
Subject: Re: [PATCH v5 1/3] fuse: add compound command to combine multiple requests
On Tue, 10 Feb 2026 at 09:46, Horst Birthelmer <horst@...thelmer.com> wrote:
> +static char *fuse_compound_build_one_op(struct fuse_conn *fc,
> + struct fuse_args *op_args,
> + char *buffer_pos)
> +{
> + struct fuse_in_header *hdr;
> + size_t needed_size = sizeof(struct fuse_in_header);
> + int j;
> +
> + for (j = 0; j < op_args->in_numargs; j++)
> + needed_size += op_args->in_args[j].size;
> +
> + hdr = (struct fuse_in_header *)buffer_pos;
> + memset(hdr, 0, sizeof(*hdr));
> + hdr->len = needed_size;
> + hdr->opcode = op_args->opcode;
> + hdr->nodeid = op_args->nodeid;
hdr->unique is notably missing.
I don't know. Maybe just fill it with the index?
> + hdr->uid = from_kuid(fc->user_ns, current_fsuid());
> + hdr->gid = from_kgid(fc->user_ns, current_fsgid());
uid/gid are not needed except for creation ops, and those need idmap
to calculate the correct values. I don't think we want to keep legacy
behavior of always setting these.
> + hdr->pid = pid_nr_ns(task_pid(current), fc->pid_ns);
This will be the same as the value in the compound header, so it's
redundant. That might not be bad, but I feel that we're better off
setting this to zero and letting the userspace server fetch the pid
value from the compound header if that's needed.
> +#define FUSE_MAX_COMPOUND_OPS 16 /* Maximum operations per compound */
Don't see a good reason to declare this in the API. More sensible
would be to negotiate a max_request_size during INIT.
> +
> +#define FUSE_COMPOUND_SEPARABLE (1<<0)
> +#define FUSE_COMPOUND_ATOMIC (1<<1)
What is the meaning of these flags?
> +
> +/*
> + * Compound request header
> + *
> + * This header is followed by the fuse requests
> + */
> +struct fuse_compound_in {
> + uint32_t count; /* Number of operations */
This is redundant, as the sum of the sub-request lengths is equal to
the compound request length, hence calculating the number of ops is
trivial.
> + uint32_t flags; /* Compound flags */
> +
> + /* Total size of all results.
> + * This is needed for preallocating the whole result for all
> + * commands in this compound.
> + */
> + uint32_t result_size;
I don't understand why this is needed. Preallocation by the userspace
server? Why is this different from a simple request?
> + uint64_t reserved;
> +};
> +
> +/*
> + * Compound response header
> + *
> + * This header is followed by complete fuse responses
> + */
> +struct fuse_compound_out {
> + uint32_t count; /* Number of results */
Again, redundant.
Thanks,
Miklos
Powered by blists - more mailing lists