On 8/21/20 7:55 AM, Glauber Costa wrote: > On Thu, Aug 20, 2020 at 11:42 PM Jens Axboe <axboe@xxxxxxxxx> wrote: >> >> On 8/20/20 8:24 PM, Glauber Costa wrote: >>> On Thu, Aug 20, 2020 at 9:57 PM Jens Axboe <axboe@xxxxxxxxx> 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
