linux-kernel - Re: [PATCH 6/7] clocksource/arm_arch_timer: Remove use of workaround static key

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 8 Apr 2019 17:05:24 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Marc Zyngier <marc.zyngier@....com>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Russell King <linux@....linux.org.uk>,
        Will Deacon <will.deacon@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Valentin Schneider <valentin.schneider@....com>
Subject: Re: [PATCH 6/7] clocksource/arm_arch_timer: Remove use of workaround
 static key

On Mon, Apr 08, 2019 at 04:49:06PM +0100, Marc Zyngier wrote:
> The use of a static key in a hotplug path has proved to be a real
> nightmare, and makes it impossible to have scream-free lockdep
> kernel.
> 
> Let's remove the static key altogether, and focus on something saner.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@....com>

Acked-by: Mark Rutland <mark.rutland@....com>

Mark.

> ---
>  arch/arm64/include/asm/arch_timer.h  |  4 ----
>  drivers/clocksource/arm_arch_timer.c | 25 +++++++------------------
>  2 files changed, 7 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index 4a06d46def7e..5502ea049b63 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -45,13 +45,9 @@
>  		(__wa && __wa->h) ? __wa->h : arch_timer_##h;		\
>  	})
>  
> -extern struct static_key_false arch_timer_read_ool_enabled;
> -#define needs_unstable_timer_counter_workaround() \
> -	static_branch_unlikely(&arch_timer_read_ool_enabled)
>  #else
>  #define has_erratum_handler(h)			   false
>  #define erratum_handler(h)			   (arch_timer_##h)
> -#define needs_unstable_timer_counter_workaround()  false
>  #endif
>  
>  enum arch_timer_erratum_match_type {
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index c7f5b66d893c..da487fbfada3 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -372,8 +372,6 @@ static u32 notrace sun50i_a64_read_cntv_tval_el0(void)
>  DEFINE_PER_CPU(const struct arch_timer_erratum_workaround *, timer_unstable_counter_workaround);
>  EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
>  
> -DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);
> -EXPORT_SYMBOL_GPL(arch_timer_read_ool_enabled);
>  
>  static void erratum_set_next_event_tval_generic(const int access, unsigned long evt,
>  						struct clock_event_device *clk)
> @@ -552,12 +550,6 @@ void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa
>  			per_cpu(timer_unstable_counter_workaround, i) = wa;
>  	}
>  
> -	/*
> -	 * Use the locked version, as we're called from the CPU
> -	 * hotplug framework. Otherwise, we end-up in deadlock-land.
> -	 */
> -	static_branch_enable_cpuslocked(&arch_timer_read_ool_enabled);
> -
>  	/*
>  	 * Don't use the vdso fastpath if errata require using the
>  	 * out-of-line counter accessor. We may change our mind pretty
> @@ -573,7 +565,7 @@ void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa
>  static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type type,
>  					    void *arg)
>  {
> -	const struct arch_timer_erratum_workaround *wa;
> +	const struct arch_timer_erratum_workaround *wa, *__wa;
>  	ate_match_fn_t match_fn = NULL;
>  	bool local = false;
>  
> @@ -597,16 +589,13 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
>  	if (!wa)
>  		return;
>  
> -	if (needs_unstable_timer_counter_workaround()) {
> -		const struct arch_timer_erratum_workaround *__wa;
> -		__wa = __this_cpu_read(timer_unstable_counter_workaround);
> -		if (__wa && wa != __wa)
> -			pr_warn("Can't enable workaround for %s (clashes with %s\n)",
> -				wa->desc, __wa->desc);
> +	__wa = __this_cpu_read(timer_unstable_counter_workaround);
> +	if (__wa && wa != __wa)
> +		pr_warn("Can't enable workaround for %s (clashes with %s\n)",
> +			wa->desc, __wa->desc);
>  
> -		if (__wa)
> -			return;
> -	}
> +	if (__wa)
> +		return;
>  
>  	arch_timer_enable_workaround(wa, local);
>  	pr_info("Enabling %s workaround for %s\n",
> -- 
> 2.20.1
>