Message ID | nfv38m$q7v$1@ger.gmane.org |
---|---|
State | New |
Headers | show |
ping Any objection? Otherwise, I'll commit this patch. On 04/29/2016 09:42 AM, Stefan Liebler wrote: > Hi, > > This patch adds support for symbol __kernel_getcpu in vDSO, > which is available with kernel 4.5. > Now sched_getcpu is using this symbol if available in mapped vDSO > by defining macro HAVE_GETCPU_VSYSCALL. If not available at runtime, > the former syscall is used. > > Bye > Stefan > > ChangeLog: > > * sysdeps/unix/sysv/linux/s390/init-first.c: > Add VDSO_SYMBOL(getcpu). > (_libc_vdso_platform_setup): Initialize VDSO_SYMBOL(getcpu). > * sysdeps/unix/sysv/linux/s390/libc-vdso.h: > Add VDSO_SYMBOL(getcpu). > * sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h: > New define HAVE_GETCPU_VSYSCALL. > * sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h: Likewise.
LGTM. It could a later clean up to move s390 HAVE_<...>_VSYSCALL to a common place. On 04/05/2016 10:15, Stefan Liebler wrote: > ping > > Any objection? Otherwise, I'll commit this patch. > > On 04/29/2016 09:42 AM, Stefan Liebler wrote: >> Hi, >> >> This patch adds support for symbol __kernel_getcpu in vDSO, >> which is available with kernel 4.5. >> Now sched_getcpu is using this symbol if available in mapped vDSO >> by defining macro HAVE_GETCPU_VSYSCALL. If not available at runtime, >> the former syscall is used. >> >> Bye >> Stefan >> >> ChangeLog: >> >> * sysdeps/unix/sysv/linux/s390/init-first.c: >> Add VDSO_SYMBOL(getcpu). >> (_libc_vdso_platform_setup): Initialize VDSO_SYMBOL(getcpu). >> * sysdeps/unix/sysv/linux/s390/libc-vdso.h: >> Add VDSO_SYMBOL(getcpu). >> * sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h: >> New define HAVE_GETCPU_VSYSCALL. >> * sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h: Likewise. >
On 04/29/2016 03:42 AM, Stefan Liebler wrote: > Hi, > > This patch adds support for symbol __kernel_getcpu in vDSO, > which is available with kernel 4.5. > Now sched_getcpu is using this symbol if available in mapped vDSO > by defining macro HAVE_GETCPU_VSYSCALL. If not available at runtime, > the former syscall is used. > > Bye > Stefan > > ChangeLog: > > * sysdeps/unix/sysv/linux/s390/init-first.c: > Add VDSO_SYMBOL(getcpu). > (_libc_vdso_platform_setup): Initialize VDSO_SYMBOL(getcpu). > * sysdeps/unix/sysv/linux/s390/libc-vdso.h: > Add VDSO_SYMBOL(getcpu). > * sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h: > New define HAVE_GETCPU_VSYSCALL. > * sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h: Likewise. Stefan, Looks good to me, but I have one question. Why are the versions of these new symbols LINUX_2.6.29? If the kernel really wants to follow the userspace conventions for symbol versioning, the version of a newly added symbol is that of the Linux version that released the symbol. This allows users and developers to know exactly which upstream public kernel version exported the symbol, and talk sensible about the exported ABI/API in terms of these versions. Right now having them all be LINUX_2.6.29 is useful only for adding compat symbols at newer versions. At this point it's too late, you have a release with these symbols at these versions, and you can't change it. Martin, Did you consider using LINUX_4.5 version for these symbols?
On 05/04/2016 06:33 PM, Carlos O'Donell wrote: > Stefan, > > Looks good to me, but I have one question. > > Why are the versions of these new symbols LINUX_2.6.29? > > If the kernel really wants to follow the userspace conventions > for symbol versioning, the version of a newly added symbol is > that of the Linux version that released the symbol. > > This allows users and developers to know exactly which upstream > public kernel version exported the symbol, and talk sensible about > the exported ABI/API in terms of these versions. Right now having > them all be LINUX_2.6.29 is useful only for adding compat symbols > at newer versions. > > At this point it's too late, you have a release with these symbols > at these versions, and you can't change it. > > Martin, > > Did you consider using LINUX_4.5 version for these symbols? > I had realised this version mismatch, too. I had asked Martin about the version number before I posted the patch and his first thought was, that it reflects the version, the symbol was introduced. Then he realised, that it does not in this case. But he won't change that for getcpu. As observation: E.g. <kernel-src>/arch/powerpc/kernel/vdso64/vdso64.lds.S was extended with __kernel_getcpu, __kernel_time. E.g. <kernel-src>/arch/tile/kernel/vdso/vdso.lds.S was extended with __vdso_clock_gettime. These additions were made without a new version number. The corresponding glibc commits logically does not introduce a new linux-version to find vdso-symbol in _libc_vdso_platform_setup(), too. For documentation: Link to discussion "Should Linux VDSO be using symbol version based on the released kernel?" (https://www.sourceware.org/ml/libc-alpha/2016-05/msg00056.html) I've pushed the patch. Thanks. Bye Stefan
commit 47c8c56d61660701e18cf17932a6da1b84fcf789 Author: Stefan Liebler <stli@linux.vnet.ibm.com> Date: Thu Apr 28 17:30:34 2016 +0200 S390: Add support for vdso getcpu symbol. This patch adds support for symbol __kernel_getcpu in vDSO, which is available with kernel 4.5. Now sched_getcpu is using this symbol if available in mapped vDSO by defining macro HAVE_GETCPU_VSYSCALL. If not available at runtime, the former syscall is used. ChangeLog: * sysdeps/unix/sysv/linux/s390/init-first.c: Add VDSO_SYMBOL(getcpu). (_libc_vdso_platform_setup): Initialize VDSO_SYMBOL(getcpu). * sysdeps/unix/sysv/linux/s390/libc-vdso.h: Add VDSO_SYMBOL(getcpu). * sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h: New define HAVE_GETCPU_VSYSCALL. * sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h: Likewise. diff --git a/sysdeps/unix/sysv/linux/s390/init-first.c b/sysdeps/unix/sysv/linux/s390/init-first.c index d3a20fd..7498cbe 100644 --- a/sysdeps/unix/sysv/linux/s390/init-first.c +++ b/sysdeps/unix/sysv/linux/s390/init-first.c @@ -29,6 +29,8 @@ long int (*VDSO_SYMBOL(clock_gettime)) (clockid_t, struct timespec *) long int (*VDSO_SYMBOL(clock_getres)) (clockid_t, struct timespec *) __attribute__ ((nocommon)); +long int (*VDSO_SYMBOL(getcpu)) (unsigned *, unsigned *, void *) + attribute_hidden; static inline void _libc_vdso_platform_setup (void) @@ -46,6 +48,10 @@ _libc_vdso_platform_setup (void) p = _dl_vdso_vsym ("__kernel_clock_getres", &linux2629); PTR_MANGLE (p); VDSO_SYMBOL (clock_getres) = p; + + p = _dl_vdso_vsym ("__kernel_getcpu", &linux2629); + PTR_MANGLE (p); + VDSO_SYMBOL (getcpu) = p; } # define VDSO_SETUP _libc_vdso_platform_setup diff --git a/sysdeps/unix/sysv/linux/s390/libc-vdso.h b/sysdeps/unix/sysv/linux/s390/libc-vdso.h index d2a8316..512b3ba 100644 --- a/sysdeps/unix/sysv/linux/s390/libc-vdso.h +++ b/sysdeps/unix/sysv/linux/s390/libc-vdso.h @@ -31,6 +31,8 @@ extern long int (*VDSO_SYMBOL(clock_gettime)) (clockid_t, struct timespec *); extern long int (*VDSO_SYMBOL(clock_getres)) (clockid_t, struct timespec *); +extern long int (*VDSO_SYMBOL(getcpu)) (unsigned *, unsigned *, void *) + attribute_hidden; #endif #endif /* _LIBC_VDSO_H */ diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h b/sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h index 3540416..651e1ee 100644 --- a/sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h +++ b/sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h @@ -283,6 +283,7 @@ #define HAVE_CLOCK_GETRES_VSYSCALL 1 #define HAVE_CLOCK_GETTIME_VSYSCALL 1 #define HAVE_GETTIMEOFDAY_VSYSCALL 1 +#define HAVE_GETCPU_VSYSCALL 1 /* This version is for internal uses when there is no desire to set errno */ diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h b/sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h index 6f390ff..702b0b6 100644 --- a/sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h +++ b/sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h @@ -289,6 +289,7 @@ #define HAVE_CLOCK_GETRES_VSYSCALL 1 #define HAVE_CLOCK_GETTIME_VSYSCALL 1 #define HAVE_GETTIMEOFDAY_VSYSCALL 1 +#define HAVE_GETCPU_VSYSCALL 1 /* This version is for internal uses when there is no desire to set errno */