IO-Uring Archive on lore.kernel.org
 help / color / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: "Pavel Begunkov" <asml.silence@gmail.com>,
	"Carter Li 李通洲" <carter.li@eoitek.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	io-uring <io-uring@vger.kernel.org>
Subject: Re: [ISSUE] The time cost of IOSQE_IO_LINK
Date: Mon, 17 Feb 2020 11:30:30 -0800
Message-ID: <64b82f99-f468-627a-3530-c72e29e747dd@kernel.dk> (raw)
In-Reply-To: <d7f5d742-609e-0de6-11ce-e621e4ec8d68@gmail.com>

On 2/17/20 3:30 AM, Pavel Begunkov wrote:
> On 2/17/2020 1:23 AM, Jens Axboe wrote:
>> On 2/16/20 12:06 PM, Pavel Begunkov wrote:
>>> On 15/02/2020 09:01, Jens Axboe wrote:
>>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>>> index fb94b8bac638..530dcd91fa53 100644
>>>> @@ -4630,6 +4753,14 @@ static void __io_queue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>>  	 */
>>>>  	if (ret == -EAGAIN && (!(req->flags & REQ_F_NOWAIT) ||
>>>>  	    (req->flags & REQ_F_MUST_PUNT))) {
>>>> +
>>>> +		if (io_arm_poll_handler(req, &retry_count)) {
>>>> +			if (retry_count == 1)
>>>> +				goto issue;
>>>
>>> Better to sqe=NULL before retrying, so it won't re-read sqe and try to
>>> init the req twice.
>>
>> Good point, that should get cleared after issue.
>>
>>> Also, the second sync-issue may -EAGAIN again, and as I remember,
>>> read/write/etc will try to copy iovec into req->io. But iovec is
>>> already in req->io, so it will self memcpy(). Not a good thing.
>>
>> I'll look into those details, that has indeed reared its head before.
>>
>>>> +			else if (!retry_count)
>>>> +				goto done_req;
>>>> +			INIT_IO_WORK(&req->work, io_wq_submit_work);
>>>
>>> It's not nice to reset it as this:
>>> - prep() could set some work.flags
>>> - custom work.func is more performant (adds extra switch)
>>> - some may rely on specified work.func to be called. e.g. close(), even though
>>> it doesn't participate in the scheme
>>
>> It's totally a hack as-is for the "can't do it, go async". I did clean
> 
> And I don't understand lifetimes yet... probably would need a couple of
> questions later.
> 
>> this up a bit (if you check the git version, it's changed quite a bit),
> 
> That's what I've been looking at
> 
>> but it's still a mess in terms of that and ->work vs union ownership.
>> The commit message also has a note about that.
>>
>> So more work needed in that area for sure.
> 
> Right, I just checked a couple of things for you

Appreciate it! I'll send out the series for easier review and
commenting. It's getting better, but still a bit rough around the edges.
It passes the test suite now, except for:

- socket-rw and connect - both of these don't get a poll wakeup from the
  networking side when the connection is made (or data is sent), which
  is very odd. So we just keep waiting for that, and nothing happens.
  Need to figure out why there's never a wakeup on it.

-- 
Jens Axboe


  reply index

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-12 16:31 Carter Li 李通洲
2020-02-12 17:11 ` Jens Axboe
2020-02-12 17:22   ` Jens Axboe
2020-02-12 17:29     ` Jens Axboe
2020-02-13  0:33   ` Carter Li 李通洲
2020-02-13 15:08     ` Pavel Begunkov
2020-02-13 15:14       ` Jens Axboe
2020-02-13 15:51         ` Carter Li 李通洲
2020-02-14  1:25           ` Carter Li 李通洲
2020-02-14  2:45             ` Jens Axboe
2020-02-14  5:03               ` Jens Axboe
2020-02-14 15:32                 ` Peter Zijlstra
2020-02-14 15:47                   ` Jens Axboe
2020-02-14 16:18                     ` Jens Axboe
2020-02-14 17:52                       ` Jens Axboe
2020-02-14 20:44                         ` Jens Axboe
2020-02-15  0:16                           ` Carter Li 李通洲
2020-02-15  1:10                             ` Jens Axboe
2020-02-15  1:25                               ` Carter Li 李通洲
2020-02-15  1:27                                 ` Jens Axboe
2020-02-15  6:01                                   ` Jens Axboe
2020-02-15  6:32                                     ` Carter Li 李通洲
2020-02-15 15:11                                       ` Jens Axboe
2020-02-16 19:06                                     ` Pavel Begunkov
2020-02-16 22:23                                       ` Jens Axboe
2020-02-17 10:30                                         ` Pavel Begunkov
2020-02-17 19:30                                           ` Jens Axboe [this message]
2020-02-16 23:06                                       ` Jens Axboe
2020-02-16 23:07                                         ` Jens Axboe
2020-02-17 12:09                           ` Peter Zijlstra
2020-02-17 16:12                             ` Jens Axboe
2020-02-17 17:16                               ` Jens Axboe
2020-02-17 17:46                                 ` Peter Zijlstra
2020-02-17 18:16                                   ` Jens Axboe
2020-02-18 13:13                                     ` Peter Zijlstra
2020-02-18 14:27                                       ` [PATCH] asm-generic/atomic: Add try_cmpxchg() fallbacks Peter Zijlstra
2020-02-18 14:40                                         ` Peter Zijlstra
2020-02-20 10:30                                         ` Will Deacon
2020-02-20 10:37                                           ` Peter Zijlstra
2020-02-20 10:39                                             ` Will Deacon
2020-02-18 14:56                                       ` [ISSUE] The time cost of IOSQE_IO_LINK Oleg Nesterov
2020-02-18 15:07                                         ` Oleg Nesterov
2020-02-18 15:38                                           ` Peter Zijlstra
2020-02-18 16:33                                             ` Jens Axboe
2020-02-18 15:07                                         ` Peter Zijlstra
2020-02-18 15:50                                           ` [PATCH] task_work_run: don't take ->pi_lock unconditionally Oleg Nesterov
2020-02-20 16:39                                             ` Peter Zijlstra
2020-02-20 17:22                                               ` Oleg Nesterov
2020-02-20 17:49                                                 ` Peter Zijlstra
2020-02-21 14:52                                                   ` Oleg Nesterov
2020-02-24 18:47                                                     ` Jens Axboe
2020-02-28 19:17                                                       ` Jens Axboe
2020-02-28 19:25                                                         ` Peter Zijlstra
2020-02-28 19:28                                                           ` Jens Axboe
2020-02-28 20:06                                                             ` Peter Zijlstra
2020-02-28 20:15                                                               ` Jens Axboe
2020-02-18 16:46                                       ` [ISSUE] The time cost of IOSQE_IO_LINK Jens Axboe
2020-02-18 16:52                                         ` Jens Axboe
2020-02-18 13:13                               ` Peter Zijlstra

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=64b82f99-f468-627a-3530-c72e29e747dd@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=asml.silence@gmail.com \
    --cc=carter.li@eoitek.com \
    --cc=io-uring@vger.kernel.org \
    --cc=peterz@infradead.org \
    /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

IO-Uring Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/io-uring/0 io-uring/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 io-uring io-uring/ https://lore.kernel.org/io-uring \
		io-uring@vger.kernel.org
	public-inbox-index io-uring

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.io-uring


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git