diff mbox

S390: Add support for vdso getcpu symbol.

Message ID nfv38m$q7v$1@ger.gmane.org
State Committed
Headers

Commit Message

Stefan Liebler April 29, 2016, 7:42 a.m. UTC 
  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.

Comments

Stefan Liebler May 4, 2016, 1:15 p.m. UTC | #1 
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.
Adhemerval Zanella May 4, 2016, 3:18 p.m. UTC | #2 
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.
>
Carlos O'Donell May 4, 2016, 4:33 p.m. UTC | #3 
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?
Stefan Liebler May 9, 2016, 9:10 a.m. UTC | #4 
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
diff mbox

Patch

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 */