From: Vincenzo Frascino <vincenzo.frascino@arm.com>
To: Thomas Gleixner <tglx@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>
Cc: x86@kernel.org, John Stultz <john.stultz@linaro.org>,
Andy Lutomirski <luto@kernel.org>,
Christophe Leroy <christophe.leroy@c-s.fr>,
Paolo Bonzini <pbonzini@redhat.com>,
Juergen Gross <jgross@suse.com>,
Michael Kelley <mikelley@microsoft.com>,
Sasha Levin <sashal@kernel.org>,
Ralf Baechle <ralf@linux-mips.org>,
Paul Burton <paulburton@kernel.org>,
James Hogan <jhogan@kernel.org>,
Russell King <linux@armlinux.org.uk>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Marc Zyngier <maz@kernel.org>, Andrei Vagin <avagin@gmail.com>
Subject: Re: [patch V2 07/17] x86/vdso: Move VDSO clocksource state tracking to callback
Date: Fri, 14 Feb 2020 11:58:54 +0000 [thread overview]
Message-ID: <56c8b4a3-31d0-ccfa-49f2-9fe0e5184e96@arm.com> (raw)
In-Reply-To: <20200207124402.934519777@linutronix.de>
On 2/7/20 12:38 PM, Thomas Gleixner wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> 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.
>
> X86 abuses the function which retrieves the architecture specific clock
> mode storage to mark the clocksource as used in the VDSO. That's silly
> because this is invoked on every tick when the VDSO data is updated.
>
> Move this functionality to the clocksource::enable() callback so it gets
> invoked once when the clocksource is installed. This allows to make the
> clock mode storage generic.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Reviewed-by: Michael Kelley <mikelley@microsoft.com> (Hyper-V parts)
> Acked-by: Juergen Gross <jgross@suse.com> (Xen parts)
>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com> (VDSO parts)
> ---
> arch/x86/entry/vdso/vma.c | 4 ++++
> arch/x86/include/asm/clocksource.h | 12 ++++++++++++
> arch/x86/include/asm/mshyperv.h | 2 ++
> arch/x86/include/asm/vdso/vsyscall.h | 10 +---------
> arch/x86/include/asm/vgtod.h | 6 ------
> arch/x86/kernel/kvmclock.c | 7 +++++++
> arch/x86/kernel/tsc.c | 32 ++++++++++++++++++++------------
> arch/x86/xen/time.c | 17 ++++++++++++-----
> drivers/clocksource/hyperv_timer.c | 7 +++++++
> 9 files changed, 65 insertions(+), 32 deletions(-)
>
> --- a/arch/x86/entry/vdso/vma.c
> +++ b/arch/x86/entry/vdso/vma.c
> @@ -38,6 +38,8 @@ struct vdso_data *arch_get_vdso_data(voi
> }
> #undef EMIT_VVAR
>
> +unsigned int vclocks_used __read_mostly;
> +
> #if defined(CONFIG_X86_64)
> unsigned int __read_mostly vdso64_enabled = 1;
> #endif
> @@ -445,6 +447,8 @@ static __init int vdso_setup(char *s)
>
> static int __init init_vdso(void)
> {
> + BUILD_BUG_ON(VCLOCK_MAX >= 32);
> +
> init_vdso_image(&vdso_image_64);
>
> #ifdef CONFIG_X86_X32_ABI
> --- a/arch/x86/include/asm/clocksource.h
> +++ b/arch/x86/include/asm/clocksource.h
> @@ -14,4 +14,16 @@ struct arch_clocksource_data {
> int vclock_mode;
> };
>
> +extern unsigned int vclocks_used;
> +
> +static inline bool vclock_was_used(int vclock)
> +{
> + return READ_ONCE(vclocks_used) & (1U << vclock);
> +}
> +
> +static inline void vclocks_set_used(unsigned int which)
> +{
> + WRITE_ONCE(vclocks_used, READ_ONCE(vclocks_used) | (1 << which));
> +}
> +
> #endif /* _ASM_X86_CLOCKSOURCE_H */
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -47,6 +47,8 @@ typedef int (*hyperv_fill_flush_list_fun
> wrmsrl(HV_X64_MSR_REFERENCE_TSC, val)
> #define hv_set_clocksource_vdso(val) \
> ((val).archdata.vclock_mode = VCLOCK_HVCLOCK)
> +#define hv_enable_vdso_clocksource() \
> + vclocks_set_used(VCLOCK_HVCLOCK);
> #define hv_get_raw_timer() rdtsc_ordered()
>
> void hyperv_callback_vector(void);
> --- a/arch/x86/include/asm/vdso/vsyscall.h
> +++ b/arch/x86/include/asm/vdso/vsyscall.h
> @@ -10,8 +10,6 @@
> #include <asm/vgtod.h>
> #include <asm/vvar.h>
>
> -int vclocks_used __read_mostly;
> -
> DEFINE_VVAR(struct vdso_data, _vdso_data);
> /*
> * Update the vDSO data page to keep in sync with kernel timekeeping.
> @@ -26,13 +24,7 @@ struct vdso_data *__x86_get_k_vdso_data(
> static __always_inline
> int __x86_get_clock_mode(struct timekeeper *tk)
> {
> - int vclock_mode = tk->tkr_mono.clock->archdata.vclock_mode;
> -
> - /* Mark the new vclock used. */
> - BUILD_BUG_ON(VCLOCK_MAX >= 32);
> - WRITE_ONCE(vclocks_used, READ_ONCE(vclocks_used) | (1 << vclock_mode));
> -
> - return vclock_mode;
> + return tk->tkr_mono.clock->archdata.vclock_mode;
> }
> #define __arch_get_clock_mode __x86_get_clock_mode
>
> --- a/arch/x86/include/asm/vgtod.h
> +++ b/arch/x86/include/asm/vgtod.h
> @@ -15,10 +15,4 @@ typedef u64 gtod_long_t;
> typedef unsigned long gtod_long_t;
> #endif
>
> -extern int vclocks_used;
> -static inline bool vclock_was_used(int vclock)
> -{
> - return READ_ONCE(vclocks_used) & (1 << vclock);
> -}
> -
> #endif /* _ASM_X86_VGTOD_H */
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -159,12 +159,19 @@ bool kvm_check_and_clear_guest_paused(vo
> return ret;
> }
>
> +static int kvm_cs_enable(struct clocksource *cs)
> +{
> + vclocks_set_used(VCLOCK_PVCLOCK);
> + return 0;
> +}
> +
> struct clocksource kvm_clock = {
> .name = "kvm-clock",
> .read = kvm_clock_get_cycles,
> .rating = 400,
> .mask = CLOCKSOURCE_MASK(64),
> .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> + .enable = kvm_cs_enable,
> };
> EXPORT_SYMBOL_GPL(kvm_clock);
>
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1108,17 +1108,24 @@ static void tsc_cs_tick_stable(struct cl
> sched_clock_tick_stable();
> }
>
> +static int tsc_cs_enable(struct clocksource *cs)
> +{
> + vclocks_set_used(VCLOCK_TSC);
> + return 0;
> +}
> +
> /*
> * .mask MUST be CLOCKSOURCE_MASK(64). See comment above read_tsc()
> */
> static struct clocksource clocksource_tsc_early = {
> - .name = "tsc-early",
> - .rating = 299,
> - .read = read_tsc,
> - .mask = CLOCKSOURCE_MASK(64),
> - .flags = CLOCK_SOURCE_IS_CONTINUOUS |
> + .name = "tsc-early",
> + .rating = 299,
> + .read = read_tsc,
> + .mask = CLOCKSOURCE_MASK(64),
> + .flags = CLOCK_SOURCE_IS_CONTINUOUS |
> CLOCK_SOURCE_MUST_VERIFY,
> - .archdata = { .vclock_mode = VCLOCK_TSC },
> + .archdata = { .vclock_mode = VCLOCK_TSC },
> + .enable = tsc_cs_enable,
> .resume = tsc_resume,
> .mark_unstable = tsc_cs_mark_unstable,
> .tick_stable = tsc_cs_tick_stable,
> @@ -1131,14 +1138,15 @@ static struct clocksource clocksource_ts
> * been found good.
> */
> static struct clocksource clocksource_tsc = {
> - .name = "tsc",
> - .rating = 300,
> - .read = read_tsc,
> - .mask = CLOCKSOURCE_MASK(64),
> - .flags = CLOCK_SOURCE_IS_CONTINUOUS |
> + .name = "tsc",
> + .rating = 300,
> + .read = read_tsc,
> + .mask = CLOCKSOURCE_MASK(64),
> + .flags = CLOCK_SOURCE_IS_CONTINUOUS |
> CLOCK_SOURCE_VALID_FOR_HRES |
> CLOCK_SOURCE_MUST_VERIFY,
> - .archdata = { .vclock_mode = VCLOCK_TSC },
> + .archdata = { .vclock_mode = VCLOCK_TSC },
> + .enable = tsc_cs_enable,
> .resume = tsc_resume,
> .mark_unstable = tsc_cs_mark_unstable,
> .tick_stable = tsc_cs_tick_stable,
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -145,12 +145,19 @@ static struct notifier_block xen_pvclock
> .notifier_call = xen_pvclock_gtod_notify,
> };
>
> +static int xen_cs_enable(struct clocksource *cs)
> +{
> + vclocks_set_used(VCLOCK_PVCLOCK);
> + return 0;
> +}
> +
> static struct clocksource xen_clocksource __read_mostly = {
> - .name = "xen",
> - .rating = 400,
> - .read = xen_clocksource_get_cycles,
> - .mask = ~0,
> - .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> + .name = "xen",
> + .rating = 400,
> + .read = xen_clocksource_get_cycles,
> + .mask = CLOCKSOURCE_MASK(64),
> + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> + .enable = xen_cs_enable,
> };
>
> /*
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -369,6 +369,12 @@ static void resume_hv_clock_tsc(struct c
> hv_set_reference_tsc(tsc_msr);
> }
>
> +static int hv_cs_enable(struct clocksource *cs)
> +{
> + hv_enable_vdso_clocksource();
> + return 0;
> +}
> +
> static struct clocksource hyperv_cs_tsc = {
> .name = "hyperv_clocksource_tsc_page",
> .rating = 250,
> @@ -377,6 +383,7 @@ static struct clocksource hyperv_cs_tsc
> .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> .suspend= suspend_hv_clock_tsc,
> .resume = resume_hv_clock_tsc,
> + .enable = hv_cs_enable,
> };
>
> static u64 notrace read_hv_clock_msr(void)
>
--
Regards,
Vincenzo
next prev parent reply other threads:[~2020-02-14 11:58 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-07 12:38 [patch V2 00/17] VDSO consolidation Thomas Gleixner
2020-02-07 12:38 ` [patch V2 01/17] x86/vdso: Mark the TSC clocksource path likely Thomas Gleixner
2020-02-14 12:00 ` Vincenzo Frascino
2020-02-17 15:12 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-07 12:38 ` [patch V2 02/17] ARM: vdso: Remove unused function Thomas Gleixner
2020-02-14 10:21 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-07 12:38 ` [patch V2 03/17] lib/vdso: Allow the high resolution parts to be compiled out Thomas Gleixner
2020-02-14 11:54 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-07 12:38 ` [patch V2 04/17] ARM: vdso: Compile high resolution parts conditionally Thomas Gleixner
2020-02-14 11:55 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-07 12:38 ` [patch V2 05/17] MIPS: " Thomas Gleixner
2020-02-14 11:55 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-07 12:38 ` [patch V2 06/17] clocksource: Cleanup struct clocksource and documentation Thomas Gleixner
2020-02-14 11:57 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-07 12:38 ` [patch V2 07/17] x86/vdso: Move VDSO clocksource state tracking to callback Thomas Gleixner
2020-02-14 11:58 ` Vincenzo Frascino [this message]
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-07 12:38 ` [patch V2 08/17] clocksource: Add common vdso clock mode storage Thomas Gleixner
2020-02-17 10:36 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-07 12:38 ` [patch V2 09/17] x86/vdso: Use generic VDSO " Thomas Gleixner
2020-02-14 10:32 ` Paolo Bonzini
2020-02-17 10:57 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-07 12:38 ` [patch V2 10/17] mips: vdso: " Thomas Gleixner
2020-02-17 10:52 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-17 19:18 ` tip-bot2 for Thomas Gleixner
2020-02-07 12:38 ` [patch V2 11/17] ARM/arm64: vdso: Use common vdso " Thomas Gleixner
2020-02-17 10:43 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-17 19:18 ` tip-bot2 for Thomas Gleixner
[not found] ` <CGME20200221115643eucas1p12ecb95c6161853a0e7dfe9207db079be@eucas1p1.samsung.com>
2020-02-21 11:56 ` [patch V2 11/17] " Marek Szyprowski
2020-02-21 13:08 ` Vincenzo Frascino
2020-02-21 18:24 ` Vincenzo Frascino
2020-02-07 12:38 ` [patch V2 12/17] lib/vdso: Cleanup clock mode storage leftovers Thomas Gleixner
2020-02-17 11:04 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-17 19:18 ` tip-bot2 for Thomas Gleixner
2020-02-07 12:39 ` [patch V2 13/17] lib/vdso: Avoid highres update if clocksource is not VDSO capable Thomas Gleixner
2020-02-17 11:07 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-17 19:18 ` tip-bot2 for Thomas Gleixner
2020-02-07 12:39 ` [patch V2 14/17] lib/vdso: Move VCLOCK_TIMENS to vdso_clock_modes Thomas Gleixner
2020-02-17 11:12 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-02-17 19:18 ` tip-bot2 for Thomas Gleixner
2020-02-07 12:39 ` [patch V2 15/17] lib/vdso: Allow fixed clock mode Thomas Gleixner
2020-02-17 11:14 ` Vincenzo Frascino
2020-02-17 15:11 ` [tip: timers/core] " tip-bot2 for Christophe Leroy
2020-02-17 19:18 ` tip-bot2 for Christophe Leroy
2020-02-07 12:39 ` [patch V2 16/17] lib/vdso: Allow architectures to override the ns shift operation Thomas Gleixner
2020-02-17 11:15 ` Vincenzo Frascino
2020-02-07 12:39 ` [patch V2 17/17] lib/vdso: Allow architectures to provide the vdso data pointer Thomas Gleixner
2020-02-17 12:09 ` Vincenzo Frascino
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56c8b4a3-31d0-ccfa-49f2-9fe0e5184e96@arm.com \
--to=vincenzo.frascino@arm.com \
--cc=avagin@gmail.com \
--cc=catalin.marinas@arm.com \
--cc=christophe.leroy@c-s.fr \
--cc=jgross@suse.com \
--cc=jhogan@kernel.org \
--cc=john.stultz@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=luto@kernel.org \
--cc=mark.rutland@arm.com \
--cc=maz@kernel.org \
--cc=mikelley@microsoft.com \
--cc=paulburton@kernel.org \
--cc=pbonzini@redhat.com \
--cc=ralf@linux-mips.org \
--cc=sashal@kernel.org \
--cc=tglx@linutronix.de \
--cc=will@kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®