[PATCH 15/22] arch: vdso: consolidate gettime prototypes

Arnd Bergmann posted 22 patches 1 year, 10 months ago
[PATCH 15/22] arch: vdso: consolidate gettime prototypes
Posted by Arnd Bergmann 1 year, 10 months ago
From: Arnd Bergmann <arnd@arndb.de>

The VDSO functions are defined as globals in the kernel sources but intended
to be called from userspace, so there is no need to declare them in a kernel
side header.

Without a prototype, this now causes warnings such as

arch/mips/vdso/vgettimeofday.c:14:5: error: no previous prototype for '__vdso_clock_gettime' [-Werror=missing-prototypes]
arch/mips/vdso/vgettimeofday.c:28:5: error: no previous prototype for '__vdso_gettimeofday' [-Werror=missing-prototypes]
arch/mips/vdso/vgettimeofday.c:36:5: error: no previous prototype for '__vdso_clock_getres' [-Werror=missing-prototypes]
arch/mips/vdso/vgettimeofday.c:42:5: error: no previous prototype for '__vdso_clock_gettime64' [-Werror=missing-prototypes]
arch/sparc/vdso/vclock_gettime.c:254:1: error: no previous prototype for '__vdso_clock_gettime' [-Werror=missing-prototypes]
arch/sparc/vdso/vclock_gettime.c:282:1: error: no previous prototype for '__vdso_clock_gettime_stick' [-Werror=missing-prototypes]
arch/sparc/vdso/vclock_gettime.c:307:1: error: no previous prototype for '__vdso_gettimeofday' [-Werror=missing-prototypes]
arch/sparc/vdso/vclock_gettime.c:343:1: error: no previous prototype for '__vdso_gettimeofday_stick' [-Werror=missing-prototypes]

Most architectures have already added workarounds for these by adding
declarations somewhere, but since these are all compatible, we should
really just have one copy, with an #ifdef check for the 32-bit vs
64-bit variant and use that everywhere.

Unfortunately, the sparc version is currently incompatible since
that never added support for __vdso_clock_gettime64() in 32-bit
userland. For the moment, I'm leaving this one out, as I can't
easily test it and it requires a larger rework.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/include/asm/vdso.h              |  5 -----
 arch/arm/vdso/vgettimeofday.c            |  1 +
 arch/arm64/kernel/vdso32/vgettimeofday.c |  1 +
 arch/csky/kernel/vdso/vgettimeofday.c    | 11 +----------
 arch/loongarch/vdso/vgettimeofday.c      |  7 +------
 arch/mips/vdso/vgettimeofday.c           |  1 +
 arch/riscv/kernel/vdso/vgettimeofday.c   |  7 +------
 arch/x86/entry/vdso/vclock_gettime.c     | 10 +---------
 arch/x86/include/asm/vdso/gettimeofday.h |  2 --
 arch/x86/um/vdso/um_vdso.c               |  1 +
 include/vdso/gettime.h                   | 23 +++++++++++++++++++++++
 11 files changed, 31 insertions(+), 38 deletions(-)
 create mode 100644 include/vdso/gettime.h

diff --git a/arch/arm/include/asm/vdso.h b/arch/arm/include/asm/vdso.h
index 422c3afa806a..5b85889f82ee 100644
--- a/arch/arm/include/asm/vdso.h
+++ b/arch/arm/include/asm/vdso.h
@@ -24,11 +24,6 @@ static inline void arm_install_vdso(struct mm_struct *mm, unsigned long addr)
 
 #endif /* CONFIG_VDSO */
 
-int __vdso_clock_gettime(clockid_t clock, struct old_timespec32 *ts);
-int __vdso_clock_gettime64(clockid_t clock, struct __kernel_timespec *ts);
-int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz);
-int __vdso_clock_getres(clockid_t clock_id, struct old_timespec32 *res);
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* __KERNEL__ */
diff --git a/arch/arm/vdso/vgettimeofday.c b/arch/arm/vdso/vgettimeofday.c
index a003beacac76..3554aa35f1ba 100644
--- a/arch/arm/vdso/vgettimeofday.c
+++ b/arch/arm/vdso/vgettimeofday.c
@@ -8,6 +8,7 @@
 #include <linux/types.h>
 #include <asm/vdso.h>
 #include <asm/unwind.h>
+#include <vdso/gettime.h>
 
 int __vdso_clock_gettime(clockid_t clock,
 			 struct old_timespec32 *ts)
diff --git a/arch/arm64/kernel/vdso32/vgettimeofday.c b/arch/arm64/kernel/vdso32/vgettimeofday.c
index 5acff29c5991..e23c7f4ef26b 100644
--- a/arch/arm64/kernel/vdso32/vgettimeofday.c
+++ b/arch/arm64/kernel/vdso32/vgettimeofday.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2018 ARM Limited
  *
  */
+#include <vdso/gettime.h>
 
 int __vdso_clock_gettime(clockid_t clock,
 			 struct old_timespec32 *ts)
diff --git a/arch/csky/kernel/vdso/vgettimeofday.c b/arch/csky/kernel/vdso/vgettimeofday.c
index c4831145eed5..55af30e83752 100644
--- a/arch/csky/kernel/vdso/vgettimeofday.c
+++ b/arch/csky/kernel/vdso/vgettimeofday.c
@@ -2,36 +2,27 @@
 
 #include <linux/time.h>
 #include <linux/types.h>
+#include <vdso/gettime.h>
 
 extern
-int __vdso_clock_gettime(clockid_t clock,
-			 struct old_timespec32 *ts);
 int __vdso_clock_gettime(clockid_t clock,
 			 struct old_timespec32 *ts)
 {
 	return __cvdso_clock_gettime32(clock, ts);
 }
 
-int __vdso_clock_gettime64(clockid_t clock,
-			   struct __kernel_timespec *ts);
 int __vdso_clock_gettime64(clockid_t clock,
 			   struct __kernel_timespec *ts)
 {
 	return __cvdso_clock_gettime(clock, ts);
 }
 
-extern
-int __vdso_gettimeofday(struct __kernel_old_timeval *tv,
-			struct timezone *tz);
 int __vdso_gettimeofday(struct __kernel_old_timeval *tv,
 			struct timezone *tz)
 {
 	return __cvdso_gettimeofday(tv, tz);
 }
 
-extern
-int __vdso_clock_getres(clockid_t clock_id,
-			struct old_timespec32 *res);
 int __vdso_clock_getres(clockid_t clock_id,
 			struct old_timespec32 *res)
 {
diff --git a/arch/loongarch/vdso/vgettimeofday.c b/arch/loongarch/vdso/vgettimeofday.c
index 8f22863bd7ea..0885c1f3a89d 100644
--- a/arch/loongarch/vdso/vgettimeofday.c
+++ b/arch/loongarch/vdso/vgettimeofday.c
@@ -5,23 +5,18 @@
  * Copyright (C) 2020-2022 Loongson Technology Corporation Limited
  */
 #include <linux/types.h>
+#include <vdso/gettime.h>
 
-extern
-int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
 int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts)
 {
 	return __cvdso_clock_gettime(clock, ts);
 }
 
-extern
-int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz);
 int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz)
 {
 	return __cvdso_gettimeofday(tv, tz);
 }
 
-extern
-int __vdso_clock_getres(clockid_t clock_id, struct __kernel_timespec *res);
 int __vdso_clock_getres(clockid_t clock_id, struct __kernel_timespec *res)
 {
 	return __cvdso_clock_getres(clock_id, res);
diff --git a/arch/mips/vdso/vgettimeofday.c b/arch/mips/vdso/vgettimeofday.c
index 6b83b6376a4b..604afea3f336 100644
--- a/arch/mips/vdso/vgettimeofday.c
+++ b/arch/mips/vdso/vgettimeofday.c
@@ -9,6 +9,7 @@
  */
 #include <linux/time.h>
 #include <linux/types.h>
+#include <vdso/gettime.h>
 
 #if _MIPS_SIM != _MIPS_SIM_ABI64
 int __vdso_clock_gettime(clockid_t clock,
diff --git a/arch/riscv/kernel/vdso/vgettimeofday.c b/arch/riscv/kernel/vdso/vgettimeofday.c
index cc0d80699c31..b35057802584 100644
--- a/arch/riscv/kernel/vdso/vgettimeofday.c
+++ b/arch/riscv/kernel/vdso/vgettimeofday.c
@@ -8,23 +8,18 @@
 
 #include <linux/time.h>
 #include <linux/types.h>
+#include <vdso/gettime.h>
 
-extern
-int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
 int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts)
 {
 	return __cvdso_clock_gettime(clock, ts);
 }
 
-extern
-int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz);
 int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz)
 {
 	return __cvdso_gettimeofday(tv, tz);
 }
 
-extern
-int __vdso_clock_getres(clockid_t clock_id, struct __kernel_timespec *res);
 int __vdso_clock_getres(clockid_t clock_id, struct __kernel_timespec *res)
 {
 	return __cvdso_clock_getres(clock_id, res);
diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c
index 7d70935b6758..0debc194bd78 100644
--- a/arch/x86/entry/vdso/vclock_gettime.c
+++ b/arch/x86/entry/vdso/vclock_gettime.c
@@ -11,12 +11,10 @@
 #include <linux/time.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
+#include <vdso/gettime.h>
 
 #include "../../../../lib/vdso/gettimeofday.c"
 
-extern int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz);
-extern __kernel_old_time_t __vdso_time(__kernel_old_time_t *t);
-
 int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz)
 {
 	return __cvdso_gettimeofday(tv, tz);
@@ -35,9 +33,6 @@ __kernel_old_time_t time(__kernel_old_time_t *t)	__attribute__((weak, alias("__v
 
 #if defined(CONFIG_X86_64) && !defined(BUILD_VDSO32_64)
 /* both 64-bit and x32 use these */
-extern int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
-extern int __vdso_clock_getres(clockid_t clock, struct __kernel_timespec *res);
-
 int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts)
 {
 	return __cvdso_clock_gettime(clock, ts);
@@ -56,9 +51,6 @@ int clock_getres(clockid_t, struct __kernel_timespec *)
 
 #else
 /* i386 only */
-extern int __vdso_clock_gettime(clockid_t clock, struct old_timespec32 *ts);
-extern int __vdso_clock_getres(clockid_t clock, struct old_timespec32 *res);
-
 int __vdso_clock_gettime(clockid_t clock, struct old_timespec32 *ts)
 {
 	return __cvdso_clock_gettime32(clock, ts);
diff --git a/arch/x86/include/asm/vdso/gettimeofday.h b/arch/x86/include/asm/vdso/gettimeofday.h
index c81858d903dc..a46edb0e0cf7 100644
--- a/arch/x86/include/asm/vdso/gettimeofday.h
+++ b/arch/x86/include/asm/vdso/gettimeofday.h
@@ -337,8 +337,6 @@ u64 vdso_calc_delta(u64 cycles, u64 last, u64 mask, u32 mult)
 }
 #define vdso_calc_delta vdso_calc_delta
 
-int __vdso_clock_gettime64(clockid_t clock, struct __kernel_timespec *ts);
-
 #endif /* !__ASSEMBLY__ */
 
 #endif /* __ASM_VDSO_GETTIMEOFDAY_H */
diff --git a/arch/x86/um/vdso/um_vdso.c b/arch/x86/um/vdso/um_vdso.c
index ff0f3b4b6c45..63768dd347ce 100644
--- a/arch/x86/um/vdso/um_vdso.c
+++ b/arch/x86/um/vdso/um_vdso.c
@@ -12,6 +12,7 @@
 #include <linux/time.h>
 #include <linux/getcpu.h>
 #include <asm/unistd.h>
+#include <vdso/gettime.h>
 
 int __vdso_clock_gettime(clockid_t clock, struct __kernel_old_timespec *ts)
 {
diff --git a/include/vdso/gettime.h b/include/vdso/gettime.h
new file mode 100644
index 000000000000..c50d152e7b3e
--- /dev/null
+++ b/include/vdso/gettime.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _VDSO_GETTIME_H
+#define _VDSO_GETTIME_H
+
+#include <linux/types.h>
+
+struct __kernel_timespec;
+struct timezone;
+
+#if !defined(CONFIG_64BIT) || defined(BUILD_VDSO32_64)
+struct old_timespec32;
+int __vdso_clock_getres(clockid_t clock, struct old_timespec32 *res);
+int __vdso_clock_gettime(clockid_t clock, struct old_timespec32 *ts);
+#else
+int __vdso_clock_getres(clockid_t clock, struct __kernel_timespec *res);
+int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
+#endif
+
+__kernel_old_time_t __vdso_time(__kernel_old_time_t *t);
+int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz);
+int __vdso_clock_gettime64(clockid_t clock, struct __kernel_timespec *ts);
+
+#endif
-- 
2.39.2
Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
Posted by Mark Brown 1 year, 9 months ago
On Wed, Nov 08, 2023 at 01:58:36PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The VDSO functions are defined as globals in the kernel sources but intended
> to be called from userspace, so there is no need to declare them in a kernel
> side header.

This is in -next as commit 42874e4eb35bdfc54f8514685e50434098ba4f6c and
breaks an arm64 defconfig build, the 32 bit vDSO build is broken:

/build/stage/linux/arch/arm64/kernel/vdso32/vgettimeofday.c:10:5: error: conflic
ting types for ‘__vdso_clock_gettime’; have ‘int(clockid_t,  struct old_timespec
32 *)’ {aka ‘int(int,  struct old_timespec32 *)’}
   10 | int __vdso_clock_gettime(clockid_t clock,
      |     ^~~~~~~~~~~~~~~~~~~~
In file included from /build/stage/linux/arch/arm64/kernel/vdso32/vgettimeofday.
c:8:
/build/stage/linux/include/vdso/gettime.h:16:5: note: previous declaration of ‘__vdso_clock_gettime’ with type ‘int(clockid_t,  struct __kernel_timespec *)’ {aka ‘int(int,  struct __kernel_timespec *)’}
   16 | int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
      |     ^~~~~~~~~~~~~~~~~~~~
/build/stage/linux/arch/arm64/kernel/vdso32/vgettimeofday.c:28:5: error: conflicting types for ‘__vdso_clock_getres’; have ‘int(clockid_t,  struct old_timespec32 *)’ {aka ‘int(int,  struct old_timespec32 *)’}
   28 | int __vdso_clock_getres(clockid_t clock_id,
      |     ^~~~~~~~~~~~~~~~~~~
/build/stage/linux/include/vdso/gettime.h:15:5: note: previous declaration of ‘__vdso_clock_getres’ with type ‘int(clockid_t,  struct __kernel_timespec *)’ {aka ‘int(int,  struct __kernel_timespec *)’}
   15 | int __vdso_clock_getres(clockid_t clock, struct __kernel_timespec *res);
      |     ^~~~~~~~~~~~~~~~~~~
Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
Posted by Vincenzo Frascino 1 year, 10 months ago
Hi Arnd,

On 11/8/23 12:58, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The VDSO functions are defined as globals in the kernel sources but intended
> to be called from userspace, so there is no need to declare them in a kernel
> side header.
> 
> Without a prototype, this now causes warnings such as
> 
> arch/mips/vdso/vgettimeofday.c:14:5: error: no previous prototype for '__vdso_clock_gettime' [-Werror=missing-prototypes]
> arch/mips/vdso/vgettimeofday.c:28:5: error: no previous prototype for '__vdso_gettimeofday' [-Werror=missing-prototypes]
> arch/mips/vdso/vgettimeofday.c:36:5: error: no previous prototype for '__vdso_clock_getres' [-Werror=missing-prototypes]
> arch/mips/vdso/vgettimeofday.c:42:5: error: no previous prototype for '__vdso_clock_gettime64' [-Werror=missing-prototypes]
> arch/sparc/vdso/vclock_gettime.c:254:1: error: no previous prototype for '__vdso_clock_gettime' [-Werror=missing-prototypes]
> arch/sparc/vdso/vclock_gettime.c:282:1: error: no previous prototype for '__vdso_clock_gettime_stick' [-Werror=missing-prototypes]
> arch/sparc/vdso/vclock_gettime.c:307:1: error: no previous prototype for '__vdso_gettimeofday' [-Werror=missing-prototypes]
> arch/sparc/vdso/vclock_gettime.c:343:1: error: no previous prototype for '__vdso_gettimeofday_stick' [-Werror=missing-prototypes]
> 
> Most architectures have already added workarounds for these by adding
> declarations somewhere, but since these are all compatible, we should
> really just have one copy, with an #ifdef check for the 32-bit vs
> 64-bit variant and use that everywhere.
> 

I agree, it is a good idea to have a single header for this purpose.

> Unfortunately, the sparc version is currently incompatible since
> that never added support for __vdso_clock_gettime64() in 32-bit
> userland. For the moment, I'm leaving this one out, as I can't
> easily test it and it requires a larger rework.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> ---
>  arch/arm/include/asm/vdso.h              |  5 -----
>  arch/arm/vdso/vgettimeofday.c            |  1 +
>  arch/arm64/kernel/vdso32/vgettimeofday.c |  1 +
>  arch/csky/kernel/vdso/vgettimeofday.c    | 11 +----------
>  arch/loongarch/vdso/vgettimeofday.c      |  7 +------
>  arch/mips/vdso/vgettimeofday.c           |  1 +
>  arch/riscv/kernel/vdso/vgettimeofday.c   |  7 +------
>  arch/x86/entry/vdso/vclock_gettime.c     | 10 +---------
>  arch/x86/include/asm/vdso/gettimeofday.h |  2 --
>  arch/x86/um/vdso/um_vdso.c               |  1 +
>  include/vdso/gettime.h                   | 23 +++++++++++++++++++++++
>  11 files changed, 31 insertions(+), 38 deletions(-)
>  create mode 100644 include/vdso/gettime.h
> 
> diff --git a/arch/arm/include/asm/vdso.h b/arch/arm/include/asm/vdso.h
> index 422c3afa806a..5b85889f82ee 100644
> --- a/arch/arm/include/asm/vdso.h
> +++ b/arch/arm/include/asm/vdso.h
> @@ -24,11 +24,6 @@ static inline void arm_install_vdso(struct mm_struct *mm, unsigned long addr)
>  
>  #endif /* CONFIG_VDSO */
>  
> -int __vdso_clock_gettime(clockid_t clock, struct old_timespec32 *ts);
> -int __vdso_clock_gettime64(clockid_t clock, struct __kernel_timespec *ts);
> -int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz);
> -int __vdso_clock_getres(clockid_t clock_id, struct old_timespec32 *res);
> -
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* __KERNEL__ */
> diff --git a/arch/arm/vdso/vgettimeofday.c b/arch/arm/vdso/vgettimeofday.c
> index a003beacac76..3554aa35f1ba 100644
> --- a/arch/arm/vdso/vgettimeofday.c
> +++ b/arch/arm/vdso/vgettimeofday.c
> @@ -8,6 +8,7 @@
>  #include <linux/types.h>
>  #include <asm/vdso.h>
>  #include <asm/unwind.h>
> +#include <vdso/gettime.h>
>  
>  int __vdso_clock_gettime(clockid_t clock,
>  			 struct old_timespec32 *ts)
> diff --git a/arch/arm64/kernel/vdso32/vgettimeofday.c b/arch/arm64/kernel/vdso32/vgettimeofday.c
> index 5acff29c5991..e23c7f4ef26b 100644
> --- a/arch/arm64/kernel/vdso32/vgettimeofday.c
> +++ b/arch/arm64/kernel/vdso32/vgettimeofday.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2018 ARM Limited
>   *
>   */
> +#include <vdso/gettime.h>
>  
>  int __vdso_clock_gettime(clockid_t clock,
>  			 struct old_timespec32 *ts)
> diff --git a/arch/csky/kernel/vdso/vgettimeofday.c b/arch/csky/kernel/vdso/vgettimeofday.c
> index c4831145eed5..55af30e83752 100644
> --- a/arch/csky/kernel/vdso/vgettimeofday.c
> +++ b/arch/csky/kernel/vdso/vgettimeofday.c
> @@ -2,36 +2,27 @@
>  
>  #include <linux/time.h>
>  #include <linux/types.h>
> +#include <vdso/gettime.h>
>  
>  extern
> -int __vdso_clock_gettime(clockid_t clock,
> -			 struct old_timespec32 *ts);
>  int __vdso_clock_gettime(clockid_t clock,
>  			 struct old_timespec32 *ts)
>  {
>  	return __cvdso_clock_gettime32(clock, ts);
>  }
>  
> -int __vdso_clock_gettime64(clockid_t clock,
> -			   struct __kernel_timespec *ts);
>  int __vdso_clock_gettime64(clockid_t clock,
>  			   struct __kernel_timespec *ts)
>  {
>  	return __cvdso_clock_gettime(clock, ts);
>  }
>  
> -extern
> -int __vdso_gettimeofday(struct __kernel_old_timeval *tv,
> -			struct timezone *tz);
>  int __vdso_gettimeofday(struct __kernel_old_timeval *tv,
>  			struct timezone *tz)
>  {
>  	return __cvdso_gettimeofday(tv, tz);
>  }
>  
> -extern
> -int __vdso_clock_getres(clockid_t clock_id,
> -			struct old_timespec32 *res);
>  int __vdso_clock_getres(clockid_t clock_id,
>  			struct old_timespec32 *res)
>  {
> diff --git a/arch/loongarch/vdso/vgettimeofday.c b/arch/loongarch/vdso/vgettimeofday.c
> index 8f22863bd7ea..0885c1f3a89d 100644
> --- a/arch/loongarch/vdso/vgettimeofday.c
> +++ b/arch/loongarch/vdso/vgettimeofday.c
> @@ -5,23 +5,18 @@
>   * Copyright (C) 2020-2022 Loongson Technology Corporation Limited
>   */
>  #include <linux/types.h>
> +#include <vdso/gettime.h>
>  
> -extern
> -int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
>  int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts)
>  {
>  	return __cvdso_clock_gettime(clock, ts);
>  }
>  
> -extern
> -int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz);
>  int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz)
>  {
>  	return __cvdso_gettimeofday(tv, tz);
>  }
>  
> -extern
> -int __vdso_clock_getres(clockid_t clock_id, struct __kernel_timespec *res);
>  int __vdso_clock_getres(clockid_t clock_id, struct __kernel_timespec *res)
>  {
>  	return __cvdso_clock_getres(clock_id, res);
> diff --git a/arch/mips/vdso/vgettimeofday.c b/arch/mips/vdso/vgettimeofday.c
> index 6b83b6376a4b..604afea3f336 100644
> --- a/arch/mips/vdso/vgettimeofday.c
> +++ b/arch/mips/vdso/vgettimeofday.c
> @@ -9,6 +9,7 @@
>   */
>  #include <linux/time.h>
>  #include <linux/types.h>
> +#include <vdso/gettime.h>
>  
>  #if _MIPS_SIM != _MIPS_SIM_ABI64
>  int __vdso_clock_gettime(clockid_t clock,
> diff --git a/arch/riscv/kernel/vdso/vgettimeofday.c b/arch/riscv/kernel/vdso/vgettimeofday.c
> index cc0d80699c31..b35057802584 100644
> --- a/arch/riscv/kernel/vdso/vgettimeofday.c
> +++ b/arch/riscv/kernel/vdso/vgettimeofday.c
> @@ -8,23 +8,18 @@
>  
>  #include <linux/time.h>
>  #include <linux/types.h>
> +#include <vdso/gettime.h>
>  
> -extern
> -int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
>  int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts)
>  {
>  	return __cvdso_clock_gettime(clock, ts);
>  }
>  
> -extern
> -int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz);
>  int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz)
>  {
>  	return __cvdso_gettimeofday(tv, tz);
>  }
>  
> -extern
> -int __vdso_clock_getres(clockid_t clock_id, struct __kernel_timespec *res);
>  int __vdso_clock_getres(clockid_t clock_id, struct __kernel_timespec *res)
>  {
>  	return __cvdso_clock_getres(clock_id, res);
> diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c
> index 7d70935b6758..0debc194bd78 100644
> --- a/arch/x86/entry/vdso/vclock_gettime.c
> +++ b/arch/x86/entry/vdso/vclock_gettime.c
> @@ -11,12 +11,10 @@
>  #include <linux/time.h>
>  #include <linux/kernel.h>
>  #include <linux/types.h>
> +#include <vdso/gettime.h>
>  
>  #include "../../../../lib/vdso/gettimeofday.c"
>  
> -extern int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz);
> -extern __kernel_old_time_t __vdso_time(__kernel_old_time_t *t);
> -
>  int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz)
>  {
>  	return __cvdso_gettimeofday(tv, tz);
> @@ -35,9 +33,6 @@ __kernel_old_time_t time(__kernel_old_time_t *t)	__attribute__((weak, alias("__v
>  
>  #if defined(CONFIG_X86_64) && !defined(BUILD_VDSO32_64)
>  /* both 64-bit and x32 use these */
> -extern int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
> -extern int __vdso_clock_getres(clockid_t clock, struct __kernel_timespec *res);
> -
>  int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts)
>  {
>  	return __cvdso_clock_gettime(clock, ts);
> @@ -56,9 +51,6 @@ int clock_getres(clockid_t, struct __kernel_timespec *)
>  
>  #else
>  /* i386 only */
> -extern int __vdso_clock_gettime(clockid_t clock, struct old_timespec32 *ts);
> -extern int __vdso_clock_getres(clockid_t clock, struct old_timespec32 *res);
> -
>  int __vdso_clock_gettime(clockid_t clock, struct old_timespec32 *ts)
>  {
>  	return __cvdso_clock_gettime32(clock, ts);
> diff --git a/arch/x86/include/asm/vdso/gettimeofday.h b/arch/x86/include/asm/vdso/gettimeofday.h
> index c81858d903dc..a46edb0e0cf7 100644
> --- a/arch/x86/include/asm/vdso/gettimeofday.h
> +++ b/arch/x86/include/asm/vdso/gettimeofday.h
> @@ -337,8 +337,6 @@ u64 vdso_calc_delta(u64 cycles, u64 last, u64 mask, u32 mult)
>  }
>  #define vdso_calc_delta vdso_calc_delta
>  
> -int __vdso_clock_gettime64(clockid_t clock, struct __kernel_timespec *ts);
> -
>  #endif /* !__ASSEMBLY__ */
>  
>  #endif /* __ASM_VDSO_GETTIMEOFDAY_H */
> diff --git a/arch/x86/um/vdso/um_vdso.c b/arch/x86/um/vdso/um_vdso.c
> index ff0f3b4b6c45..63768dd347ce 100644
> --- a/arch/x86/um/vdso/um_vdso.c
> +++ b/arch/x86/um/vdso/um_vdso.c
> @@ -12,6 +12,7 @@
>  #include <linux/time.h>
>  #include <linux/getcpu.h>
>  #include <asm/unistd.h>
> +#include <vdso/gettime.h>
>  
>  int __vdso_clock_gettime(clockid_t clock, struct __kernel_old_timespec *ts)
>  {
> diff --git a/include/vdso/gettime.h b/include/vdso/gettime.h
> new file mode 100644
> index 000000000000..c50d152e7b3e
> --- /dev/null
> +++ b/include/vdso/gettime.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _VDSO_GETTIME_H
> +#define _VDSO_GETTIME_H
> +
> +#include <linux/types.h>
> +
> +struct __kernel_timespec;
> +struct timezone;
> +
> +#if !defined(CONFIG_64BIT) || defined(BUILD_VDSO32_64)
> +struct old_timespec32;
> +int __vdso_clock_getres(clockid_t clock, struct old_timespec32 *res);
> +int __vdso_clock_gettime(clockid_t clock, struct old_timespec32 *ts);
> +#else
> +int __vdso_clock_getres(clockid_t clock, struct __kernel_timespec *res);
> +int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
> +#endif
> +
> +__kernel_old_time_t __vdso_time(__kernel_old_time_t *t);
> +int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz);
> +int __vdso_clock_gettime64(clockid_t clock, struct __kernel_timespec *ts);
> +
> +#endif

-- 
Regards,
Vincenzo
Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
Posted by kernel test robot 1 year, 10 months ago
Hi Arnd,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[cannot apply to v6.6 next-20231109]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Arnd-Bergmann/ida-make-ida_dump-static/20231109-005742
base:   linus/master
patch link:    https://lore.kernel.org/r/20231108125843.3806765-16-arnd%40kernel.org
patch subject: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
config: um-allnoconfig (https://download.01.org/0day-ci/archive/20231109/202311091740.g315K93o-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231109/202311091740.g315K93o-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311091740.g315K93o-lkp@intel.com/

All errors (new ones prefixed by >>):

>> arch/x86/um/vdso/um_vdso.c:17:5: error: conflicting types for '__vdso_clock_gettime'
      17 | int __vdso_clock_gettime(clockid_t clock, struct __kernel_old_timespec *ts)
         |     ^
   include/vdso/gettime.h:16:5: note: previous declaration is here
      16 | int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
         |     ^
   arch/x86/um/vdso/um_vdso.c:58:1: warning: no previous prototype for function '__vdso_getcpu' [-Wmissing-prototypes]
      58 | __vdso_getcpu(unsigned *cpu, unsigned *node, struct getcpu_cache *unused)
         | ^
   arch/x86/um/vdso/um_vdso.c:57:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
      57 | long
         | ^
         | static 
   1 warning and 1 error generated.


vim +/__vdso_clock_gettime +17 arch/x86/um/vdso/um_vdso.c

f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  16  
82210fc778982d arch/x86/um/vdso/um_vdso.c        Arnd Bergmann      2019-10-27 @17  int __vdso_clock_gettime(clockid_t clock, struct __kernel_old_timespec *ts)
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  18  {
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  19  	long ret;
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  20  
5541992e512de8 arch/x86/um/vdso/um_vdso.c        Ammar Faizi        2022-12-24  21  	asm("syscall"
5541992e512de8 arch/x86/um/vdso/um_vdso.c        Ammar Faizi        2022-12-24  22  		: "=a" (ret)
5541992e512de8 arch/x86/um/vdso/um_vdso.c        Ammar Faizi        2022-12-24  23  		: "0" (__NR_clock_gettime), "D" (clock), "S" (ts)
5541992e512de8 arch/x86/um/vdso/um_vdso.c        Ammar Faizi        2022-12-24  24  		: "rcx", "r11", "memory");
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  25  
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  26  	return ret;
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  27  }
82210fc778982d arch/x86/um/vdso/um_vdso.c        Arnd Bergmann      2019-10-27  28  int clock_gettime(clockid_t, struct __kernel_old_timespec *)
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  29  	__attribute__((weak, alias("__vdso_clock_gettime")));
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  30  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
Posted by kernel test robot 1 year, 10 months ago
Hi Arnd,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[cannot apply to v6.6 next-20231109]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Arnd-Bergmann/ida-make-ida_dump-static/20231109-005742
base:   linus/master
patch link:    https://lore.kernel.org/r/20231108125843.3806765-16-arnd%40kernel.org
patch subject: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
config: um-defconfig (https://download.01.org/0day-ci/archive/20231109/202311091754.K9wZD9Nv-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231109/202311091754.K9wZD9Nv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311091754.K9wZD9Nv-lkp@intel.com/

All errors (new ones prefixed by >>):

>> arch/x86/um/vdso/um_vdso.c:17:5: error: conflicting types for '__vdso_clock_gettime'; have 'int(clockid_t,  struct __kernel_old_timespec *)' {aka 'int(int,  struct __kernel_old_timespec *)'}
      17 | int __vdso_clock_gettime(clockid_t clock, struct __kernel_old_timespec *ts)
         |     ^~~~~~~~~~~~~~~~~~~~
   In file included from arch/x86/um/vdso/um_vdso.c:15:
   include/vdso/gettime.h:16:5: note: previous declaration of '__vdso_clock_gettime' with type 'int(clockid_t,  struct __kernel_timespec *)' {aka 'int(int,  struct __kernel_timespec *)'}
      16 | int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
         |     ^~~~~~~~~~~~~~~~~~~~
   arch/x86/um/vdso/um_vdso.c:58:1: warning: no previous prototype for '__vdso_getcpu' [-Wmissing-prototypes]
      58 | __vdso_getcpu(unsigned *cpu, unsigned *node, struct getcpu_cache *unused)
         | ^~~~~~~~~~~~~


vim +17 arch/x86/um/vdso/um_vdso.c

f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  16  
82210fc778982d arch/x86/um/vdso/um_vdso.c        Arnd Bergmann      2019-10-27 @17  int __vdso_clock_gettime(clockid_t clock, struct __kernel_old_timespec *ts)
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  18  {
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  19  	long ret;
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  20  
5541992e512de8 arch/x86/um/vdso/um_vdso.c        Ammar Faizi        2022-12-24  21  	asm("syscall"
5541992e512de8 arch/x86/um/vdso/um_vdso.c        Ammar Faizi        2022-12-24  22  		: "=a" (ret)
5541992e512de8 arch/x86/um/vdso/um_vdso.c        Ammar Faizi        2022-12-24  23  		: "0" (__NR_clock_gettime), "D" (clock), "S" (ts)
5541992e512de8 arch/x86/um/vdso/um_vdso.c        Ammar Faizi        2022-12-24  24  		: "rcx", "r11", "memory");
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  25  
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  26  	return ret;
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  27  }
82210fc778982d arch/x86/um/vdso/um_vdso.c        Arnd Bergmann      2019-10-27  28  int clock_gettime(clockid_t, struct __kernel_old_timespec *)
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  29  	__attribute__((weak, alias("__vdso_clock_gettime")));
f1c2bb8b9964ed arch/um/sys-x86_64/vdso/um_vdso.c Richard Weinberger 2011-07-25  30  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
Posted by Christophe Leroy 1 year, 10 months ago
Hi Arnd,

Le 08/11/2023 à 13:58, Arnd Bergmann a écrit :
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The VDSO functions are defined as globals in the kernel sources but intended
> to be called from userspace, so there is no need to declare them in a kernel
> side header.
> 
> Without a prototype, this now causes warnings such as
> 
> arch/mips/vdso/vgettimeofday.c:14:5: error: no previous prototype for '__vdso_clock_gettime' [-Werror=missing-prototypes]
> arch/mips/vdso/vgettimeofday.c:28:5: error: no previous prototype for '__vdso_gettimeofday' [-Werror=missing-prototypes]
> arch/mips/vdso/vgettimeofday.c:36:5: error: no previous prototype for '__vdso_clock_getres' [-Werror=missing-prototypes]
> arch/mips/vdso/vgettimeofday.c:42:5: error: no previous prototype for '__vdso_clock_gettime64' [-Werror=missing-prototypes]
> arch/sparc/vdso/vclock_gettime.c:254:1: error: no previous prototype for '__vdso_clock_gettime' [-Werror=missing-prototypes]
> arch/sparc/vdso/vclock_gettime.c:282:1: error: no previous prototype for '__vdso_clock_gettime_stick' [-Werror=missing-prototypes]
> arch/sparc/vdso/vclock_gettime.c:307:1: error: no previous prototype for '__vdso_gettimeofday' [-Werror=missing-prototypes]
> arch/sparc/vdso/vclock_gettime.c:343:1: error: no previous prototype for '__vdso_gettimeofday_stick' [-Werror=missing-prototypes]
> 
> Most architectures have already added workarounds for these by adding
> declarations somewhere, but since these are all compatible, we should
> really just have one copy, with an #ifdef check for the 32-bit vs
> 64-bit variant and use that everywhere.
> 
> Unfortunately, the sparc version is currently incompatible since
> that never added support for __vdso_clock_gettime64() in 32-bit
> userland. For the moment, I'm leaving this one out, as I can't
> easily test it and it requires a larger rework.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   arch/arm/include/asm/vdso.h              |  5 -----
>   arch/arm/vdso/vgettimeofday.c            |  1 +
>   arch/arm64/kernel/vdso32/vgettimeofday.c |  1 +
>   arch/csky/kernel/vdso/vgettimeofday.c    | 11 +----------
>   arch/loongarch/vdso/vgettimeofday.c      |  7 +------
>   arch/mips/vdso/vgettimeofday.c           |  1 +
>   arch/riscv/kernel/vdso/vgettimeofday.c   |  7 +------
>   arch/x86/entry/vdso/vclock_gettime.c     | 10 +---------
>   arch/x86/include/asm/vdso/gettimeofday.h |  2 --
>   arch/x86/um/vdso/um_vdso.c               |  1 +
>   include/vdso/gettime.h                   | 23 +++++++++++++++++++++++
>   11 files changed, 31 insertions(+), 38 deletions(-)
>   create mode 100644 include/vdso/gettime.h

powerpc has functions doing more or less the same, they are called 
__c_kernel_clock_gettime() and alike with their prototypes siting in 
arch/powerpc/include/asm/vdso/gettimeofday.h

Should those prototypes be moved to include/vdso/gettime.h too and 
eventually renamed, or are they considered too powerpc specific ?

Christophe
Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
Posted by Arnd Bergmann 1 year, 10 months ago
On Wed, Nov 8, 2023, at 19:31, Christophe Leroy wrote:
> Le 08/11/2023 à 13:58, Arnd Bergmann a écrit :

> powerpc has functions doing more or less the same, they are called 
> __c_kernel_clock_gettime() and alike with their prototypes siting in 
> arch/powerpc/include/asm/vdso/gettimeofday.h
>
> Should those prototypes be moved to include/vdso/gettime.h too and 
> eventually renamed, or are they considered too powerpc specific ?

I don't actually know, my initial interpretation was that
these function names are part of the user ABI for the vdso,
but I never looked closely enough at how vdso works to
be sure what the actual ABI is.

If __c_kernel_clock_gettime() etc are not part of the user-facing
ABI, I think renaming them for consistency with the other
architectures would be best.

     Arnd
Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
Posted by Michael Ellerman 1 year, 10 months ago
"Arnd Bergmann" <arnd@arndb.de> writes:
> On Wed, Nov 8, 2023, at 19:31, Christophe Leroy wrote:
>> Le 08/11/2023 à 13:58, Arnd Bergmann a écrit :
>
>> powerpc has functions doing more or less the same, they are called 
>> __c_kernel_clock_gettime() and alike with their prototypes siting in 
>> arch/powerpc/include/asm/vdso/gettimeofday.h
>>
>> Should those prototypes be moved to include/vdso/gettime.h too and 
>> eventually renamed, or are they considered too powerpc specific ?
>
> I don't actually know, my initial interpretation was that
> these function names are part of the user ABI for the vdso,
> but I never looked closely enough at how vdso works to
> be sure what the actual ABI is.

AFAIK the ABI is just the symbols we export, as defined in the linker
script:

/*
 * This controls what symbols we export from the DSO.
 */
VERSION
{
	VDSO_VERSION_STRING {
	global:
		__kernel_get_syscall_map;
		__kernel_gettimeofday;
		__kernel_clock_gettime;
		__kernel_clock_getres;
		__kernel_get_tbfreq;
		__kernel_sync_dicache;
		__kernel_sigtramp_rt64;
		__kernel_getcpu;
		__kernel_time;

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/kernel/vdso/vdso64.lds.S?h=v6.6&#n117

> If __c_kernel_clock_gettime() etc are not part of the user-facing
> ABI, I think renaming them for consistency with the other
> architectures would be best.

The __c symbols are not part of the ABI, so we could rename them.

At the moment though they don't have the same prototype as the generic
versions, because we find the VDSO data in asm and pass it to the C
functions, eg:

int __c_kernel_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz,
			    const struct vdso_data *vd);

I think we can rework that though, by implementing
__arch_get_vdso_data() and getting the vdso_data in C. Then we'd be able
to share the prototypes.

cheers
Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
Posted by Christophe Leroy 1 year, 10 months ago

Le 09/11/2023 à 11:18, Michael Ellerman a écrit :
> "Arnd Bergmann" <arnd@arndb.de> writes:
>> On Wed, Nov 8, 2023, at 19:31, Christophe Leroy wrote:
>>> Le 08/11/2023 à 13:58, Arnd Bergmann a écrit :
>>
>>> powerpc has functions doing more or less the same, they are called
>>> __c_kernel_clock_gettime() and alike with their prototypes siting in
>>> arch/powerpc/include/asm/vdso/gettimeofday.h
>>>
>>> Should those prototypes be moved to include/vdso/gettime.h too and
>>> eventually renamed, or are they considered too powerpc specific ?
>>
>> I don't actually know, my initial interpretation was that
>> these function names are part of the user ABI for the vdso,
>> but I never looked closely enough at how vdso works to
>> be sure what the actual ABI is.
> 
> AFAIK the ABI is just the symbols we export, as defined in the linker
> script:
> 
> /*
>   * This controls what symbols we export from the DSO.
>   */
> VERSION
> {
> 	VDSO_VERSION_STRING {
> 	global:
> 		__kernel_get_syscall_map;
> 		__kernel_gettimeofday;
> 		__kernel_clock_gettime;
> 		__kernel_clock_getres;
> 		__kernel_get_tbfreq;
> 		__kernel_sync_dicache;
> 		__kernel_sigtramp_rt64;
> 		__kernel_getcpu;
> 		__kernel_time;
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/kernel/vdso/vdso64.lds.S?h=v6.6&#n117
> 
>> If __c_kernel_clock_gettime() etc are not part of the user-facing
>> ABI, I think renaming them for consistency with the other
>> architectures would be best.
> 
> The __c symbols are not part of the ABI, so we could rename them.
> 
> At the moment though they don't have the same prototype as the generic
> versions, because we find the VDSO data in asm and pass it to the C
> functions, eg:
> 
> int __c_kernel_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz,
> 			    const struct vdso_data *vd);
> 
> I think we can rework that though, by implementing
> __arch_get_vdso_data() and getting the vdso_data in C. Then we'd be able
> to share the prototypes.

I think it would not a been good idea, it would be less performant, for 
explanation see commit 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e876f0b69dc993e86ca7795e63e98385aa9a7ef3

Christophe
Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
Posted by Michael Ellerman 1 year, 10 months ago
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 09/11/2023 à 11:18, Michael Ellerman a écrit :
>> "Arnd Bergmann" <arnd@arndb.de> writes:
>>> On Wed, Nov 8, 2023, at 19:31, Christophe Leroy wrote:
>>>> Le 08/11/2023 à 13:58, Arnd Bergmann a écrit :
>>>
>>>> powerpc has functions doing more or less the same, they are called
>>>> __c_kernel_clock_gettime() and alike with their prototypes siting in
>>>> arch/powerpc/include/asm/vdso/gettimeofday.h
>>>>
>>>> Should those prototypes be moved to include/vdso/gettime.h too and
>>>> eventually renamed, or are they considered too powerpc specific ?
>>>
>>> I don't actually know, my initial interpretation was that
>>> these function names are part of the user ABI for the vdso,
>>> but I never looked closely enough at how vdso works to
>>> be sure what the actual ABI is.
>> 
>> AFAIK the ABI is just the symbols we export, as defined in the linker
>> script:
>> 
>> /*
>>   * This controls what symbols we export from the DSO.
>>   */
>> VERSION
>> {
>> 	VDSO_VERSION_STRING {
>> 	global:
>> 		__kernel_get_syscall_map;
>> 		__kernel_gettimeofday;
>> 		__kernel_clock_gettime;
>> 		__kernel_clock_getres;
>> 		__kernel_get_tbfreq;
>> 		__kernel_sync_dicache;
>> 		__kernel_sigtramp_rt64;
>> 		__kernel_getcpu;
>> 		__kernel_time;
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/kernel/vdso/vdso64.lds.S?h=v6.6&#n117
>> 
>>> If __c_kernel_clock_gettime() etc are not part of the user-facing
>>> ABI, I think renaming them for consistency with the other
>>> architectures would be best.
>> 
>> The __c symbols are not part of the ABI, so we could rename them.
>> 
>> At the moment though they don't have the same prototype as the generic
>> versions, because we find the VDSO data in asm and pass it to the C
>> functions, eg:
>> 
>> int __c_kernel_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz,
>> 			    const struct vdso_data *vd);
>> 
>> I think we can rework that though, by implementing
>> __arch_get_vdso_data() and getting the vdso_data in C. Then we'd be able
>> to share the prototypes.
>
> I think it would not a been good idea, it would be less performant, for 
> explanation see commit 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e876f0b69dc993e86ca7795e63e98385aa9a7ef3

Ah thanks. I was wondering why you had done it in asm.

It's a pity but you're right that's probably a measurable performance
hit for some of those calls.

cheers
Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
Posted by Christophe Leroy 1 year, 10 months ago

Le 08/11/2023 à 20:37, Arnd Bergmann a écrit :
> On Wed, Nov 8, 2023, at 19:31, Christophe Leroy wrote:
>> Le 08/11/2023 à 13:58, Arnd Bergmann a écrit :
> 
>> powerpc has functions doing more or less the same, they are called
>> __c_kernel_clock_gettime() and alike with their prototypes siting in
>> arch/powerpc/include/asm/vdso/gettimeofday.h
>>
>> Should those prototypes be moved to include/vdso/gettime.h too and
>> eventually renamed, or are they considered too powerpc specific ?
> 
> I don't actually know, my initial interpretation was that
> these function names are part of the user ABI for the vdso,
> but I never looked closely enough at how vdso works to
> be sure what the actual ABI is.
> 
> If __c_kernel_clock_gettime() etc are not part of the user-facing
> ABI, I think renaming them for consistency with the other
> architectures would be best.
> 

User-facing ABI function is __kernel_clock_gettime(), defined in 
arch/powerpc/kernel/vdso/gettimeofday.S , see man vdso.
There is no prototype defined for it anywhere, obviously that's 
undetected because it is assembly. Should a prototype be added somewhere 
anyway ?

__kernel_clock_gettime() sets up a stack frame, retrieves the address of 
the datapage then calls __c_kernel_clock_gettime() which then calls 
__cvdso_clock_gettime_data() which is part of the generic CVDSO.

Maybe that's too different from what other architectures do ?

Christophe