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

[Miklos Szeredi <miklos@szeredi.hu>]

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?

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

Thanks,
Miklos