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

[Miklos Szeredi]

On Wed, 11 Feb 2026 at 17:35, Horst Birthelmer <horst@xxxxxxxxxxxxx> wrote:
>
> Hi Miklos,
>
> thanks for taking the time to look at this!
>
> On Wed, Feb 11, 2026 at 05:13:21PM +0100, Miklos Szeredi wrote:
> > On Tue, 10 Feb 2026 at 09:46, Horst Birthelmer <horst@xxxxxxxxxxxxxx> 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?
>
> OK, will do. Since it was never used in libfuse, I didn't notice.
>
> >
> > > +       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.
> >
>
> Wouldn't that make for a very complicated implementation of larger compounds.
> If a fuse server negotiates something like 2?

I didn't mean negotiating the number of ops, rather the size of the
buffer for the compound.

But let's not overthink this.   If compound doesn't fit in 4k, then it
probably not worth doing anyway.

> > > +
> > > +#define FUSE_COMPOUND_SEPARABLE (1<<0)
> > > +#define FUSE_COMPOUND_ATOMIC (1<<1)
> >
> > What is the meaning of these flags?
>
> FUSE_COMPOUND_SEPARABLE is a hint for the fuse server that the requests are all
> complete and there is no need to use the result of one request to complete the input
> of another request further down the line.

Aha, so it means parallel execution is allowed.

> Think of LOOKUP+MKNOD+OPEN ... that would never be 'separable'.

Right.  I think for the moment we don't need to think about parallel
execution within a compound.

> At the moment I use this flag to signal libfuse that it can decode the compund
> and execute sequencially completely in the lib and just call the separate requests
> of the fuse server.

I think decoding and executing the ops sequentially should always be
possible, and it would be one of the major advantages of the compound
architecture: kernel packs a number of requests that it would do
sequentially, sends to server, server decodes and calls individual
callbacks in filesystem, then replies with the compound result.  This
reduces the number of syscalls/context switches which can be a win
even with an unchanged library API.

The trick in a case like MKNOD + OPEN is to let the server know how to
feed the output of one request to the input of the next.

> FUSE_COMPOUND_ATOMIC was an idea to hint to the fuse server that the kernel treats
> the compound as one atomic request. This can maybe save us some checks for some
> compounds.

Do you have an example?

Thanks,
Miklos