Re: Poll ring behavior broken by f0c5c54945ae92a00cdbb43bdf3abaeab6bd3a23

[Jens Axboe <axboe@kernel.dk>]

On 8/21/20 7:55 AM, Glauber Costa wrote:
> On Thu, Aug 20, 2020 at 11:42 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 8/20/20 8:24 PM, Glauber Costa wrote:
>>> On Thu, Aug 20, 2020 at 9:57 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>> On 8/20/20 6:46 PM, Glauber Costa wrote:
>>>>> I have just noticed that the commit in $subject broke the behavior I
>>>>> introduced in
>>>>> bf3aeb3dbbd7f41369ebcceb887cc081ffff7b75
>>>>>
>>>>> In this commit, I have explained why and when it does make sense to
>>>>> enter the ring if there are no sqes to submit.
>>>>>
>>>>> I guess one could argue that in that case one could call the system
>>>>> call directly, but it is nice that the application didn't have to
>>>>> worry about that, had to take no conditionals, and could just rely on
>>>>> io_uring_submit as an entry point.
>>>>>
>>>>> Since the author is the first to say in the patch that the patch may
>>>>> not be needed, my opinion is that not only it is not needed but in
>>>>> fact broke applications that relied on previous behavior on the poll
>>>>> ring.
>>>>>
>>>>> Can we please revert?
>>>>
>>>> Yeah let's just revert it for now. Any chance you can turn this into
>>>> a test case for liburing? Would help us not break this in the future.
>>>
>>> would be my pleasure.
>>>
>>> Biggest issue is that poll mode really only works with ext4 and xfs as
>>> far as I know. That may mean it won't get as much coverage, but maybe
>>> that's not relevant.
>>
>> And raw nvme too, of course. But I'd say coverage is pretty decent with
>> those two, in reality that's most likely what people would use for
>> polling anyway. So not too concerned about that, and it'll hit multiple
>> items in my test suite.
>>
>> I reverted the change manually, it didn't revert cleanly. Please test
>> current -git, thanks!
>>
> 
> Just tested (through a new unit test) and it works, thanks.
> 
> I wrote a unit test that works on HEAD but not on HEAD^.

Perfect, thanks!

> However you will have to excuse my lack of manners, but in my new work
> account I can't have app passwords for GSuite so I am unable to
> git-send-email it without talking to a host of corporate IT people...
> I'll have to send a ... urgh... PR

No worries, I don't mind GH PRs, as long as the patch looks good. The
problem is that lots of GH PRs end up being done poorly.

-- 
Jens Axboe