Re: [PATCH RFC v3 06/17] fuse: Add the queue configuration ioctl

[Joanne Koong <joannelkoong@gmail.com>]

On Wed, Sep 4, 2024 at 3:38 PM Bernd Schubert
<bernd.schubert@fastmail.fm> wrote:
>
> On 9/5/24 00:23, Joanne Koong wrote:
> > On Sun, Sep 1, 2024 at 6:37 AM Bernd Schubert <bschubert@ddn.com> wrote:
> >>
> >> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
> >> ---
> >>  fs/fuse/dev.c             | 30 ++++++++++++++++++++++++++++++
> >>  fs/fuse/dev_uring.c       |  2 ++
> >>  fs/fuse/dev_uring_i.h     | 13 +++++++++++++
> >>  fs/fuse/fuse_i.h          |  4 ++++
> >>  include/uapi/linux/fuse.h | 39 +++++++++++++++++++++++++++++++++++++++
> >>  5 files changed, 88 insertions(+)
> >>
> >> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> >> index 6489179e7260..06ea4dc5ffe1 100644
> >> --- a/fs/fuse/dev.c
> >> +++ b/fs/fuse/dev.c
> >> @@ -2379,6 +2379,33 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> >>         return fuse_backing_close(fud->fc, backing_id);
> >>  }
> >>
> >> +#ifdef CONFIG_FUSE_IO_URING
> >> +static long fuse_uring_queue_ioc(struct file *file, __u32 __user *argp)
> >> +{
> >> +       int res = 0;
> >> +       struct fuse_dev *fud;
> >> +       struct fuse_conn *fc;
> >> +       struct fuse_ring_queue_config qcfg;
> >> +
> >> +       res = copy_from_user(&qcfg, (void *)argp, sizeof(qcfg));
> >> +       if (res != 0)
> >> +               return -EFAULT;
> >> +
> >> +       res = _fuse_dev_ioctl_clone(file, qcfg.control_fd);
> >
> > I'm confused how this works for > 1 queues. If I'm understanding this
> > correctly, if a system has multiple cores and the server would like
> > multi-queues, then the server needs to call the ioctl
> > FUSE_DEV_IOC_URING_QUEUE_CFG multiple times (each with a different
> > qid).
> >
> > In this handler, when we get to _fuse_dev_ioctl_clone() ->
> > fuse_device_clone(), it allocates and installs a new fud and then sets
> > file->private_data to fud, but isn't this underlying file the same for
> > all of the queues since they are using the same fd for the ioctl
> > calls? It seems like every queue after the 1st would fail with -EINVAL
> > from the "if (new->private_data)" check in fuse_device_clone()?
>
> Each queue is using it's own fd - this works exactly the same as
> a existing FUSE_DEV_IOC_CLONE - each clone has to open /dev/fuse on its

Ah gotcha, this is the part I was missing. I didn't realize the
expectation is that the server needs to open a new /dev/fuse for each
queue. This makes more sense to me now, thanks.

> own. A bit a pity that dup() isn't sufficient. Only difference to
> FUSE_DEV_IOC_CLONE is the additional qid.
>
> >
> > Not sure if I'm missing something or if this intentionally doesn't
> > support multi-queue yet. If the latter, then I'm curious how you're
> > planning to get the fud for a specific queue given that
> > file->private_data and fuse_get_dev() only can support the single
> > queue case.
>
>
> Strictly in the current patch set, the clone is only needed in the
> next patch
> "07/17] fuse: {uring} Add a dev_release exception for fuse-over-io-uring"
> Though, since we have the fud anyway and link to the ring-queue, it makes
> use of it in
> 08/17] fuse: {uring} Handle SQEs - register commands
>
> in fuse_uring_cmd().
>
>
> I hope I understood your question right.
>
>
> Thanks,
> Bernd