Re: [PATCH v2] hppa/vdso: Add wrappers for vDSO functions

[Helge Deller <deller@kernel.org>]

* Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>:
> 
> On 25/06/24 16:48, Helge Deller wrote:
> > On 6/24/24 22:31, Adhemerval Zanella Netto wrote:
> >>
> >>
> >> On 21/06/24 16:26, Helge Deller wrote:
> >>> The upcoming parisc (hppa) v6.11 Linux kernel will include vDSO
> >>> support for gettimeofday(), clock_gettime() and clock_gettime64()
> >>> syscalls for 32- and 64-bit userspace.
> >>> The patch below adds the necessary glue code for glibc.
> >>>
> >>> Signed-off-by: Helge Deller <deller@gmx.de>
> >>>
> >>> Changes in v2:
> >>> - add vsyscalls for 64-bit too
> >>>
> >>> diff -up ./sysdeps/unix/sysv/linux/hppa/sysdep.h.org ./sysdeps/unix/sysv/linux/hppa/sysdep.h
> >>> --- ./sysdeps/unix/sysv/linux/hppa/sysdep.h.org    2024-06-15 20:20:58.992000000 +0000
> >>> +++ ./sysdeps/unix/sysv/linux/hppa/sysdep.h    2024-06-21 19:19:02.132000000 +0000
> >>> @@ -468,6 +468,18 @@ L(pre_end):                    ASM_LINE_SEP    \
> >>>   #define CLOB_ARGS_1 CLOB_ARGS_2, "%r25"
> >>>   #define CLOB_ARGS_0 CLOB_ARGS_1, "%r26"
> >>>
> >>> +#define VDSO_NAME    "LINUX_6.11"
> >>> +#define VDSO_HASH    182951793
> >>> +
> >>> +#ifdef __LP64__
> >>> +# define HAVE_CLOCK_GETTIME_VSYSCALL    "__vdso_clock_gettime"
> >>> +# define HAVE_GETTIMEOFDAY_VSYSCALL     "__vdso_gettimeofday"
> >>> +#else
> >>> +# define HAVE_CLOCK_GETTIME_VSYSCALL    "__vdso_clock_gettime"
> >>> +# define HAVE_CLOCK_GETTIME64_VSYSCALL  "__vdso_clock_gettime64"
> >>> +# define HAVE_GETTIMEOFDAY_VSYSCALL     "__vdso_gettimeofday"
> >>> +#endif /* __LP64__ */
> >>> +
> >>>   #endif    /* __ASSEMBLER__ */
> >>
> >> Not sure why you have added the gettimeofday support, currently 32 bits
> >> it is already routed to to clock_gettime (which will use
> >> __vdso_clock_gettime64 anyway).
> > 
> > Yep, you are right.
> > I actually didn't checked if it's being used but just saw it's defined
> > for other arches too, so I assumed it to be used.
> > 
> >> For hppa to actually use, it would require to add a way to call it
> >> for !USE_IFUNC_GETTIMEOFDAY gettimeofday, which I am not it really be
> >> an improvement here.
> > 
> > Yes, that doesn't make sense.
> > 
> > Looking at the code it seems riscv, sparc, arm, mips and s390 define it
> > too, without being used. Do you suggest we should remove gettimeofday
> > vsyscall from hppa again (or just keep like the others even if not being used)?
> 
> The riscv only added it for CONFIG_64BIT, while old ABIs like arm, sparc,
> mips, and s390 added before 64 bit time_t landed on 5.1.  With current 
> 32 bits ABIs moving to 64 bit time_t userland, there is no much sense in 
> providing a 32 bit gettimeofday, nor I think I adding a  gettimeoday64 would 
> make much sense (maybe on really high sensitive workloads that require 
> low-latency timestamping, but clock_gettime would work better anyway).
> 
> It is highly unlikely that the symbol will ever be used by userland,
> and I think it only make sense to provide clock_gettime64.  The glibc 
> 32 bit time_t clock_gettime routes to clock_gettime64 and it will only 
> fallback to 32 bit vDSO symbol if the 64 bit time_t one is not present.

Like patch below?
Helge


[PATCH] hppa/vdso: Provide 64-bit clock_gettime() vDSO only

Adhemerval noticed that the gettimeofday() and 32-bit clock_gettime()
vDSO calls won't be used by glibc on hppa, so there is no need to
declare them.  Both syscalls will be emulated by utilizing return values
of the 64-bit clock_gettime() vDSO instead..

Signed-off-by: Helge Deller <deller@gmx.de>
Suggested-by: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>

diff --git a/sysdeps/unix/sysv/linux/hppa/sysdep.h b/sysdeps/unix/sysv/linux/hppa/sysdep.h
index af62f7501e..e47975e5cf 100644
--- a/sysdeps/unix/sysv/linux/hppa/sysdep.h
+++ b/sysdeps/unix/sysv/linux/hppa/sysdep.h
@@ -473,11 +473,8 @@ L(pre_end):					ASM_LINE_SEP	\
 
 #ifdef __LP64__
 # define HAVE_CLOCK_GETTIME_VSYSCALL    "__vdso_clock_gettime"
-# define HAVE_GETTIMEOFDAY_VSYSCALL     "__vdso_gettimeofday"
 #else
-# define HAVE_CLOCK_GETTIME_VSYSCALL    "__vdso_clock_gettime"
 # define HAVE_CLOCK_GETTIME64_VSYSCALL  "__vdso_clock_gettime64"
-# define HAVE_GETTIMEOFDAY_VSYSCALL     "__vdso_gettimeofday"
 #endif /* __LP64__ */
 
 #endif	/* __ASSEMBLER__ */