Re: [PATCH 4/5] io_uring: add support for batch wait timeout

[Pavel Begunkov <asml.silence@gmail.com>]

On 8/22/24 16:37, Jens Axboe wrote:
> On 8/22/24 7:46 AM, Pavel Begunkov wrote:
>> On 8/21/24 15:16, Jens Axboe wrote:
...
>>>          if (ext_arg->sig) {
>>> @@ -2484,14 +2544,16 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events, u32 flags,
>>>            unsigned long check_cq;
>>>              if (ctx->flags & IORING_SETUP_DEFER_TASKRUN) {
>>> -            atomic_set(&ctx->cq_wait_nr, nr_wait);
>>> +            /* if min timeout has been hit, don't reset wait count */
>>> +            if (!READ_ONCE(iowq.hit_timeout))
>>
>> Why read once? You're out of io_cqring_schedule_timeout(),
>> timers are cancelled and everything should've been synchronised
>> by this point.
> 
> Just for consistency's sake.

Please drop it. Sync primitives tell a story, and this one says
that it's racing with something when it's not. It's always hard to
work with code with unnecessary protection. If it has to change in
the future the first question asked would be why read once is there,
what does it try to achieve / protect and if it's safe to kill it.
It'll also hide real races from sanitizers.

-- 
Pavel Begunkov