Re: [PATCH RFC v6 06/16] fuse: {uring} Handle SQEs - register commands

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

On 11/23/24 14:09, Miklos Szeredi wrote:
> On Sat, 23 Nov 2024 at 13:42, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>>
>>
>>
>> On 11/23/24 10:52, Miklos Szeredi wrote:
>>> On Fri, 22 Nov 2024 at 00:44, Bernd Schubert <bschubert@ddn.com> wrote:
>>>
>>>> +static struct fuse_ring *fuse_uring_create(struct fuse_conn *fc)
>>>> +{
>>>> +       struct fuse_ring *ring = NULL;
>>>> +       size_t nr_queues = num_possible_cpus();
>>>> +       struct fuse_ring *res = NULL;
>>>> +
>>>> +       ring = kzalloc(sizeof(*fc->ring) +
>>>> +                              nr_queues * sizeof(struct fuse_ring_queue),
>>>
>>> Left over from a previous version?
>>
>> Why? This struct holds all the queues? We could also put into fc, but
>> it would take additional memory, even if uring is not used.
> 
> But fuse_ring_queue is allocated on demand in
> fuse_uring_create_queue().  Where is this space actually gets used?


In fuse_uring_fetch()

	err = -ENOMEM;
	if (!ring) {
		ring = fuse_uring_create(fc);
		if (!ring)
			return err;
	}

	queue = ring->queues[cmd_req->qid];
	if (!queue) {
		queue = fuse_uring_create_queue(ring, cmd_req->qid);
		if (!queue)
			return err;
	}

I.е. the ring object is created dynamically. Btw, I still a bit struggling
with struct names - maybe 'struct fuse_ring_pool' is a better name?


> 
>> there you really need a ring state, because access is outside of lists.
>> Unless you want to iterate over the lists, if the the entry is still
>> in there. Please see the discussion with Joanne in RFC v5.
>> I have also added in v6 15/16 comments about non-list access.
> 
> Okay, let that be then.
> 
>> Even though libfuse sends the SQEs before
>> setting up /dev/fuse threads, handling the SQEs takes longer.
>> So what happens is that while IORING_OP_URING_CMD/FUSE_URING_REQ_FETCH
>> are coming in, FUSE_INIT reply gets through. In userspace we do not
>> know at all, when these SQEs are registered, because we don't get
>> a reply. Even worse, we don't even know if io-uring works at all and
>> cannot adjust number of /dev/fuse handling threads. Here setup with
>> ioctls had a clear advantage - there was a clear reply.
> 
> Server could negotiate fuse uring availability in INIT, which is how
> all other feature negotiations work.
> 
>> The other issue is, that we will probably first need handle FUSE_INIT
>> in userspace before sending SQEs at all, in order to know the payload
>> buffer size.
> 
> Yeah.

Fine with me will move it in libfuse


Thanks,
Bernd