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

[Bernd Schubert <bernd.schubert@fastmail.fm>]

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
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