Re: [PATCH] io_uring: drain next sqe instead of shadowing

[Jens Axboe <axboe@kernel.dk>]

On 11/21/19 7:16 AM, Jackie Liu wrote:
> 在 2019/11/21 21:47, Jens Axboe 写道:
>> On 11/21/19 5:49 AM, Jackie Liu wrote:
>>>
>>>
>>> On 2019/11/21 20:40, Pavel Begunkov wrote:
>>>>> 在 2019/11/21 17:43, Pavel Begunkov 写道:
>>>>>> On 11/21/2019 12:26 PM, Jackie Liu wrote:
>>>>>>> 2019年11月21日 16:54，Pavel Begunkov <asml.silence@gmail.com> 写道：
>>>>>>>>
>>>>>>>> If there is a DRAIN in the middle of a link, it uses shadow req. Defer
>>>>>>>> the next request/link instead. This:
>>>>>>>>
>>>>>>>> Pros:
>>>>>>>> 1. removes semi-duplicated code
>>>>>>>> 2. doesn't allocate memory for shadows
>>>>>>>> 3. works better if only the head marked for drain
>>>>>>>
>>>>>>> I thought about this before, just only drain the head, but if the
>>>>>>> latter IO depends
>>>>>>> on the link-list, then latter IO will run in front of the link-list.
>>>>>>> If we think it
>>>>>>> is acceptable, then I think it is ok for me.
>>>>>>
>>>>>> If I got your point right, latter requests won't run ahead of the
>>>>>> link-list. There shouldn't be change of behaviour.
>>>>>>
>>>>>> The purpose of shadow requests is to mark some request right ahead of
>>>>>> the link for draining. This patch uses not a specially added shadow
>>>>>> request, but the following regular one. And, as drained IO shouldn't be
>>>>>> issued until every request behind completed, this should give the same
>>>>>> effect.
>>>>>>
>>>>>> Am I missed something?
>>>>>
>>>>> Thanks for explaining. This is also correct, if I understand
>>>>> correctly, It seems that other IOs will wait for all the links are
>>>>> done. this is a little different, is it?
>>>>
>>>> Yes, you're right, it also was briefly stated in the patch description
>>>> (see Cons). I hope, links + drain in the middle is an uncommon case.
>>>> But it can be added back, but may become a little bit uglier.
>>>>
>>>> What do you think, should we care about this case?
>>>
>>> Yes, this is a very tiny scene. When I first time wrote this part of the
>>> code, my suggestion was to ban it directly.
>>>
>>> For this patch, I am fine, Jens, what do you think.
>>
>> I am fine with it as well, it'd be nice to get rid of needing that
>> extra request.
>>
>> Was that a reviewed-by from you? It'd be nice to get these more
>> formally so I can add the attributes. I'll drop the other patch.
> 
> I want to do more tests. There is no test machine at this time, at least
> until tomorrow morning.

OK, no worries, for the record I did run it through testing this morning
and it passes for me. Before this (or my) patch, we'd stall pretty
quickly in the link_drain tested if run repeatedly.

>>> The mailing list always rejects my mail, is my smtp server IP banned?
>>
>> Probably because you have text/html in your email, the list is pretty
>> picky when it comes to anything that isn't just text/plain.
> 
> I don't know, I use thunderbird to write emails, and it has been set to
> plain text, perhaps because of the signature, I have to check my mail
> client.

Feel free to try and send an email to me personally, then I can see what
it looks like.

-- 
Jens Axboe