linux-kernel - Re: [PATCH 01/19] vdso: Consolidate vdso_calc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <094a2fd9-23d9-405b-9b6f-0e2516cd5a57@csgroup.eu>
Date: Sat, 9 Mar 2024 07:48:44 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Adrian Hunter <adrian.hunter@...el.com>, Thomas Gleixner
	<tglx@...utronix.de>
CC: Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin
	<npiggin@...il.com>, Aneesh Kumar K.V <aneesh.kumar@...nel.org>, "Naveen N.
 Rao" <naveen.n.rao@...ux.ibm.com>, Heiko Carstens <hca@...ux.ibm.com>, Vasily
 Gorbik <gor@...ux.ibm.com>, Alexander Gordeev <agordeev@...ux.ibm.com>,
	Christian Borntraeger <borntraeger@...ux.ibm.com>, Sven Schnelle
	<svens@...ux.ibm.com>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov
	<bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>, "x86@...nel.org"
	<x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>, Andy Lutomirski
	<luto@...nel.org>, Vincenzo Frascino <vincenzo.frascino@....com>, John Stultz
	<jstultz@...gle.com>, Stephen Boyd <sboyd@...nel.org>, Peter Zijlstra
	<peterz@...radead.org>, Randy Dunlap <rdunlap@...radead.org>, Bjorn Helgaas
	<bhelgaas@...gle.com>, Arnd Bergmann <arnd@...db.de>, Anna-Maria Behnsen
	<anna-maria@...utronix.de>, "linuxppc-dev@...ts.ozlabs.org"
	<linuxppc-dev@...ts.ozlabs.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-s390@...r.kernel.org"
	<linux-s390@...r.kernel.org>
Subject: Re: [PATCH 01/19] vdso: Consolidate vdso_calc_delta()



Le 08/03/2024 à 14:14, Adrian Hunter a écrit :
> [Vous ne recevez pas souvent de courriers de adrian.hunter@...el.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> Consolidate vdso_calc_delta(), in preparation for further simplification.
> 
> Suggested-by: Thomas Gleixner <tglx@...utronix.de>
> Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
> ---
>   arch/powerpc/include/asm/vdso/gettimeofday.h | 17 ++---------------
>   arch/s390/include/asm/vdso/gettimeofday.h    |  7 ++-----
>   lib/vdso/gettimeofday.c                      |  4 ++++
>   3 files changed, 8 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h b/arch/powerpc/include/asm/vdso/gettimeofday.h
> index f0a4cf01e85c..f4da8e18cdf3 100644
> --- a/arch/powerpc/include/asm/vdso/gettimeofday.h
> +++ b/arch/powerpc/include/asm/vdso/gettimeofday.h
> @@ -14,6 +14,8 @@
> 
>   #define VDSO_HAS_TIME                  1
> 
> +#define VDSO_DELTA_NOMASK              1
> +
>   static __always_inline int do_syscall_2(const unsigned long _r0, const unsigned long _r3,
>                                          const unsigned long _r4)
>   {
> @@ -105,21 +107,6 @@ static inline bool vdso_clocksource_ok(const struct vdso_data *vd)
>   }
>   #define vdso_clocksource_ok vdso_clocksource_ok
> 
> -/*
> - * powerpc specific delta calculation.
> - *
> - * This variant removes the masking of the subtraction because the
> - * clocksource mask of all VDSO capable clocksources on powerpc is U64_MAX
> - * which would result in a pointless operation. The compiler cannot
> - * optimize it away as the mask comes from the vdso data and is not compile
> - * time constant.
> - */

Please keep the comment. You can move it close to VDSO_DELTA_NOMASK

> -static __always_inline u64 vdso_calc_delta(u64 cycles, u64 last, u64 mask, u32 mult)
> -{
> -       return (cycles - last) * mult;
> -}
> -#define vdso_calc_delta vdso_calc_delta
> -
>   #ifndef __powerpc64__
>   static __always_inline u64 vdso_shift_ns(u64 ns, unsigned long shift)
>   {
> diff --git a/arch/s390/include/asm/vdso/gettimeofday.h b/arch/s390/include/asm/vdso/gettimeofday.h
> index db84942eb78f..7937765ccfa5 100644
> --- a/arch/s390/include/asm/vdso/gettimeofday.h
> +++ b/arch/s390/include/asm/vdso/gettimeofday.h
> @@ -6,16 +6,13 @@
> 
>   #define VDSO_HAS_CLOCK_GETRES 1
> 
> +#define VDSO_DELTA_NOMASK 1
> +
>   #include <asm/syscall.h>
>   #include <asm/timex.h>
>   #include <asm/unistd.h>
>   #include <linux/compiler.h>
> 
> -#define vdso_calc_delta __arch_vdso_calc_delta
> -static __always_inline u64 __arch_vdso_calc_delta(u64 cycles, u64 last, u64 mask, u32 mult)
> -{
> -       return (cycles - last) * mult;
> -}
> 
>   static __always_inline const struct vdso_data *__arch_get_vdso_data(void)
>   {
> diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
> index ce2f69552003..042b95e8164d 100644
> --- a/lib/vdso/gettimeofday.c
> +++ b/lib/vdso/gettimeofday.c
> @@ -13,7 +13,11 @@
>   static __always_inline
>   u64 vdso_calc_delta(u64 cycles, u64 last, u64 mask, u32 mult)
>   {
> +#ifdef VDSO_DELTA_NOMASK
> +       return (cycles - last) * mult;
> +#else
>          return ((cycles - last) & mask) * mult;
> +#endif

See 
https://docs.kernel.org/process/coding-style.html#conditional-compilation

You don't need #ifdefs here.

One solution is to define VDSO_DELTA_NOMASK to 0 in 
include/vdso/datapage.h after including asm/vdso/gettimeofday.h :

#ifndef VDSO_DELTA_NOMASK
#define VDSO_DELTA_NOMASK 0
#endif

Then

u64 vdso_calc_delta(u64 cycles, u64 last, u64 mask, u32 mult)
{
	if (VDSO_DELTA_NOMASK)
		mask = ~0ULL;

	return ((cycles - last) & mask) * mult;
}

or

u64 vdso_calc_delta(u64 cycles, u64 last, u64 mask, u32 mult)
{
	if (VDSO_DELTA_NOMASK)
		return (cycles - last) * mult;

	return ((cycles - last) & mask) * mult;
}




>   }
>   #endif
> 
> --
> 2.34.1
>