Hi,
I am a little bit disappointed that this seems to get ignored.
I will reply to this mail with some patches, but I really think someone
with a deeper understanding needs to think this through.
If there were some clear documentation/specification how those flags are
supposed to work (especially in combination) it would be much easier to
tell what is broken and how to fix it...
cheers,
Stefan
On 28.04.19 17:54, Stefan Bühler wrote:
> 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
>
