public inbox for [email protected]
 help / color / mirror / Atom feed
From: Jens Axboe <[email protected]>
To: Jackie Liu <[email protected]>,
	Pavel Begunkov <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] io_uring: drain next sqe instead of shadowing
Date: Thu, 21 Nov 2019 06:47:15 -0700	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

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 <[email protected]> 写道:
>>>>>>
>>>>>> 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.

> 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.

-- 
Jens Axboe


  parent reply	other threads:[~2019-11-21 13:47 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-20 23:07 [PATCH] io_uring: fix race with shadow drain deferrals Jens Axboe
2019-11-20 23:58 ` Jens Axboe
2019-11-21  1:32   ` Jackie Liu
2019-11-21  1:35     ` Jackie Liu
2019-11-21  1:40       ` Jens Axboe
2019-11-21  1:49         ` Jens Axboe
2019-11-21  1:57           ` Jackie Liu
2019-11-20 23:14             ` Jens Axboe
     [not found]               ` <[email protected]>
2019-11-20 23:03                 ` Jens Axboe
2019-11-21  8:54           ` [PATCH] io_uring: drain next sqe instead of shadowing Pavel Begunkov
     [not found]             ` <[email protected]>
2019-11-21  9:43               ` Pavel Begunkov
     [not found]                 ` <[email protected]>
2019-11-21 12:40                   ` Pavel Begunkov
     [not found]                     ` <[email protected]>
2019-11-21 13:47                       ` Jens Axboe [this message]
     [not found]                         ` <[email protected]>
2019-11-21 13:54                           ` Jens Axboe
     [not found]                         ` <[email protected]>
2019-11-21 14:28                           ` Pavel Begunkov
2019-11-21 13:53                             ` Jens Axboe
2019-11-21 15:23                               ` Pavel Begunkov
2019-11-21 13:50                                 ` Jens Axboe
2019-11-21  1:39     ` [PATCH] io_uring: fix race with shadow drain deferrals Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox