Re: io_uring: O_NONBLOCK/IOCB_NOWAIT/RWF_NOWAIT mess

["Stefan Bühler" <source@stbuehler.de>]

Hi,

On 23.04.19 22:31, Jens Axboe wrote:
> On 4/23/19 1:06 PM, Stefan Bühler wrote:
>> 2. {read,write}_iter and FMODE_NOWAIT / IOCB_NOWAIT is broken at the vfs
>> layer: vfs_{read,write} should set IOCB_NOWAIT if O_NONBLOCK is set when
>> they call {read,write}_iter (i.e. init_sync_kiocb/iocb_flags needs to
>> convert the flag).
>>
>> And all {read,write}_iter should check IOCB_NOWAIT instead of O_NONBLOCK
>> (hi there pipe.c!), and set FMODE_NOWAIT if they support IOCB_NOWAIT.
>>
>> {read,write}_iter should only queue the IOCB though if is_sync_kiocb()
>> returns false (i.e. if ki_callback is set).
> 
> That's a trivial fix. I agree that it should be done.

Doesn't look trivial to me.

Various functions take rwf_t flags, e.g. do_readv, which is called with
0 from readv and with flags from userspace in preadv2.

Now is preadv2() supposed to be non-blocking if the file has O_NONBLOCK,
or only if RWF_NOWAIT was passed?

Other places seem (at least to me) explicitly mean "please block" if
they don't pass RWF_NOWAIT, e.g. ovl_read_iter from fs/overlayfs, which
uses ovl_iocb_to_rwf to convert iocb flags back to rwf.

Imho the clean way is to ignore O_NONBLOCK when there are rwf_t flags;
e.g. kiocb_set_rw_flags should unset IOCB_NOWAIT if RWF_NOWAIT was not set.

But then various functions (like readv) will need to create rwf_t
"default" flags from a file (similar to iocb_flags) instead of using 0.
And ovl_iocb_to_rwf should probably be used in more places as well.

There is also generic_file_splice_read, which should use
SPLICE_F_NONBLOCK to trigger IOCB_NOWAIT; again it is unclear whether
O_NONBLOCK should trigger IOCB_NOWAIT too (do_sendfile explicitly does
NOT with a "need to debate" comment).


I don't think I'm the right person to do this - I think it requires a
deeper understanding of all the code involved.

I do have patches for pipe.c and and socket.c to ignore O_NONBLOCK, use
IOCB_NOWAIT and set FMODE_NOAWAIT after the fs part is ready.

cheers,
Stefan