public inbox for [email protected]
 help / color / mirror / Atom feed
From: Hao Xu <[email protected]>
To: Jens Axboe <[email protected]>, Pavel Begunkov <[email protected]>
Cc: [email protected], Joseph Qi <[email protected]>
Subject: Re: [PATCH 5.13 v2] io_uring: maintain drain requests' logic
Date: Fri, 9 Apr 2021 14:15:38 +0800	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

在 2021/4/9 上午12:18, Jens Axboe 写道:
> On 4/8/21 6:22 AM, Pavel Begunkov wrote:
>> On 08/04/2021 12:43, Hao Xu wrote:
>>> 在 2021/4/8 下午6:16, Hao Xu 写道:
>>>> 在 2021/4/7 下午11:49, Jens Axboe 写道:
>>>>> On 4/7/21 5:23 AM, Hao Xu wrote:
>>>>>> more tests comming, send this out first for comments.
>>>>>>
>>>>>> Hao Xu (3):
>>>>>>     io_uring: add IOSQE_MULTI_CQES/REQ_F_MULTI_CQES for multishot requests
>>>>>>     io_uring: maintain drain logic for multishot requests
>>>>>>     io_uring: use REQ_F_MULTI_CQES for multipoll IORING_OP_ADD
>>>>>>
>>>>>>    fs/io_uring.c                 | 34 +++++++++++++++++++++++++++++-----
>>>>>>    include/uapi/linux/io_uring.h |  8 +++-----
>>>>>>    2 files changed, 32 insertions(+), 10 deletions(-)
>>>>>
>>>>> Let's do the simple cq_extra first. I don't see a huge need to add an
>>>>> IOSQE flag for this, probably best to just keep this on a per opcode
>>>>> basis for now, which also then limits the code path to just touching
>>>>> poll for now, as nothing else supports multishot CQEs at this point.
>>>>>
>>>> gotcha.
>>>> a small issue here:
>>>>    sqe-->sqe(link)-->sqe(link)-->sqe(link, multishot)-->sqe(drain)
>>>>
>>>> in the above case, assume the first 3 single-shot reqs have completed.
>>>> then I think the drian request won't be issued now unless the multishot request in the linkchain has been issued. The trick is: a multishot req
>>>> in a linkchain consumes cached_sq_head when io_get_sqe(), which means it
>>>> is counted in seq, but we will deduct the sqe when it is issued if we
>>>> want to do the job per opcode not in the main code path.
>>>> before the multishot req issued:
>>>>        all_sqes(4) - multishot_sqes(0) == all_cqes(3) - multishot_cqes(0)
>>>> after the multishot req issued:
>>>>        all_sqes(4) - multishot_sqes(1) == all_cqes(3) - multishot_cqes(0)
>>>
>>> Sorry, my statement is wrong. It's not "won't be issued now unless the
>>> multishot request in the linkchain has been issued". Actually I now
>>> think the drain req won't be issued unless the multishot request in the
>>> linkchain has completed. Because we may first check req_need_defer()
>>> then issue(req->link), so:
>>>     sqe0-->sqe1(link)-->sqe2(link)-->sqe3(link, multishot)-->sqe4(drain)
>>>
>>>    sqe2 is completed:
>>>      call req_need_defer:
>>>      all_sqes(4) - multishot_sqes(0) == all_cqes(3) - multishot_cqes(0)
>>>    sqe3 is issued:
>>>      all_sqes(4) - multishot_sqes(1) == all_cqes(3) - multishot_cqes(0)
>>>    sqe3 is completed:
>>>      call req_need_defer:
>>>      all_sqes(4) - multishot_sqes(1) == all_cqes(3) - multishot_cqes(0)
>>>
>>> sqe4 shouldn't wait sqe3.
>>
>> Do you mean it wouldn't if the patch is applied? Because any drain
>> request must wait for all requests submitted before to complete. And
>> so before issuing sqe4 it must wait for sqe3 __request__ to die, and
>> so for all sqe3's CQEs.
>>
>> previously
> 
> I think we need to agree on what multishot means for dependencies. Does
> it mean it just needs to trigger once? Or does it mean that it needs to
> be totally finished. The latter may obviously never happen, depending on
> the use case. Or it may be an expected condition because the caller will
> cancel it at some point.
> 
> The most logical view imho is that multishot changes nothing wrt drain.
> If you ask for drain before something executes and you are using
> multishot, then you need to understand that the multishot request needs
> to fully complete before that condition is true and your dependency can
> execute.
This makes sense, and the implementation would be quite simpler. but we
really need to document it somewhere so that users easily get to know
that they cannot put a drain req after some multishot reqs if they don't
want it to wait for them. Otherwise I worry about wrong use of it since
the meaning of 'put a drain req after some multishot reqs' isn't so
obvious:
     - does it waits for those multishot reqs to complete once
     - or does it waits for those ones to fully complete
     - or does it ignore those ones at all



> 


  reply	other threads:[~2021-04-09  6:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-07 11:23 [PATCH 5.13 v2] io_uring: maintain drain requests' logic Hao Xu
2021-04-07 11:23 ` [PATCH 1/3] io_uring: add IOSQE_MULTI_CQES/REQ_F_MULTI_CQES for multishot requests Hao Xu
2021-04-07 11:38   ` Pavel Begunkov
2021-04-07 11:23 ` [PATCH 2/3] io_uring: maintain drain logic " Hao Xu
2021-04-07 11:41   ` Pavel Begunkov
2021-04-07 11:23 ` [PATCH 3/3] io_uring: use REQ_F_MULTI_CQES for multipoll IORING_OP_ADD Hao Xu
2021-04-07 15:49 ` [PATCH 5.13 v2] io_uring: maintain drain requests' logic Jens Axboe
2021-04-08 10:16   ` Hao Xu
2021-04-08 11:43     ` Hao Xu
2021-04-08 12:22       ` Pavel Begunkov
2021-04-08 16:18         ` Jens Axboe
2021-04-09  6:15           ` Hao Xu [this message]
2021-04-09  7:05             ` Hao Xu
2021-04-09  7:50               ` Pavel Begunkov
2021-04-12 15:07                 ` Hao Xu
2021-04-12 15:29                   ` Hao Xu
2021-04-09  3:12         ` Hao Xu
2021-04-09  3:43           ` Hao Xu

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 \
    --in-reply-to=9fd3a4c0-488f-ca82-083a-78d448a1564e@linux.alibaba.com \
    [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