public inbox for [email protected]
 help / color / mirror / Atom feed
From: Pavel Begunkov <[email protected]>
To: Hao Xu <[email protected]>, Jens Axboe <[email protected]>
Cc: [email protected], Joseph Qi <[email protected]>
Subject: Re: [PATCH for-5.16 0/2] async hybrid, a new way for pollable requests
Date: Tue, 12 Oct 2021 12:39:12 +0100	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 10/11/21 04:08, Hao Xu wrote:
> 在 2021/10/9 下午8:51, Pavel Begunkov 写道:
>> On 10/8/21 13:36, Hao Xu wrote:
>>> this is a new feature for pollable requests, see detail in commit
>>> message.
>>
>> It really sounds we should do it as a part of IOSQE_ASYNC, so
>> what are the cons and compromises?
> I wrote the pros and cons here:
> https://github.com/axboe/liburing/issues/426#issuecomment-939221300

I see. The problem is as always, adding extra knobs, which users
should tune and it's not exactly clear where to use what. Not specific
to the new flag, there is enough confusion around IOSQE_ASYNC, but it
only makes it worse. It would be nice to have it applied
"automatically".

Say, with IOSQE_ASYNC the copy is always (almost) done by io-wq but
there is that polling optimisation on top. Do we care enough about
copying specifically in task context to have a different flag?

a quick question, what is "tps" in "IOSQE_ASYNC: 76664.151 tps"?

>>> Hao Xu (2):
>>>    io_uring: add IOSQE_ASYNC_HYBRID flag for pollable requests
>>
>> btw, it doesn't make sense to split it into two patches
> Hmm, I thought we should make adding a new flag as a separate patch.
> Could you give me more hints about the considerration here?

You can easily ignore it, just looked weird to me. Let's try to
phrase it:

1) 1/2 doesn't do anything useful w/o 2/2, iow it doesn't feel like
an atomic change. And it would be breaking the userspace, if it's
not just a hint flag.

2) it's harder to read, you search the git history, find the
implementation (and the flag is already there), you think what's
happening here, where the flag was used and so to find out that
it was added separately a commit ago.

3) sometimes it's done similarly because the API change is not
simple, but it's not the case here.
By similarly I mean the other way around, first implement it
internally, but not exposing any mean to use it, and adding
the userspace API in next commits.

>>>    io_uring: implementation of IOSQE_ASYNC_HYBRID logic
>>>
>>>   fs/io_uring.c                 | 48 +++++++++++++++++++++++++++++++----
>>>   include/uapi/linux/io_uring.h |  4 ++-
>>>   2 files changed, 46 insertions(+), 6 deletions(-)
>>>
>>
> 

-- 
Pavel Begunkov

  reply	other threads:[~2021-10-12 11:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-08 12:36 [PATCH for-5.16 0/2] async hybrid, a new way for pollable requests Hao Xu
2021-10-08 12:36 ` [PATCH 1/2] io_uring: add IOSQE_ASYNC_HYBRID flag " Hao Xu
2021-10-08 12:36 ` [PATCH 2/2] io_uring: implementation of IOSQE_ASYNC_HYBRID logic Hao Xu
2021-10-09 12:46   ` Pavel Begunkov
2021-10-11  8:55     ` Hao Xu
2021-10-11  8:58       ` Hao Xu
2021-10-09 12:51 ` [PATCH for-5.16 0/2] async hybrid, a new way for pollable requests Pavel Begunkov
2021-10-11  3:08   ` Hao Xu
2021-10-12 11:39     ` Pavel Begunkov [this message]
2021-10-14  8:53       ` Hao Xu
2021-10-14  9:20         ` Hao Xu
2021-10-14 13:53         ` Hao Xu
2021-10-14 14:17         ` Pavel Begunkov

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] \
    [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