On Thu, Jan 23, 2025 at 9:19 AM Bernd Schubert <bernd.schubert@xxxxxxxxxxx> wrote: > > Hi Joanne, > > >>> Thanks, applied and pushed with some cleanups including Luis's clamp idea. > >> > >> Hi Miklos, > >> > >> I don't think the timeouts do work with io-uring yet, I'm not sure > >> yet if I have time to work on that today or tomorrow (on something > >> else right now, I can try, but no promises). > > > > Hi Bernd, > > > > What are your thoughts on what is missing on the io-uring side for > > timeouts? If a request times out, it will abort the connection and > > AFAICT, the abort logic should already be fine for io-uring, as users > > can currently abort the connection through the sysfs interface and > > there's no internal difference in aborting through sysfs vs timeouts. > > > > in fuse_check_timeout() it iterates over each fud and then fpq. > In dev_uring.c fpq is is per queue but unrelated to fud. In current > fuse-io-uring fud is not cloned anymore - using fud won't work. > And Requests are also not queued at all on the other list > fuse_check_timeout() is currently checking. In the io-uring case, there still can be fuds and their associated fpqs given that /dev/fuse can be used still, no? So wouldn't the io-uring case still need this logic in fuse_check_timeout() for checking requests going through /dev/fuse? > > Also, with a ring per core, maybe better to use > a per queue check that is core bound? I.e. zero locking overhead? How do you envision a queue check that bypasses grabbing the queue->lock? The timeout handler could be triggered on any core, so I'm not seeing how it could be core bound. > And I think we can also avoid iterating over hash lists (queue->fpq), > but can use the 'ent_in_userspace' list. > > We need to iterate over these other entry queues anyway: > > ent_w_req_queue > fuse_req_bg_queue > ent_commit_queue > Why do we need to iterate through the ent lists (ent_w_req_queue and ent_commit_queue)? AFAICT, in io-uring a request is either on the fuse_req_queue/fuse_req_bg_queue or on the fpq->processing list. Even when an entry has been queued to ent_w_req_queue or ent_commit_queue, the request itself is still queued on fuse_req_queue/fuse_req_bg_queue/fpq->processing. I'm not sure I understand why we still need to look at the ent lists? > > And we also need to iterate over > > fuse_req_queue > fuse_req_bg_queue Why do we need to iterate through fuse_req_queue and fuse_req_bg_queue? fuse_uring_request_expired() checks the head of fuse_req_queue and fuse_req_bg_queue and given that requests are added to fuse_req_queue/fuse_req_bg_queue sequentially (eg added to the tail of these lists), why isn't this enough? If it's helpful, I can resubmit this patch series so that the io-uring changes are isolated to its own patch (eg have patch 1 and 2 from the original series and then have patch 3 be the io-uring changes). Thanks, Joanne > > > > Thanks, > Bernd > >
