On 10/3/24 14:02, Miklos Szeredi wrote: > On Thu, 3 Oct 2024 at 12:10, Bernd Schubert <bernd.schubert@xxxxxxxxxxx> wrote: > >> What I mean is that you wanted to get rid of the 'tag' - using any kind of >> search means we still need it. I.e. we cannot just take last list head >> or tail and use that. >> The array is only dynamic at initialization time. And why spending O(logN) >> to search instead of O(1)? > > Because for sane queue depths they are essentially the same. This is > not where we can gain or lose any significant performance. > >> And I know that it is an implementation detail, I just would like to avoid >> many rebasing rounds on these details. > > I think the logical interface would be: > > - pass a userspace buffer to FETCH (you told me, but I don't remember > why sqe->addr isn't suitable) I think we could change to that now. > > - set sqe->user_data to an implementation dependent value, this could > be just the userspace buffer, but it could be a request object Libfuse already does this, it points to 'struct fuse_ring_ent', which then points to the buffer. Maybe that could be optimized to have 'struct fuse_ring_ent' as part of the buffer. > > - kernel allocates an idle request and queues it. > > - request comes in, kernel takes a request from the idle queue and fills it > > - cqe->user_data is returned with the original sqe->user_data, which > should be sufficient for the server to identify the request > > - process request, send COMMIT_AND_FETCH with the userspace buffer > and user data > > - the kernel reads the header from the userspace buffer, finds > outh->unique, finds and completes the request > > - then queues the request on the idle queue > > ... > > What's wrong with that? In my opinion a bit sad that we have to search instead of just doing an array[ID] access. I certainly don't want to rely on the current hashed list search, this only works reasonably well, because with the current threading model requests in fly is typically small. And then rb entries also have pointers - it won't take any less memory, more on the contrary. At best one could argue that on tear down races are avoided as one has to do a search now. Although that was handled but checks tear-down checks in fuse_uring_cmd. Well, if you really prefer, I'm going to add use an rbtree or maybe better xarray search. No reason for anything like that in libfuse, it can just continue to type cast 'user_data'. Thanks, Bernd
