Re: [PATCH for-next 4/7] io_uring: hide eventfd assumptions in evenfd paths

[Jens Axboe <axboe@kernel.dk>]

On 6/19/22 12:49 PM, Pavel Begunkov wrote:
> On 6/19/22 19:18, Jens Axboe wrote:
>> On Sun, Jun 19, 2022 at 5:26 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>>
>>> Some io_uring-eventfd users assume that there won't be spurious wakeups.
>>> That assumption has to be honoured by all io_cqring_ev_posted() callers,
>>> which is inconvenient and from time to time leads to problems but should
>>> be maintained to not break the userspace.
>>>
>>> Instead of making the callers to track whether a CQE was posted or not,
>>> hide it inside io_eventfd_signal(). It saves ->cached_cq_tail it saw
>>> last time and triggers the eventfd only when ->cached_cq_tail changed
>>> since then.
>>
>> This one is causing frequent errors with poll-cancel.t:
>>
>> axboe@m1pro-kvm ~/g/liburing (master)> test/poll-cancel.t
>> axboe@m1pro-kvm ~/g/liburing (master)> test/poll-cancel.t
>> Timed out!
>> axboe@m1pro-kvm ~/g/liburing (master) [1]> test/poll-cancel.t
>> Timed out!
>> axboe@m1pro-kvm ~/g/liburing (master) [1]> test/poll-cancel.t
>> Timed out!
>>
>> I've dropped this one, and 6-7/7 as they then also throw a bunch of
>> rejects.
> 
> I mentioned it in the cover letter, extra wake ups slowing task
> exit cancellations down, which make it to timeout (increasing
> alarm time helps). The problem is in cancellation, in particular
> because for some reason it spins on the cancellation (including
> timeout and poll remove all).

But that's not really usable at all. Before the patch, the test case
takes 1-2ms, predictably. After the patch, I see lots of 10.0s runs.
That's clearly not going to work.

-- 
Jens Axboe