Re: [PATCH] io_uring: fix blocking inline submission

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

On 6/9/21 1:42 PM, Hao Xu wrote:
> 在 2021/6/9 下午7:07, Pavel Begunkov 写道:
>> There is a complaint against sys_io_uring_enter() blocking if it submits
>> stdin reads. The problem is in __io_file_supports_async(), which
>> sees that it's a cdev and allows it to be processed inline.
>>
>> Punt char devices using generic rules of io_file_supports_async(),
>> including checking for presence of *_iter() versions of rw callbacks.
>> Apparently, it will affect most of cdevs with some exceptions like
>> null and zero devices.
>>
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>
>> "...For now, just ensure that anything potentially problematic is done
>> inline". I believe this part is outdated, but what use cases we miss?
>> Anything that we care about?
>>
>> IMHO the best option is to do like in this patch and add
>> (read,write)_iter(), to places we care about.
>>
>> /dev/[u]random, consoles, any else?
>>
> This reminds me another thing, once I did nowait read on a brd(block
> ramdisk), I saw a 10%~30% regression after __io_file_supports_async()
> added. brd is bio based device (block layer doesn't support nowait IO
> for this kind of device), so theoretically it makes sense to punt it to
> iowq threads in advance in __io_file_supports_async(), but actually
> what originally happen is: IOCB_NOWAIT is not delivered to block
> layer(REQ_NOWAIT) and then the IO request is executed inline (It seems

IIUC we fixed it was fixed by

f8b78caf21d5bc3fcfc40c18898f9d52ed1451a5 ("block: don't ignore REQ_NOWAIT for direct IO")

> brd device won't block). This finally makes 'check it in advance'
> slower..

Trying to understand what kind of slower it is... It makes an attempt
to execute it inline. Does it return -EAGAIN? Always succeed
submission (e.g. queued for truly async execution ki_complete style)?

>>   fs/io_uring.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index 42380ed563c4..44d1859f0dfb 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -2616,7 +2616,7 @@ static bool __io_file_supports_async(struct file *file, int rw)
>>               return true;
>>           return false;
>>       }
>> -    if (S_ISCHR(mode) || S_ISSOCK(mode))
>> +    if (S_ISSOCK(mode))
>>           return true;
>>       if (S_ISREG(mode)) {
>>           if (IS_ENABLED(CONFIG_BLOCK) &&
>>
> 

-- 
Pavel Begunkov