2020-01-14 19:50:19

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 09/15] clocksource: Add common vdso clock mode storage

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 <[email protected]>
---
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;

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

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


2020-01-15 05:51:50

by Christophe Leroy

[permalink] [raw]
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 <[email protected]>
> ---
> 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);
>

2020-01-15 08:01:17

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 09/15] clocksource: Add common vdso clock mode storage

Christophe Leroy <[email protected]> writes:
>> --- 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.

I know. It's intermediate and goes away a few patches later, but yes I
can fix it up.

Thanks,

tglx