Re: [PATCH] posix: Add p{read,write}v2 RWF_NOWAIT flag (BZ#21738)

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 10/07/2017 16:34, Zack Weinberg wrote:
> On Mon, Jul 10, 2017 at 3:24 PM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> Linux 4.12 (b745fafaf70c0a98a2e1e7ac8cb14542889ceb0e) adds a new
>> p{read,write}v2 flag RWF_NOWAIT.  This patch adds it for linux
>> uio-ext.h header along with RWF_SUPPORTED (a mask with all supported
>> flags).
> 
> LGTM with a couple of notes on the documentation:
> 
>> +@item RWF_NOWAIT
>> +Set @code{preadv2} to return -EAGAIN if operation would block.
> 
> This is awkward grammar.  Also, this function sets errno, doesn't it?
> "Return -EAGAIN" is an intra-kernel convention, not what user space
> sees, which is what the manual should document.
> 
> Suggest instead "Use nonblocking mode for this operation; that is,
> this call to @code{preadv2} will fail and set @code{errno} to
> @code{EAGAIN} if the operation would block."

> On Mon, Jul 10, 2017 at 3:34 PM, Zack Weinberg <zackw@panix.com> wrote:
>>
>> Suggest instead "Use nonblocking mode for this operation; that is,
>> this call to @code{preadv2} will fail and set @code{errno} to
>> @code{EAGAIN} if the operation would block."
> 
> "... this call to @code{preadv2} or @code{pwritev2} ..." It does work
> in both, right?
> 
> zw

Indeed the wording is unpolished, I will use your version instead,
although the topic is referenced to preadv2 (on pwritev2 topic there is
a note it accepts the same flags from preadv2).

> 
>> +@item RWF_SUPPORTED
>> +Mask with all supported flags.
> 
> It's important to be clear that this does not necessarily reflect the
> running kernel:  "Bitmask of all @samp{RWF_*} flags that have so far
> been defined.  This is a compile-time constant; the running kernel
> does not necessarily support all of the flags defined in this header.
> New flags were most recently added in Linux 4.12."

>From Florian's comment I also think it does not worth the trouble to
provide it when it might be have different values than running kernel.
I will remove it.

> 
>> +#define RWF_NOWAIT     0x00000008 /* per-IO, return -EAGAIN if operation
>> +                                     would block */
> 
> Same as above: "per-IO nonblocking mode" would be sufficient here, I think.

Right (although I just copied and pasted the kernel comment).