Re: [PATCH] Implement the mlock2 function

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

On 27/11/2017 16:46, Florian Weimer wrote:
> On 11/27/2017 07:40 PM, Adhemerval Zanella wrote:
>>
>>
>> On 27/11/2017 14:43, Florian Weimer wrote:
>>> On 11/27/2017 05:35 PM, Adhemerval Zanella wrote:
>>>>
>>>>
>>>> On 27/11/2017 14:07, Florian Weimer wrote:
>>>>> On 11/27/2017 02:11 PM, Adhemerval Zanella wrote:
>>>>>> This is a reasonable approach and I am ok with this patch with the
>>>>>> INLINE_SYSCALL_CALL change. I wonder if it is worth to add a similar
>>>>>> change to p{read,write}v2 to return ENOSUP in the case of ENOSYS.
>>>>>
>>>>> My copy of the manual page says that EINVAL is used there as well.
>>>>>
>>>>> Thanks,
>>>>> Florian
>>>>
>>>> Indeed manpages [1] states that EINVAL is returned, but our documentation
>>>> states otherwise:
>>>>
>>>> manual/llio.texi
>>>>
>>>> 1286 @item EOPNOTSUPP
>>>> 1287
>>>> 1288 @c The default sysdeps/posix code will return it for any flags value
>>>> 1289 @c different than 0.
>>>> 1290 An unsupported @var{flags} was used.
>>>>
>>>> Also, "tst-preadvwritev" on a 4.13.0-17-generic indeed generates Linux
>>>> EOPNOTSUPP (ENOSUP):
>>>>
>>>> [pid  7896] preadv2(3,  <unfinished ...>
>>>> [pid  7895] <... rt_sigaction resumed> {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
>>>> [pid  7896] <... preadv2 resumed> [{iov_base=0x7ffca77846d0, iov_len=32}], 1, 0, 0x10 /* RWF_??? */) = -1 EOPNOTSUPP (Operation not supported)
>>>> [pid  7895] wait4(7896,  <unfinished ...>
>>>> [pid  7896] pwritev2(3, [{iov_base="\0\0\0\0\0\0\0\0\241H\363?\244\177\0\0@Gx\247\374\177\0\0\0\0\0\0\0\0\0\0", iov_len=32}], 1, 0, 0x10 /* RWF_??? */) = -1 EOPNOTSUPP (Operation not supported)
>>>>
>>>> So I think it would be worth to change p{read,write}v2 on GLIBC to
>>>> return EINVAL for invalid flags. I will prepare a patch.
>>>>
>>>> [1] http://man7.org/linux/man-pages/man2/preadv2.2.html
>>>
>>> Typo?  Shouldn't we match the kernel behavior, so fail with EOPNOTSUPP?
>>
>> Not for ENOSYS. And I though about following manpages definition, but
>> thinking twice I agree following the kernel would be better.  I still
>> think it will be a small improvement to handle ENOSYS as ENOSUP as you
>> did for mlock2 and EINVAL.
> 
> Agreed: if you do zero-flag emulation using pwritev in user space, returning the kernel unknown flag used by pwritev2 (here: EOPNOTSUPP) when *any* non-zero flag is unknown by the kernel (because pwritev2 is not implemented) is the right approach.

We are already doing the correct thing for p{read,write}v2: if ENOSYS is 
returned Linux implementation will return ENOTSUP for flags different
than 0 otherwise call p{read,write}v.