Re: [PATCH 2/4] Adjust kernel-features.h for sendmmsg/recvmmsg

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

On 21-03-2016 20:26, Joseph Myers wrote:
> On Mon, 21 Mar 2016, Adhemerval Zanella wrote:
> 
>>  * Remove the __ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL for ports that
>>    for current minimum supported kernel already have direct recvmmsg
>>    syscall support (microblaze, powerpc, and spart).
>>
>>  * Add __ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL define for ports that
>>    still uses socketcall interface (i386, m68k, s390).
> 
> This inverts the present semantics for the _WITH_SOCKETCALL macros.  At 
> present, it's defined when the syscall should be always preferred to 
> socketcall (even if the syscall isn't guaranteed to be available); you're 
> changing it to mean that the syscall should never be preferred to 
> socketcall unless guaranteed to be available.  Obviously such a change 
> requires an update to the comment in the generic kernel-features.h 
> defining the semantics of __ASSUME_ACCEPT4_SYSCALL_WITH_SOCKETCALL (and 
> that one must be updated with the other two - having different macro 
> semantics for the three functions would be excessively confusing).  
> However, I think it would be much better to use a clearly different macro 
> name if inverting the semantics (again, for all three functions in 
> question).  And similarly, any refactoring of implementations should be 
> done for all three functions together.
> 
> It's true that even with the present semantics, 
> __ASSUME_*_SYSCALL_WITH_SOCKETCALL definitions in the 
> architecture-specific header are unnecessary but harmless in the case 
> where the syscall is assumed to be present with the minimum supported 
> kernel.

In fact I wasn't sure which would be best approach, but base on your comments
and the wrong assumptions I had with the x86 'fixes' I think best approach
would be:

  1. Use syscall if __NR_recvmmsg is defined
  2. Otherwise if socketcall is __ASSUME_RECVMMSG_SYSCALL is defined
  3. Otherwise if runtime socketcall tests if __ASSUME_SOCKETCALL

For current support architectures and kernels, only microblaze and
s390 will use 2 (with s390 using 1. with kernel 4.3+).  The tests
3 will be used only for i386 with default minimum kernel version
(2.6.32) and I think we can remove the stub since it will be 
always defined with the minimum kernel headers we are supporting
(3.2).

> 
>> This approach also fixes the following i386/x86_64 issue:
>>
>>  * For i686 with kernel 3.2.0 but with --enable-kernel=2.6.32 (minimum
>>    supported kernel) will use direct syscall where it should use runtime
>>    socketcall
> 
> Why is this a bug?  For all released i686 kernels, recvmmsg is either 
> available through both socketcall and the syscall, or through neither 
> socketcall and the syscall, and likewise for sendmmsg.  And where both are 
> available, it's better to use the syscall than socketcall (on general 
> principles of socketcall being deprecated).  So libc built for i686 should 
> never try to use socketcall at all for recvmmsg or sendmmsg under any 
> circumstances (it *should* try for accept4 if the minimum kernel is below 
> 4.3, because accept4 is available through socketcall on more kernels than 
> the syscall).
> 
>>  * For x86_64 with kernel 3.2.0 but with --enable-kernel=2.6.32 (minimum
>>    kernel) will use syscall where it should use stub.
> 
> No, it should use the syscall (which might or might not exist at runtime).  
> It's always wrong to use the stub implementation if the syscall number is 
> known and so the syscall might be available at runtime.
> 
> The only reason stubs might be used at all for any of accept4, recvmmsg, 
> sendmmsg on any GNU/Linux architecture is on non-socketcall architectures 
> where the syscall number is unavailable in the kernel headers used.  Given 
> the requirement for >= 3.2 headers on all architectures, this applies to 
> accept4 on ia64, and no other cases.
>