| Message ID | 1458592867-3057-3-git-send-email-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
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. > 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.
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. >
diff --git a/sysdeps/unix/sysv/linux/i386/kernel-features.h b/sysdeps/unix/sysv/linux/i386/kernel-features.h index 148963c..3e25476 100644 --- a/sysdeps/unix/sysv/linux/i386/kernel-features.h +++ b/sysdeps/unix/sysv/linux/i386/kernel-features.h @@ -21,10 +21,14 @@ #define __ASSUME_SOCKETCALL 1 /* The recvmmsg syscall was added for i386 in 2.6.33. */ -#define __ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL 1 +#if __LINUX_KERNEL_VERSION >= 0x020621 +# define __ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL 1 +#endif /* The sendmmsg syscall was added for i386 in 3.0. */ -#define __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL 1 +#if __LINUX_KERNEL_VERSION >= 0x030000 +# define __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL 1 +#endif /* Direct socketcalls available with kernel 4.3. */ #if __LINUX_KERNEL_VERSION >= 0x040300 diff --git a/sysdeps/unix/sysv/linux/m68k/kernel-features.h b/sysdeps/unix/sysv/linux/m68k/kernel-features.h index dec04f0..0945779 100644 --- a/sysdeps/unix/sysv/linux/m68k/kernel-features.h +++ b/sysdeps/unix/sysv/linux/m68k/kernel-features.h @@ -19,6 +19,8 @@ /* m68k uses socketcall. */ #define __ASSUME_SOCKETCALL 1 +#define __ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL 1 +#define __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL 1 /* Direct socketcalls available with kernel 4.3. */ #if __LINUX_KERNEL_VERSION >= 0x040300 diff --git a/sysdeps/unix/sysv/linux/microblaze/kernel-features.h b/sysdeps/unix/sysv/linux/microblaze/kernel-features.h index db471ef..e25b6ed 100644 --- a/sysdeps/unix/sysv/linux/microblaze/kernel-features.h +++ b/sysdeps/unix/sysv/linux/microblaze/kernel-features.h @@ -18,6 +18,7 @@ /* MicroBlaze uses socketcall. */ #define __ASSUME_SOCKETCALL 1 +#define __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL 1 /* All supported kernel versions for MicroBlaze have these syscalls. */ #define __ASSUME_SOCKET_SYSCALL 1 @@ -36,9 +37,6 @@ #define __ASSUME_GETSOCKOPT_SYSCALL 1 #define __ASSUME_SETSOCKOPT_SYSCALL 1 -/* Support for the accept4 and recvmmsg syscalls was added in 2.6.33. */ -#define __ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL 1 - /* Support for the futimesat syscall was added in 2.6.33. */ #define __ASSUME_FUTIMESAT 1 diff --git a/sysdeps/unix/sysv/linux/mips/mips32/recvmmsg.c b/sysdeps/unix/sysv/linux/mips/mips32/recvmmsg.c deleted file mode 100644 index c2a3440..0000000 --- a/sysdeps/unix/sysv/linux/mips/mips32/recvmmsg.c +++ /dev/null @@ -1,29 +0,0 @@ -/* Copyright (C) 2010-2016 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library. If not, see - <http://www.gnu.org/licenses/>. */ - -/* Avoid recvmmsg.c trying to use a definition based on the socketcall - syscall and internal_recvmmsg.S. */ - -#include <errno.h> -#include <sys/socket.h> - -#include <sysdep-cancel.h> -#include <sys/syscall.h> - -#undef __NR_socketcall - -#include <sysdeps/unix/sysv/linux/recvmmsg.c> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sendmmsg.c b/sysdeps/unix/sysv/linux/mips/mips32/sendmmsg.c deleted file mode 100644 index 7cd4e15..0000000 --- a/sysdeps/unix/sysv/linux/mips/mips32/sendmmsg.c +++ /dev/null @@ -1,29 +0,0 @@ -/* Copyright (C) 2011-2016 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library. If not, see - <http://www.gnu.org/licenses/>. */ - -/* Avoid sendmmsg.c trying to use a definition based on the socketcall - syscall and internal_sendmmsg.S. */ - -#include <errno.h> -#include <sys/socket.h> - -#include <sysdep-cancel.h> -#include <sys/syscall.h> - -#undef __NR_socketcall - -#include <sysdeps/unix/sysv/linux/sendmmsg.c> diff --git a/sysdeps/unix/sysv/linux/powerpc/kernel-features.h b/sysdeps/unix/sysv/linux/powerpc/kernel-features.h index 5449f18..825c0f0 100644 --- a/sysdeps/unix/sysv/linux/powerpc/kernel-features.h +++ b/sysdeps/unix/sysv/linux/powerpc/kernel-features.h @@ -37,7 +37,4 @@ #define __ASSUME_GETSOCKOPT_SYSCALL 1 #define __ASSUME_SETSOCKOPT_SYSCALL 1 -/* The sendmmsg syscall was added for PowerPC in 3.0. */ -#define __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL 1 - #include_next <kernel-features.h> diff --git a/sysdeps/unix/sysv/linux/recvmmsg.c b/sysdeps/unix/sysv/linux/recvmmsg.c index bf18260..fb3f325 100644 --- a/sysdeps/unix/sysv/linux/recvmmsg.c +++ b/sysdeps/unix/sysv/linux/recvmmsg.c @@ -16,39 +16,26 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -#include <errno.h> #include <sys/socket.h> - #include <sysdep-cancel.h> -#include <sys/syscall.h> -#include <kernel-features.h> - -/* Do not use the recvmmsg syscall on socketcall architectures unless - it was added at the same time as the socketcall support or can be - assumed to be present. */ -#if defined __ASSUME_SOCKETCALL \ - && !defined __ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL \ - && !defined __ASSUME_RECVMMSG_SYSCALL -# undef __NR_recvmmsg -#endif +#include <socketcall.h> +#include <shlib-compat.h> -#ifdef __NR_recvmmsg +#ifdef __ASSUME_RECVMMSG_SYSCALL int recvmmsg (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags, struct timespec *tmo) { return SYSCALL_CANCEL (recvmmsg, fd, vmessages, vlen, flags, tmo); } -#elif defined __NR_socketcall -# include <socketcall.h> -# ifdef __ASSUME_RECVMMSG_SOCKETCALL +#elif defined __ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL int recvmmsg (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags, struct timespec *tmo) { return SOCKETCALL_CANCEL (recvmmsg, fd, vmessages, vlen, flags, tmo); } -# else +#elif defined __ASSUME_SOCKETCALL static int have_recvmmsg; int @@ -87,7 +74,6 @@ recvmmsg (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags, __set_errno (ENOSYS); return -1; } -# endif /* __ASSUME_RECVMMSG_SOCKETCALL */ -#else +#else /* __ASSUME_RECVMMSG_SOCKETCALL */ # include <socket/recvmmsg.c> #endif diff --git a/sysdeps/unix/sysv/linux/s390/kernel-features.h b/sysdeps/unix/sysv/linux/s390/kernel-features.h index b3edee4..4656c4a 100644 --- a/sysdeps/unix/sysv/linux/s390/kernel-features.h +++ b/sysdeps/unix/sysv/linux/s390/kernel-features.h @@ -19,6 +19,8 @@ /* S/390 uses socketcall. */ #define __ASSUME_SOCKETCALL 1 +#define __ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL 1 +#define __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL 1 /* Direct socketcalls available with kernel 4.3. */ #if __LINUX_KERNEL_VERSION >= 0x040300 diff --git a/sysdeps/unix/sysv/linux/sendmmsg.c b/sysdeps/unix/sysv/linux/sendmmsg.c index 6e0d46b..37f0ec9 100644 --- a/sysdeps/unix/sysv/linux/sendmmsg.c +++ b/sysdeps/unix/sysv/linux/sendmmsg.c @@ -16,23 +16,11 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -#include <errno.h> #include <sys/socket.h> - #include <sysdep-cancel.h> -#include <sys/syscall.h> -#include <kernel-features.h> - -/* Do not use the sendmmsg syscall on socketcall architectures unless - it was added at the same time as the socketcall support or can be - assumed to be present. */ -#if defined __ASSUME_SOCKETCALL \ - && !defined __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL \ - && !defined __ASSUME_SENDMMSG_SYSCALL -# undef __NR_sendmmsg -#endif +#include <socketcall.h> -#ifdef __NR_sendmmsg +#ifdef __ASSUME_SENDMMSG_SYSCALL int __sendmmsg (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags) { @@ -40,15 +28,15 @@ __sendmmsg (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags) } libc_hidden_def (__sendmmsg) weak_alias (__sendmmsg, sendmmsg) -#elif defined __NR_socketcall -# include <socketcall.h> -# ifdef __ASSUME_SENDMMSG_SOCKETCALL +#elif defined __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL int __sendmmsg (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags) { return SOCKETCALL_CANCEL (sendmmsg, fd, vmessages, vlen, flags); } -# else +libc_hidden_def (__sendmmsg) +weak_alias (__sendmmsg, sendmmsg) +#elif defined __ASSUME_SOCKETCALL static int have_sendmmsg; int @@ -85,9 +73,8 @@ __sendmmsg (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags) __set_errno (ENOSYS); return -1; } -# endif /* __ASSUME_SENDMMSG_SOCKETCALL */ libc_hidden_def (__sendmmsg) weak_alias (__sendmmsg, sendmmsg) -#else +#else /* __ASSUME_SENDMMSG_SOCKETCALL */ # include <socket/sendmmsg.c> #endif diff --git a/sysdeps/unix/sysv/linux/sparc/kernel-features.h b/sysdeps/unix/sysv/linux/sparc/kernel-features.h index 386f230..486036d 100644 --- a/sysdeps/unix/sysv/linux/sparc/kernel-features.h +++ b/sysdeps/unix/sysv/linux/sparc/kernel-features.h @@ -23,12 +23,6 @@ /* The accept4 syscall was added for SPARC in 2.6.28. */ #define __ASSUME_ACCEPT4_SYSCALL_WITH_SOCKETCALL 1 -/* The recvmmsg syscall was added for SPARC in 2.6.33. */ -#define __ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL 1 - -/* The sendmmsg syscall was added for SPARC in 3.0. */ -#define __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL 1 - #include_next <kernel-features.h> /* 32-bit SPARC kernels do not support
This patch cleanups the __ASSUME_RECVMMSG_* and __ASSUME_SENDMMSG_* usage in linux recvmmsg and sendmmsg implementation. It also fixes a bug on i686/x86_64 when building with 2.6.32 kernel support. The changes in this patches are the following: * 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). * Remove the __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL for ports that for current minimum supported kernel alread have direct sendmmsg syscall support (powerpc, sparc). * Add __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL define for ports that still uses socketcall interface (i386, m68k, microblaze, and s390) Along with the define changes, the linux recvmmsg and sendmmsg implementation now uses a different strategy to use direct syscall, socketcall, runtime socketcall (which issues a socketcall and update have_*), and stub function: 1. If the architecture defines __ASSUME_{RECVMMSG,SENDMMSG}_SYSCALL the direct syscall is used instead. This is the default behavior for most ports that do not support socketcall. 2. Otherwise, if the architecture defines __ASSUME_{RECVMMSG,SENDMMSG}_SYSCALL_WITH_SOCKETCALL the socketcall interface will be used instead. Different than the default __ASSUME_{RECVMMSG,SENDMMSG}_SOCKETCALL, this define is set per architecture. 3. Otherwise, if the architecure define __ASSUME_SOCKETCALL the runtime socketcall call will be used which will test if the kernel supports {sendmmsg,recvmmsg} and set have_{recvmmsg,sendmmsg} accordingly. 4. Otherwise the stub warning implementation will be use. 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 * 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. Tested on i686, x86_64, aarch64, armhf, and powerpc64le. * sysdeps/unix/sysv/linux/i386/kernel-features.h (__ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL): Remove define. [__LINUX_KERNEL_VERSION > 0x030000] (__ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL): Add define. * sysdeps/unix/sysv/linux/m68k/kernel-features.h (__ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL): Likewise. (__ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL): Likewise. * sysdeps/unix/sysv/linux/microblaze/kernel-features.h (__ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL): Likewise. (__ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL): Remove define. * sysdeps/unix/sysv/linux/powerpc/kernel-features.h (__ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL): Likewise. * sysdeps/unix/sysv/linux/s390/kernel-features.h (__ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL): Add define. (__ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL): Likewise. * sysdeps/unix/sysv/linux/sh/kernel-features.h (__ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL): Remove define. * sysdeps/unix/sysv/linux/sparc/kernel-features.h (__ASSUME_RECVMMSG_SYSCALL_WITH_SOCKETCALL): Likewise. (__ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL): Likewise. * sysdeps/unix/sysv/linux/mips/mips32/recvmmsg.c: Remove file. * sysdeps/unix/sysv/linux/mips/mips32/sendmmsg.c: Likewise. * sysdeps/unix/sysv/linux/recvmmsg.c (recvmmsg): Refactor __ASSUME macro usage. * sysdeps/unix/sysv/linux/sendmmsg.c (__sendmmsg): Likewise. --- ChangeLog | 26 +++++++++++++++++++ sysdeps/unix/sysv/linux/i386/kernel-features.h | 8 ++++-- sysdeps/unix/sysv/linux/m68k/kernel-features.h | 2 ++ .../unix/sysv/linux/microblaze/kernel-features.h | 4 +-- sysdeps/unix/sysv/linux/mips/mips32/recvmmsg.c | 29 ---------------------- sysdeps/unix/sysv/linux/mips/mips32/sendmmsg.c | 29 ---------------------- sysdeps/unix/sysv/linux/powerpc/kernel-features.h | 3 --- sysdeps/unix/sysv/linux/recvmmsg.c | 26 +++++-------------- sysdeps/unix/sysv/linux/s390/kernel-features.h | 2 ++ sysdeps/unix/sysv/linux/sendmmsg.c | 27 ++++++-------------- sysdeps/unix/sysv/linux/sparc/kernel-features.h | 6 ----- 11 files changed, 50 insertions(+), 112 deletions(-) delete mode 100644 sysdeps/unix/sysv/linux/mips/mips32/recvmmsg.c delete mode 100644 sysdeps/unix/sysv/linux/mips/mips32/sendmmsg.c