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

[Joanne Koong]

On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@xxxxxxxxxxxxxx> wrote:
>
> From: Horst Birthelmer <hbirthelmer@xxxxxxx>
>
> +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;
> +}