Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls

Joanne Koong
2025-01-23T17:49:05Z

On Thu, Jan 23, 2025 at 9:19 AM Bernd Schubert
<bernd.schubert@fastmail.fm> 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
>
>