Re: [PATCH] Fuse: Add backing file support for uring_cmd

[Amir Goldstein]

On Fri, Feb 21, 2025 at 5:17 PM Bernd Schubert <bernd@xxxxxxxxxxx> wrote:
>
>
>
> On 2/21/25 17:14, Bernd Schubert wrote:
> >
> >
> > On 2/21/25 16:36, Moinak Bhattacharyya wrote:
> >> Sorry about that. Correctly-formatted patch follows. Should I send out a
> >> V2 instead?
> >>
> >> Add support for opening and closing backing files in the fuse_uring_cmd
> >> callback. Store backing_map (for open) and backing_id (for close) in the
> >> uring_cmd data.
> >> ---
> >>  fs/fuse/dev_uring.c       | 50 +++++++++++++++++++++++++++++++++++++++
> >>  include/uapi/linux/fuse.h |  6 +++++
> >>  2 files changed, 56 insertions(+)
> >>
> >> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> >> index ebd2931b4f2a..df73d9d7e686 100644
> >> --- a/fs/fuse/dev_uring.c
> >> +++ b/fs/fuse/dev_uring.c
> >> @@ -1033,6 +1033,40 @@ fuse_uring_create_ring_ent(struct io_uring_cmd *cmd,
> >>      return ent;
> >>  }
> >>
> >> +/*
> >> + * Register new backing file for passthrough, getting backing map from
> >> URING_CMD data
> >> + */
> >> +static int fuse_uring_backing_open(struct io_uring_cmd *cmd,
> >> +    unsigned int issue_flags, struct fuse_conn *fc)
> >> +{
> >> +    const struct fuse_backing_map *map = io_uring_sqe_cmd(cmd->sqe);
> >> +    int ret = fuse_backing_open(fc, map);
> >
> > Do you have the libfuse part somewhere? I need to hurry up to split and
> > clean up my uring branch. Not promised, but maybe this weekend.
> > What we need to be careful here about is that in my current 'uring'
> > libfuse always expects to get a CQE - here you introduce a 2nd user
> > for CQEs - it needs credit management.
> >
> >
> >> +
> >> +    if (ret < 0) {
> >> +        return ret;
> >> +    }
> >> +
> >> +    io_uring_cmd_done(cmd, ret, 0, issue_flags);
> >> +    return 0;
> >> +}
> >> +
> >> +/*
> >> + * Remove file from passthrough tracking, getting backing_id from
> >> URING_CMD data
> >> + */
> >> +static int fuse_uring_backing_close(struct io_uring_cmd *cmd,
> >> +    unsigned int issue_flags, struct fuse_conn *fc)
> >> +{
> >> +    const int *backing_id = io_uring_sqe_cmd(cmd->sqe);
> >> +    int ret = fuse_backing_close(fc, *backing_id);
> >> +
> >> +    if (ret < 0) {
> >> +        return ret;
> >> +    }
> >
> >
> > Both functions don't have the check for
> >
> >       if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> >               return -EOPNOTSUPP;
> >
> > but their ioctl counter parts have that.
> >
>
> In order to avoid code dup, maybe that check could be moved
> into fuse_backing_open() / fuse_backing_close() as preparation
> patch? Amir?

Without CONFIG_FUSE_PASSTHROUGH, fuse/passthrough.c
is compiled out, so the check cannot be moved into fuse_backing_*
we'd need inline helpers that return -EOPNOTSUPP when
CONFIG_FUSE_PASSTHROUGH is not defined.
I don't mind, but I am not sure this is justified (yet).

Thanks,
Amir.