On 11/18/22 3:39 PM, Pavel Begunkov wrote:
> On 11/18/22 20:20, Pavel Begunkov wrote:
>> poll_refs carry two functions, the first is ownership over the request.
>> The second is notifying the io_poll_check_events() that there was an
>> event but wake up couldn't grab the ownership, so io_poll_check_events()
>> should retry.
>>
>> We want to make poll_refs more robust against overflows. Instead of
>> always incrementing it, which covers two purposes with one atomic, check
>> if poll_refs is large and if so set a retry flag without attempts to
>> grab ownership. The gap between the bias check and following atomics may
>> seem racy, but we don't need it to be strict. Moreover there might only
>> be maximum 4 parallel updates: by the first and the second poll entries,
>> __io_arm_poll_handler() and cancellation. From those four, only poll wake
>> ups may be executed multiple times, but they're protected by a spin.
>>
>> Cc: stable@xxxxxxxxxxxxxxx
>> Reported-by: Lin Horse <kylin.formalin@xxxxxxxxx>
>> Fixes: aa43477b04025 ("io_uring: poll rework")
>> Signed-off-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
>> ---
> [...]
>> @@ -590,7 +624,7 @@ static int __io_arm_poll_handler(struct io_kiocb *req,
>> * locked, kick it off for them.
>> */
>> v = atomic_dec_return(&req->poll_refs);
>> - if (unlikely(v & IO_POLL_REF_MASK))
>> + if (unlikely(v & IO_POLL_RETRY_MASK))
>
> Still no good, this chunk is about ownership and so should use
> IO_POLL_REF_MASK as before. Jens, please drop the patch, needs
> more testing
Bummed - I dropped it for now.
--
Jens Axboe
