[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f6c5ce65-be49-add3-5959-85fa9cdc75a2@c-s.fr>
Date: Wed, 15 Jan 2020 06:50:42 +0100
From: Christophe Leroy <christophe.leroy@....fr>
To: Thomas Gleixner <tglx@...utronix.de>,
LKML <linux-kernel@...r.kernel.org>
Cc: x86@...nel.org, John Stultz <john.stultz@...aro.org>,
Vincenzo Frascino <vincenzo.frascino@....com>,
Andy Lutomirski <luto@...nel.org>,
Paolo Bonzini <pbonzini@...hat.com>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
Juergen Gross <jgross@...e.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
Sasha Levin <sashal@...nel.org>,
Ralf Baechle <ralf@...ux-mips.org>,
Paul Burton <paulburton@...nel.org>,
James Hogan <jhogan@...nel.org>,
Russell King <linux@...linux.org.uk>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Marc Zyngier <maz@...nel.org>
Subject: Re: [patch 09/15] clocksource: Add common vdso clock mode storage
Le 14/01/2020 à 19:52, Thomas Gleixner a écrit :
> All architectures which use the generic VDSO code have their own storage
> for the VDSO clock mode. That's pointless and just requires duplicate code.
>
> Provide generic storage for it. The new Kconfig symbol is intermediate and
> will be removed once all architectures are converted over.
>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
> include/linux/clocksource.h | 12 +++++++++++-
> kernel/time/clocksource.c | 9 +++++++++
> kernel/time/vsyscall.c | 7 +++++++
> lib/vdso/Kconfig | 3 +++
> lib/vdso/gettimeofday.c | 13 +++++++++++--
> 5 files changed, 41 insertions(+), 3 deletions(-)
>
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -23,10 +23,19 @@
> struct clocksource;
> struct module;
>
> -#ifdef CONFIG_ARCH_CLOCKSOURCE_DATA
> +#if defined(CONFIG_ARCH_CLOCKSOURCE_DATA) || \
> + defined(CONFIG_GENERIC_VDSO_CLOCK_MODE)
> #include <asm/clocksource.h>
> #endif
>
> +enum vdso_clock_mode {
> + VDSO_CLOCKMODE_NONE,
> +#ifdef CONFIG_GENERIC_VDSO_CLOCK_MODE
> + VDSO_ARCH_CLOCKMODES,
> +#endif
> + VDSO_CLOCKMODE_MAX,
> +};
> +
> /**
> * struct clocksource - hardware abstraction for a free running counter
> * Provides mostly state-free accessors to the underlying hardware.
> @@ -97,6 +106,7 @@ struct clocksource {
> const char *name;
> struct list_head list;
> int rating;
> + enum vdso_clock_mode vdso_clock_mode;
> unsigned long flags;
>
> int (*enable)(struct clocksource *cs);
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -921,6 +921,15 @@ int __clocksource_register_scale(struct
>
> clocksource_arch_init(cs);
>
> +#ifdef CONFIG_GENERIC_VDSO_CLOCK_MODE
> + if (cs->vdso_clock_mode < 0 ||
> + cs->vdso_clock_mode >= VDSO_CLOCKMODE_MAX) {
> + pr_warn("clocksource %s registered with invalid VDSO mode %d. Disabling VDSO support.\n",
> + cs->name, cs->vdso_clock_mode);
> + cs->vdso_clock_mode = VDSO_CLOCKMODE_NONE;
> + }
> +#endif
> +
> /* Initialize mult/shift and max_idle_ns */
> __clocksource_update_freq_scale(cs, scale, freq);
>
> --- a/kernel/time/vsyscall.c
> +++ b/kernel/time/vsyscall.c
> @@ -72,12 +72,19 @@ void update_vsyscall(struct timekeeper *
> struct vdso_data *vdata = __arch_get_k_vdso_data();
> struct vdso_timestamp *vdso_ts;
> u64 nsec;
> + s32 mode;
gcc will claim 'mode' is unused when CONFIG_GENERIC_VDSO_CLOCK_MODE is
not defined.
>
> /* copy vsyscall data */
> vdso_write_begin(vdata);
>
> +#ifdef CONFIG_GENERIC_VDSO_CLOCK_MODE
> + mode = tk->tkr_mono.clock->vdso_clock_mode;
> + vdata[CS_HRES_COARSE].clock_mode = mode;
> + vdata[CS_RAW].clock_mode = mode;
> +#else
> vdata[CS_HRES_COARSE].clock_mode = __arch_get_clock_mode(tk);
> vdata[CS_RAW].clock_mode = __arch_get_clock_mode(tk);
> +#endif
Can we do :
#ifdef CONFIG_GENERIC_VDSO_CLOCK_MODE
mode = tk->tkr_mono.clock->vdso_clock_mode;
#else
mode = __arch_get_clock_mode(tk);
#endif
vdata[CS_HRES_COARSE].clock_mode = mode;
vdata[CS_RAW].clock_mode = mode;
Christophe
>
> /* CLOCK_REALTIME also required for time() */
> vdso_ts = &vdata[CS_HRES_COARSE].basetime[CLOCK_REALTIME];
> --- a/lib/vdso/Kconfig
> +++ b/lib/vdso/Kconfig
> @@ -30,4 +30,7 @@ config GENERIC_VDSO_TIME_NS
> Selected by architectures which support time namespaces in the
> VDSO
>
> +config GENERIC_VDSO_CLOCK_MODE
> + bool
> +
> endif
> --- a/lib/vdso/gettimeofday.c
> +++ b/lib/vdso/gettimeofday.c
> @@ -7,6 +7,7 @@
> #include <linux/time.h>
> #include <linux/kernel.h>
> #include <linux/hrtimer_defs.h>
> +#include <linux/clocksource.h>
> #include <vdso/datapage.h>
> #include <vdso/helpers.h>
>
> @@ -64,10 +65,14 @@ static int do_hres_timens(const struct v
>
> do {
> seq = vdso_read_begin(vd);
> + if (IS_ENABLED(CONFIG_GENERIC_VDSO_CLOCK_MODE) &&
> + vd->clock_mode == VDSO_CLOCKMODE_NONE)
> + return -1;
> cycles = __arch_get_hw_counter(vd->clock_mode);
> ns = vdso_ts->nsec;
> last = vd->cycle_last;
> - if (unlikely((s64)cycles < 0))
> + if (!IS_ENABLED(CONFIG_GENERIC_VDSO_CLOCK_MODE) &&
> + unlikely((s64)cycles < 0))
> return -1;
>
> ns += vdso_calc_delta(cycles, last, vd->mask, vd->mult);
> @@ -132,10 +137,14 @@ static __always_inline int do_hres(const
> }
> smp_rmb();
>
> + if (IS_ENABLED(CONFIG_GENERIC_VDSO_CLOCK_MODE) &&
> + vd->clock_mode == VDSO_CLOCKMODE_NONE)
> + return -1;
> cycles = __arch_get_hw_counter(vd->clock_mode);
> ns = vdso_ts->nsec;
> last = vd->cycle_last;
> - if (unlikely((s64)cycles < 0))
> + if (!IS_ENABLED(CONFIG_GENERIC_VDSO_CLOCK_MODE) &&
> + unlikely((s64)cycles < 0))
> return -1;
>
> ns += vdso_calc_delta(cycles, last, vd->mask, vd->mult);
>
Powered by blists - more mailing lists