On 11/1/21 10:49 AM, Linus Torvalds wrote:
> On Sun, Oct 31, 2021 at 12:41 PM Jens Axboe <axboe@xxxxxxxxx> wrote:
>>
>> This will throw two merge conflicts, see below for how I resolved it.
>> There are two spots, one is trivial, and the other needs
>> io_queue_linked_timeout() moved into io_queue_sqe_arm_apoll().
>
> So I ended up resolving it the same way you did, because that was the
> mindless direct thing.
>
> But I don't much like it.
>
> Basically, io_queue_sqe_arm_apoll() now ends up doing
>
> case IO_APOLL_READY:
> if (linked_timeout) {
> io_queue_linked_timeout(linked_timeout);
> linked_timeout = NULL;
> }
> io_req_task_queue(req);
> break;
> ...
> if (linked_timeout)
> io_queue_linked_timeout(linked_timeout);
>
> and that really seems *completely* pointless. Notice how it does that
>
> if (linked_timeout)
> io_queue_linked_timeout()
>
> basically twice, and sets linked_timeout to NULL just to avoid the second one...
>
> Why isn't it just
>
> case IO_APOLL_READY:
> io_req_task_queue(req);
> break;
> ...
> if (linked_timeout)
> io_queue_linked_timeout(linked_timeout);
>
> where the only difference would seem to be the order of operations
> between io_req_task_queue() and io_queue_linked_timeout()?
>
> Does the order of operations really matter here? As far as I can tell,
> io_req_task_queue() really just queues up work for later, so it's not
> really very ordered wrt that io_queue_linked_timeout(), and in the
> _other_ case statement it's apparently fine to do that
> io_queue_async_work() before the io_queue_linked_timeout()..
>
> Again - I ended up resolving this the same way you had done, because I
> don't know the exact rules here well enough to do anything else. But
> it _looks_ a bit messy.
Yes I agree, and it's mostly just to keep the resolution simpler as I
don't think the current construct makes too much sense when both of them
end up being queueing the linked timeout. I think the cleanup done here
made more sense in the context before, not now.
We'll get a cleanup done for this shortly.
--
Jens Axboe
