linux-kernel - Re: [PATCH 1/2] cpu/hotplug: Mute hotplug lockdep during init

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:   Wed, 19 Dec 2018 18:31:49 +0000
From:   Marc Zyngier <marc.zyngier@....com>
To:     Valentin Schneider <valentin.schneider@....com>,
        linux-kernel@...r.kernel.org
Cc:     cai@....us, daniel.lezcano@...aro.org,
        linux-arm-kernel@...ts.infradead.org, longman@...hat.com,
        mark.rutland@....com, mingo@...nel.org, peterz@...radead.org,
        tglx@...utronix.de, dietmar.eggemann@....com
Subject: Re: [PATCH 1/2] cpu/hotplug: Mute hotplug lockdep during init

On 19/12/2018 18:23, Valentin Schneider wrote:
> Since we've had:
> 
>   commit cb538267ea1e ("jump_label/lockdep: Assert we hold the hotplug lock for _cpuslocked() operations")
> 
> we've been getting some lockdep warnings during init, such as on HiKey960:
> 
> [    0.820495] WARNING: CPU: 4 PID: 0 at kernel/cpu.c:316 lockdep_assert_cpus_held+0x3c/0x48
> [    0.820498] Modules linked in:
> [    0.820509] CPU: 4 PID: 0 Comm: swapper/4 Tainted: G S                4.20.0-rc5-00051-g4cae42a #34
> [    0.820511] Hardware name: HiKey960 (DT)
> [    0.820516] pstate: 600001c5 (nZCv dAIF -PAN -UAO)
> [    0.820520] pc : lockdep_assert_cpus_held+0x3c/0x48
> [    0.820523] lr : lockdep_assert_cpus_held+0x38/0x48
> [    0.820526] sp : ffff00000a9cbe50
> [    0.820528] x29: ffff00000a9cbe50 x28: 0000000000000000
> [    0.820533] x27: 00008000b69e5000 x26: ffff8000bff4cfe0
> [    0.820537] x25: ffff000008ba69e0 x24: 0000000000000001
> [    0.820541] x23: ffff000008fce000 x22: ffff000008ba70c8
> [    0.820545] x21: 0000000000000001 x20: 0000000000000003
> [    0.820548] x19: ffff00000a35d628 x18: ffffffffffffffff
> [    0.820552] x17: 0000000000000000 x16: 0000000000000000
> [    0.820556] x15: ffff00000958f848 x14: 455f3052464d4d34
> [    0.820559] x13: 00000000769dde98 x12: ffff8000bf3f65a8
> [    0.820564] x11: 0000000000000000 x10: ffff00000958f848
> [    0.820567] x9 : ffff000009592000 x8 : ffff00000958f848
> [    0.820571] x7 : ffff00000818ffa0 x6 : 0000000000000000
> [    0.820574] x5 : 0000000000000000 x4 : 0000000000000001
> [    0.820578] x3 : 0000000000000000 x2 : 0000000000000001
> [    0.820582] x1 : 00000000ffffffff x0 : 0000000000000000
> [    0.820587] Call trace:
> [    0.820591]  lockdep_assert_cpus_held+0x3c/0x48
> [    0.820598]  static_key_enable_cpuslocked+0x28/0xd0
> [    0.820606]  arch_timer_check_ool_workaround+0xe8/0x228
> [    0.820610]  arch_timer_starting_cpu+0xe4/0x2d8
> [    0.820615]  cpuhp_invoke_callback+0xe8/0xd08
> [    0.820619]  notify_cpu_starting+0x80/0xb8
> [    0.820625]  secondary_start_kernel+0x118/0x1d0
> 
> We've also had a similar warning in sched_init_smp() for every
> asymmetric system that would enable the sched_asym_cpucapacity static
> key, although that was singled out in:
> 
>   commit 40fa3780bac2 ("sched/core: Take the hotplug lock in sched_init_smp()")
> 
> Those warnings are actually harmless, since we cannot have hotplug
> operations at the time they appear. Instead of starting to sprinkle
> useless hotplug lock operations in the init codepaths, mute the
> warnings until they start warning about real problems.
> 
> Suggested-by: Peter Zijlstra <peterz@...radead.org>
> Signed-off-by: Valentin Schneider <valentin.schneider@....com>
> ---
> FYI Thomas Gleixner suggested using SYSTEM_SCHEDULING instead of
> SYSTEM_RUNNING, but that seems to still be too early - sched_init_smp()
> (and kernel_init() actually) hasn't completed yet, so we'd still get
> those warnings.
> ---
>  kernel/cpu.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 91d5c38..34e40ef 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -313,6 +313,15 @@ void cpus_write_unlock(void)
>  
>  void lockdep_assert_cpus_held(void)
>  {
> +	/*
> +	 * We can't have hotplug operations before userspace starts running,
> +	 * and some init codepaths will knowingly not take the hotplug lock.
> +	 * This is all valid, so mute lockdep until it makes sense to report
> +	 * unheld locks.
> +	 */
> +	if (system_state < SYSTEM_RUNNING)
> +		return;
> +
>  	percpu_rwsem_assert_held(&cpu_hotplug_lock);
>  }
>  
> --
> 2.7.4
> 

FWIW: Acked-by: Marc Zyngier <marc.zyngier@....com>

Thanks for having kept an eye on it!

	M.
-- 
Jazz is not dead. It just smells funny...