Message ID | 5660A8D0.5090003@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On 12/3/15 6:40 PM, Paul E. Murphy wrote: > PPC kernel 4.4rc1 and newer support these syscalls. They should be > a little faster. This adds support for the common implementations. > Other architectures need only define the appropriate macros in > their kernel-features.h, if they can take advantage of them. > > 2015-12-03 Paul E. Murphy <murphyp@linux.vnet.ibm.com> > LGTM.
On 03-12-2015 18:40, Paul E. Murphy wrote: > PPC kernel 4.4rc1 and newer support these syscalls. They should be > a little faster. This adds support for the common implementations. > Other architectures need only define the appropriate macros in > their kernel-features.h, if they can take advantage of them. > > > diff --git a/sysdeps/unix/sysv/linux/msgctl.c b/sysdeps/unix/sysv/linux/msgctl.c > index a24dc38..86f34a2 100644 > --- a/sysdeps/unix/sysv/linux/msgctl.c > +++ b/sysdeps/unix/sysv/linux/msgctl.c > @@ -56,7 +56,11 @@ int > attribute_compat_text_section > __old_msgctl (int msqid, int cmd, struct __old_msqid_ds *buf) > { > +#ifdef __ASSUME_MSGCTL_SYSCALL > + return INLINE_SYSCALL (msgctl, 3, msqid, cmd, buf); > +#else > return INLINE_SYSCALL (ipc, 5, IPCOP_msgctl, msqid, cmd, 0, buf); > +#endif > } My inclination would to avoid the __ASSUME_XXX macros definition and just check if the current kernel header defines of not __NR_msgctl and use it as the condition to enable the wire-up syscall. It has the advantage of the avoid each enabled architecture to explicit add these macros to actually enable the new mechanism. The downside is you would not be able to set this new mechanism using --enable-kernel.
"Paul E. Murphy" <murphyp@linux.vnet.ibm.com> writes: > diff --git a/sysdeps/unix/sysv/linux/msgctl.c b/sysdeps/unix/sysv/linux/msgctl.c > index a24dc38..86f34a2 100644 > --- a/sysdeps/unix/sysv/linux/msgctl.c > +++ b/sysdeps/unix/sysv/linux/msgctl.c > @@ -56,7 +56,11 @@ int > attribute_compat_text_section > __old_msgctl (int msqid, int cmd, struct __old_msqid_ds *buf) > { > +#ifdef __ASSUME_MSGCTL_SYSCALL > + return INLINE_SYSCALL (msgctl, 3, msqid, cmd, buf); > +#else > return INLINE_SYSCALL (ipc, 5, IPCOP_msgctl, msqid, cmd, 0, buf); > +#endif > } > compat_symbol (libc, __old_msgctl, msgctl, GLIBC_2_0); There should be no need for a compat code to use a new syscall. > #endif > @@ -64,7 +68,12 @@ compat_symbol (libc, __old_msgctl, msgctl, GLIBC_2_0); > int > __new_msgctl (int msqid, int cmd, struct msqid_ds *buf) > { > +#ifdef __ASSUME_MSGCTL_SYSCALL > + return INLINE_SYSCALL (msgctl, 3, msqid, cmd | __IPC_64, buf); Why does a brand new syscall need IPC_64? Andreas.
On Thu, 3 Dec 2015, Adhemerval Zanella wrote: > My inclination would to avoid the __ASSUME_XXX macros definition and just > check if the current kernel header defines of not __NR_msgctl and use it > as the condition to enable the wire-up syscall. I'm not clear exactly what you're suggesting. If you check like that (whether the kernel headers, possibly for a newer kernel than the minimum supported by that glibc build, define a syscall number) then you need a run-time fallback if the new syscall fails with ENOSYS.
On 03-12-2015 20:22, Joseph Myers wrote: > On Thu, 3 Dec 2015, Adhemerval Zanella wrote: > >> My inclination would to avoid the __ASSUME_XXX macros definition and just >> check if the current kernel header defines of not __NR_msgctl and use it >> as the condition to enable the wire-up syscall. > > I'm not clear exactly what you're suggesting. If you check like that > (whether the kernel headers, possibly for a newer kernel than the minimum > supported by that glibc build, define a syscall number) then you need a > run-time fallback if the new syscall fails with ENOSYS. > But building with a newer kernel header will bind the libc.so to the specific version, isn't? I am assuming ENOSYS will be retuned only when the libc.so issues the syscall on an older kernel (which it should not suppose to support).
On Thu, 3 Dec 2015, Adhemerval Zanella wrote: > On 03-12-2015 20:22, Joseph Myers wrote: > > On Thu, 3 Dec 2015, Adhemerval Zanella wrote: > > > >> My inclination would to avoid the __ASSUME_XXX macros definition and just > >> check if the current kernel header defines of not __NR_msgctl and use it > >> as the condition to enable the wire-up syscall. > > > > I'm not clear exactly what you're suggesting. If you check like that > > (whether the kernel headers, possibly for a newer kernel than the minimum > > supported by that glibc build, define a syscall number) then you need a > > run-time fallback if the new syscall fails with ENOSYS. > > But building with a newer kernel header will bind the libc.so to the > specific version, isn't? I am assuming ENOSYS will be retuned only when No. You should almost always build with the newest kernel headers available so that glibc contains the support for new syscalls etc. (which may or may not work at runtime depending on the kernel headers version used then). Each of (kernel headers version, kernel version used at runtime) must be at least (minimum kernel version specified with --enable-kernel), but there is no ordering requirement between (kernel headers version) and (kernel version used at runtime). Use of new syscalls to implement functionality previously supported on older kernels needs runtime fallback unless restricted with --enable-kernel (for completely new functionality, we can judge case-by-case how useful fallbacks are).
On Thursday 03 December 2015 23:12:10 Andreas Schwab wrote: > > #endif > > @@ -64,7 +68,12 @@ compat_symbol (libc, __old_msgctl, msgctl, GLIBC_2_0); > > int > > __new_msgctl (int msqid, int cmd, struct msqid_ds *buf) > > { > > +#ifdef __ASSUME_MSGCTL_SYSCALL > > + return INLINE_SYSCALL (msgctl, 3, msqid, cmd | __IPC_64, buf); > > Why does a brand new syscall need IPC_64? This is a bug in the kernel, which we should fix there. The same problem currently exists on ARM and AVR32, which also support the old IPC API (pre-__IPC64) and are adding separate syscalls now. Arnd
On 12/03/2015 04:12 PM, Andreas Schwab wrote: > "Paul E. Murphy" <murphyp@linux.vnet.ibm.com> writes: > >> diff --git a/sysdeps/unix/sysv/linux/msgctl.c b/sysdeps/unix/sysv/linux/msgctl.c >> index a24dc38..86f34a2 100644 >> --- a/sysdeps/unix/sysv/linux/msgctl.c >> +++ b/sysdeps/unix/sysv/linux/msgctl.c >> @@ -56,7 +56,11 @@ int >> attribute_compat_text_section >> __old_msgctl (int msqid, int cmd, struct __old_msqid_ds *buf) >> { >> +#ifdef __ASSUME_MSGCTL_SYSCALL >> + return INLINE_SYSCALL (msgctl, 3, msqid, cmd, buf); >> +#else >> return INLINE_SYSCALL (ipc, 5, IPCOP_msgctl, msqid, cmd, 0, buf); >> +#endif >> } >> compat_symbol (libc, __old_msgctl, msgctl, GLIBC_2_0); > > There should be no need for a compat code to use a new syscall. Can you elaborate? I suspect I'm missing something subtle. This appears functionally identical. > >> #endif >> @@ -64,7 +68,12 @@ compat_symbol (libc, __old_msgctl, msgctl, GLIBC_2_0); >> int >> __new_msgctl (int msqid, int cmd, struct msqid_ds *buf) >> { >> +#ifdef __ASSUME_MSGCTL_SYSCALL >> + return INLINE_SYSCALL (msgctl, 3, msqid, cmd | __IPC_64, buf); > > Why does a brand new syscall need IPC_64? Looking at the kernel source, it would still appear it is required for correct behavior. The muxer just calls the new syscall, so I guess it simplifies things. > > Andreas. >
On Friday 04 December 2015 00:09:08 Arnd Bergmann wrote: > On Thursday 03 December 2015 23:12:10 Andreas Schwab wrote: > > > #endif > > > @@ -64,7 +68,12 @@ compat_symbol (libc, __old_msgctl, msgctl, GLIBC_2_0); > > > int > > > __new_msgctl (int msqid, int cmd, struct msqid_ds *buf) > > > { > > > +#ifdef __ASSUME_MSGCTL_SYSCALL > > > + return INLINE_SYSCALL (msgctl, 3, msqid, cmd | __IPC_64, buf); > > > > Why does a brand new syscall need IPC_64? > > This is a bug in the kernel, which we should fix there. The same > problem currently exists on ARM and AVR32, which also support the > old IPC API (pre-__IPC64) and are adding separate syscalls now. Correction, I looked at the wrong place: ARM and AVR32 have had this problem for a long time, so we can't fix it any more. But we should fix it for PowerPC and all other architectures that add these calls in the future. Arnd
On 03-12-2015 21:00, Joseph Myers wrote: > On Thu, 3 Dec 2015, Adhemerval Zanella wrote: > >> On 03-12-2015 20:22, Joseph Myers wrote: >>> On Thu, 3 Dec 2015, Adhemerval Zanella wrote: >>> >>>> My inclination would to avoid the __ASSUME_XXX macros definition and just >>>> check if the current kernel header defines of not __NR_msgctl and use it >>>> as the condition to enable the wire-up syscall. >>> >>> I'm not clear exactly what you're suggesting. If you check like that >>> (whether the kernel headers, possibly for a newer kernel than the minimum >>> supported by that glibc build, define a syscall number) then you need a >>> run-time fallback if the new syscall fails with ENOSYS. >> >> But building with a newer kernel header will bind the libc.so to the >> specific version, isn't? I am assuming ENOSYS will be retuned only when > > No. You should almost always build with the newest kernel headers > available so that glibc contains the support for new syscalls etc. (which > may or may not work at runtime depending on the kernel headers version > used then). Each of (kernel headers version, kernel version used at > runtime) must be at least (minimum kernel version specified with > --enable-kernel), but there is no ordering requirement between (kernel > headers version) and (kernel version used at runtime). Use of new > syscalls to implement functionality previously supported on older kernels > needs runtime fallback unless restricted with --enable-kernel (for > completely new functionality, we can judge case-by-case how useful > fallbacks are). > I just realized after sent the kernel of the case where you have newer kernels, but enable the older kernel support explicitly. I withdraw my suggestion.
On 12/03/2015 05:13 PM, Arnd Bergmann wrote: > On Friday 04 December 2015 00:09:08 Arnd Bergmann wrote: >> On Thursday 03 December 2015 23:12:10 Andreas Schwab wrote: >>>> #endif >>>> @@ -64,7 +68,12 @@ compat_symbol (libc, __old_msgctl, msgctl, GLIBC_2_0); >>>> int >>>> __new_msgctl (int msqid, int cmd, struct msqid_ds *buf) >>>> { >>>> +#ifdef __ASSUME_MSGCTL_SYSCALL >>>> + return INLINE_SYSCALL (msgctl, 3, msqid, cmd | __IPC_64, buf); >>> >>> Why does a brand new syscall need IPC_64? >> >> This is a bug in the kernel, which we should fix there. The same >> problem currently exists on ARM and AVR32, which also support the >> old IPC API (pre-__IPC64) and are adding separate syscalls now. > > Correction, I looked at the wrong place: ARM and AVR32 have had this > problem for a long time, so we can't fix it any more. But we should > fix it for PowerPC and all other architectures that add these calls > in the future. I'm not clear as to what you are suggesting for this patch. Looking at the kernel code, it does not look trivial to remove the IPC_64 bit. It seems to boil down to whether ARCH_WANT_IPC_PARSE_VERSION is configured on the kernel. Should the compat versions of these be left untouched? Or is it safe to switch them to the demuxed version? > Arnd >
diff --git a/sysdeps/unix/sysv/linux/msgctl.c b/sysdeps/unix/sysv/linux/msgctl.c index a24dc38..86f34a2 100644 --- a/sysdeps/unix/sysv/linux/msgctl.c +++ b/sysdeps/unix/sysv/linux/msgctl.c @@ -56,7 +56,11 @@ int attribute_compat_text_section __old_msgctl (int msqid, int cmd, struct __old_msqid_ds *buf) { +#ifdef __ASSUME_MSGCTL_SYSCALL + return INLINE_SYSCALL (msgctl, 3, msqid, cmd, buf); +#else return INLINE_SYSCALL (ipc, 5, IPCOP_msgctl, msqid, cmd, 0, buf); +#endif } compat_symbol (libc, __old_msgctl, msgctl, GLIBC_2_0); #endif @@ -64,7 +68,12 @@ compat_symbol (libc, __old_msgctl, msgctl, GLIBC_2_0); int __new_msgctl (int msqid, int cmd, struct msqid_ds *buf) { +#ifdef __ASSUME_MSGCTL_SYSCALL + return INLINE_SYSCALL (msgctl, 3, msqid, cmd | __IPC_64, buf); +#else return INLINE_SYSCALL (ipc, 5, IPCOP_msgctl, msqid, cmd | __IPC_64, 0, buf); +#endif + } versioned_symbol (libc, __new_msgctl, msgctl, GLIBC_2_2); diff --git a/sysdeps/unix/sysv/linux/msgget.c b/sysdeps/unix/sysv/linux/msgget.c index a206b84..b57497e 100644 --- a/sysdeps/unix/sysv/linux/msgget.c +++ b/sysdeps/unix/sysv/linux/msgget.c @@ -30,5 +30,9 @@ int msgget (key_t key, int msgflg) { +#ifdef __ASSUME_MSGGET_SYSCALL + return INLINE_SYSCALL (msgget, 2, key, msgflg); +#else return INLINE_SYSCALL (ipc, 5, IPCOP_msgget, key, msgflg, 0, NULL); +#endif } diff --git a/sysdeps/unix/sysv/linux/msgrcv.c b/sysdeps/unix/sysv/linux/msgrcv.c index 660ff29..eb36178 100644 --- a/sysdeps/unix/sysv/linux/msgrcv.c +++ b/sysdeps/unix/sysv/linux/msgrcv.c @@ -36,6 +36,9 @@ ssize_t __libc_msgrcv (int msqid, void *msgp, size_t msgsz, long int msgtyp, int msgflg) { +#ifdef __ASSUME_MSGRCV_SYSCALL + return SYSCALL_CANCEL (msgrcv, msqid, msgp, msgsz, msgtyp, msgflg); +#else /* The problem here is that Linux' calling convention only allows up to fives parameters to a system call. */ struct ipc_kludge tmp; @@ -44,5 +47,6 @@ __libc_msgrcv (int msqid, void *msgp, size_t msgsz, long int msgtyp, tmp.msgtyp = msgtyp; return SYSCALL_CANCEL (ipc, IPCOP_msgrcv, msqid, msgsz, msgflg, &tmp); +#endif } weak_alias (__libc_msgrcv, msgrcv) diff --git a/sysdeps/unix/sysv/linux/msgsnd.c b/sysdeps/unix/sysv/linux/msgsnd.c index dfed539..176027c 100644 --- a/sysdeps/unix/sysv/linux/msgsnd.c +++ b/sysdeps/unix/sysv/linux/msgsnd.c @@ -26,7 +26,11 @@ int __libc_msgsnd (int msqid, const void *msgp, size_t msgsz, int msgflg) { +#ifdef __ASSUME_MSGSND_SYSCALL + return SYSCALL_CANCEL (msgsnd, msqid, (void*) msgp, msgsz, msgflg); +#else return SYSCALL_CANCEL (ipc, IPCOP_msgsnd, msqid, msgsz, msgflg, (void *) msgp); +#endif } weak_alias (__libc_msgsnd, msgsnd) diff --git a/sysdeps/unix/sysv/linux/powerpc/kernel-features.h b/sysdeps/unix/sysv/linux/powerpc/kernel-features.h index 8a536cf..213c3a7 100644 --- a/sysdeps/unix/sysv/linux/powerpc/kernel-features.h +++ b/sysdeps/unix/sysv/linux/powerpc/kernel-features.h @@ -57,4 +57,20 @@ #endif #define __ASSUME_SENDMMSG_SYSCALL_WITH_SOCKETCALL 1 +/* Demuxed sysv IPC syscalls added in 4.4. */ +#if __LINUX_KERNEL_VERSION >= 0x040400 +# define __ASSUME_SEMOP_SYSCALL 1 +# define __ASSUME_SEMGET_SYSCALL 1 +# define __ASSUME_SEMCTL_SYSCALL 1 +# define __ASSUME_SEMTIMEDOP_SYSCALL 1 +# define __ASSUME_MSGSND_SYSCALL 1 +# define __ASSUME_MSGRCV_SYSCALL 1 +# define __ASSUME_MSGGET_SYSCALL 1 +# define __ASSUME_MSGCTL_SYSCALL 1 +# define __ASSUME_SHMAT_SYSCALL 1 +# define __ASSUME_SHMDT_SYSCALL 1 +# define __ASSUME_SHMGET_SYSCALL 1 +# define __ASSUME_SHMCTL_SYSCALL 1 +#endif + #include_next <kernel-features.h> diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c index 81a33a9..7d8533e 100644 --- a/sysdeps/unix/sysv/linux/semctl.c +++ b/sysdeps/unix/sysv/linux/semctl.c @@ -83,8 +83,12 @@ __old_semctl (int semid, int semnum, int cmd, ...) break; } +#ifdef __ASSUME_SEMCTL_SYSCALL + return INLINE_SYSCALL (semctl, 4, semid, semnum, cmd, &arg); +#else return INLINE_SYSCALL (ipc, 5, IPCOP_semctl, semid, semnum, cmd, &arg); +#endif } compat_symbol (libc, __old_semctl, semctl, GLIBC_2_0); #endif @@ -113,8 +117,12 @@ __new_semctl (int semid, int semnum, int cmd, ...) break; } +#ifdef __ASSUME_SEMCTL_SYSCALL + return INLINE_SYSCALL (semctl, 4, semid, semnum, cmd | __IPC_64, &arg); +#else return INLINE_SYSCALL (ipc, 5, IPCOP_semctl, semid, semnum, cmd | __IPC_64, &arg); +#endif } versioned_symbol (libc, __new_semctl, semctl, GLIBC_2_2); diff --git a/sysdeps/unix/sysv/linux/semget.c b/sysdeps/unix/sysv/linux/semget.c index 29ca0ae..48948ff 100644 --- a/sysdeps/unix/sysv/linux/semget.c +++ b/sysdeps/unix/sysv/linux/semget.c @@ -30,5 +30,9 @@ int semget (key_t key, int nsems, int semflg) { +#ifdef __ASSUME_SEMGET_SYSCALL + return INLINE_SYSCALL (semget, 3, key, nsems, semflg); +#else return INLINE_SYSCALL (ipc, 5, IPCOP_semget, key, nsems, semflg, NULL); +#endif } diff --git a/sysdeps/unix/sysv/linux/semop.c b/sysdeps/unix/sysv/linux/semop.c index 18a2928..165ce69 100644 --- a/sysdeps/unix/sysv/linux/semop.c +++ b/sysdeps/unix/sysv/linux/semop.c @@ -28,5 +28,9 @@ int semop (int semid, struct sembuf *sops, size_t nsops) { +#ifdef __ASSUME_SEMOP_SYSCALL + return INLINE_SYSCALL (semop, 3, semid, sops, (int) nsops); +#else return INLINE_SYSCALL (ipc, 5, IPCOP_semop, semid, (int) nsops, 0, sops); +#endif } diff --git a/sysdeps/unix/sysv/linux/semtimedop.c b/sysdeps/unix/sysv/linux/semtimedop.c index 7a80b48..13afa7d 100644 --- a/sysdeps/unix/sysv/linux/semtimedop.c +++ b/sysdeps/unix/sysv/linux/semtimedop.c @@ -29,7 +29,11 @@ int semtimedop (int semid, struct sembuf *sops, size_t nsops, const struct timespec *timeout) { +#ifdef __ASSUME_SEMTIMEDOP_SYSCALL + return INLINE_SYSCALL (semtimedop, 4, semid, sops, (int) nsops, timeout); +#else return INLINE_SYSCALL (ipc, 6, IPCOP_semtimedop, semid, (int) nsops, 0, sops, timeout); +#endif } diff --git a/sysdeps/unix/sysv/linux/shmat.c b/sysdeps/unix/sysv/linux/shmat.c index b75bb9e..f2fc5b1 100644 --- a/sysdeps/unix/sysv/linux/shmat.c +++ b/sysdeps/unix/sysv/linux/shmat.c @@ -33,12 +33,18 @@ shmat (int shmid, const void *shmaddr, int shmflg) { INTERNAL_SYSCALL_DECL(err); unsigned long resultvar; - void *raddr; + void *raddr __attribute__ ((unused)); +#ifdef __ASSUME_SHMAT_SYSCALL + resultvar = INTERNAL_SYSCALL (shmat, err, 3, shmid, (void *) shmaddr, + shmflg); + +#else resultvar = INTERNAL_SYSCALL (ipc, err, 5, IPCOP_shmat, shmid, shmflg, (long int) &raddr, (void *) shmaddr); +#endif if (INTERNAL_SYSCALL_ERROR_P (resultvar, err)) return (void *) INLINE_SYSCALL_ERROR_RETURN_VALUE (INTERNAL_SYSCALL_ERRNO (resultvar, err)); diff --git a/sysdeps/unix/sysv/linux/shmctl.c b/sysdeps/unix/sysv/linux/shmctl.c index 55cd4b3..04c19ca 100644 --- a/sysdeps/unix/sysv/linux/shmctl.c +++ b/sysdeps/unix/sysv/linux/shmctl.c @@ -63,7 +63,11 @@ int attribute_compat_text_section __old_shmctl (int shmid, int cmd, struct __old_shmid_ds *buf) { +#ifdef __ASSUME_SHMCTL_SYSCALL + return INLINE_SYSCALL (shmctl, 3, shmid, cmd, buf); +#else return INLINE_SYSCALL (ipc, 5, IPCOP_shmctl, shmid, cmd, 0, buf); +#endif } compat_symbol (libc, __old_shmctl, shmctl, GLIBC_2_0); #endif @@ -71,8 +75,12 @@ compat_symbol (libc, __old_shmctl, shmctl, GLIBC_2_0); int __new_shmctl (int shmid, int cmd, struct shmid_ds *buf) { +#ifdef __ASSUME_SHMCTL_SYSCALL + return INLINE_SYSCALL (shmctl, 3, shmid, cmd | __IPC_64, buf); +#else return INLINE_SYSCALL (ipc, 5, IPCOP_shmctl, shmid, cmd | __IPC_64, 0, buf); +#endif } versioned_symbol (libc, __new_shmctl, shmctl, GLIBC_2_2); diff --git a/sysdeps/unix/sysv/linux/shmdt.c b/sysdeps/unix/sysv/linux/shmdt.c index e7f4196..40a2ba5 100644 --- a/sysdeps/unix/sysv/linux/shmdt.c +++ b/sysdeps/unix/sysv/linux/shmdt.c @@ -29,5 +29,9 @@ int shmdt (const void *shmaddr) { +#ifdef __ASSUME_SHMDT_SYSCALL + return INLINE_SYSCALL (shmdt, 1, (void *) shmaddr); +#else return INLINE_SYSCALL (ipc, 5, IPCOP_shmdt, 0, 0, 0, (void *) shmaddr); +#endif } diff --git a/sysdeps/unix/sysv/linux/shmget.c b/sysdeps/unix/sysv/linux/shmget.c index 0cc2650..5f0a662 100644 --- a/sysdeps/unix/sysv/linux/shmget.c +++ b/sysdeps/unix/sysv/linux/shmget.c @@ -30,5 +30,9 @@ int shmget (key_t key, size_t size, int shmflg) { +#ifdef __ASSUME_SHMGET_SYSCALL + return INLINE_SYSCALL (shmget, 3, key, size, shmflg); +#else return INLINE_SYSCALL (ipc, 5, IPCOP_shmget, key, size, shmflg, NULL); +#endif }