On 1/24/25 23:05, Joanne Koong wrote:
> On Fri, Jan 24, 2025 at 10:22 AM Bernd Schubert
> <bernd.schubert@xxxxxxxxxxx> wrote:
>>
>> Hi Joanne,
>>
>> On 1/24/25 12:30, Bernd Schubert wrote:
>>> Hmm, would only need to check head? Oh I see it, we need to use
>>> list_move_tail().
>>
>>
>> how about the attached updated patch, which uses
>> list_first_entry_or_null()? It also changes from list_move()
>> to list_move_tail() so that oldest entry is always on top.
>> I didn't give it any testing, though.
>
> Woah that's cool, I didn't know you could send attachments over the
> mailing list.
> Ah I didn't realize list_move doesn't already by default add to the
> tail of the list - thanks for catching that, yes those should be
> list_move_tail() then.
>
> In t he attached patch, I think we still need the original
> ent_list_request_expired() logic:
>
> static bool ent_list_request_expired(struct fuse_conn *fc, struct
> list_head *list)
> {
> struct fuse_ring_ent *ent;
> struct fuse_req *req;
>
> list_for_each_entry(ent, list, list) {
> req = ent->fuse_req;
> if (req)
> return time_is_before_jiffies(req->create_time +
> fc->timeout.req_timeout);
> }
>
> return false;
> }
Could you explain why? That would be super expensive if lists
have many entries? Due to fg and bg queues it might not be
perfectly ordered, but that it actually also true for
fuse_req_queue and also without io-uring. Server might process
requests in different order, so ent_commit_queue might also not
be perfectly sorted. But then I'm not even sure if we need to
process that queue, as it has entries that are already processed - at
best that would catch fuse client/kernel bugs.
Though, if there is some kind if req processing issue, either
everything times out, or the head will eventually get the older
requests. So I don't understand why would need to go over all entries.
>
> and we can't assume req is non-NULL. For entries that have been
> committed, their ->req is set to NULL but they are still on the
> ent_commit_queue.
Yeah sorry, right, though I wonder if we can just avoid checking
that queue.
Thanks,
Bernd
