Re: [PATCH for-5.16 0/2] async hybrid, a new way for pollable requests

[Pavel Begunkov <asml.silence@gmail.com>]

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