On 11/23/24 14:09, Miklos Szeredi wrote:
> On Sat, 23 Nov 2024 at 13:42, Bernd Schubert <bernd.schubert@xxxxxxxxxxx> wrote:
>>
>>
>>
>> On 11/23/24 10:52, Miklos Szeredi wrote:
>>> On Fri, 22 Nov 2024 at 00:44, Bernd Schubert <bschubert@xxxxxxx> 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
