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

[Bernd Schubert <bernd@bsbernd.com>]

On 2/21/25 17:35, Amir Goldstein wrote:
> On Fri, Feb 21, 2025 at 5:17 PM Bernd Schubert <bernd@bsbernd.com> 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).
> 

Ah right, then let's duplicate the check.