[PATCH 0/2] System-wide mute of hotplug lockdep during init

[PATCH 0/2] System-wide mute of hotplug lockdep during init

[Valentin Schneider]

In [1] Peter had a suggestion to broadly mute hotplug lockdep warnings during
init instead of going after every single warning and wrapping them with
cpus_read_{lock, unlock}(). This is implemented in [PATCH 1/2].

[PATCH 2/2] is a logical follow-up as [PATCH 1/2] is the generalized version
of the reverted patch.

[1]: https://lore.kernel.org/lkml/1543877121-4098-1-git-send-email-cai@gmx.us/#t

Valentin Schneider (2):
  cpu/hotplug: Mute hotplug lockdep during init
  Revert "sched/core: Take the hotplug lock in sched_init_smp()"

 kernel/cpu.c        | 9 +++++++++
 kernel/sched/core.c | 5 +----
 2 files changed, 10 insertions(+), 4 deletions(-)

--
2.7.4

[PATCH 0/2] System-wide mute of hotplug lockdep during init

[Valentin Schneider]

In [1] Peter had a suggestion to broadly mute hotplug lockdep warnings during
init instead of going after every single warning and wrapping them with
cpus_read_{lock, unlock}(). This is implemented in [PATCH 1/2].

[PATCH 2/2] is a logical follow-up as [PATCH 1/2] is the generalized version
of the reverted patch.

[1]: https://lore.kernel.org/lkml/1543877121-4098-1-git-send-email-cai@gmx.us/#t

Valentin Schneider (2):
  cpu/hotplug: Mute hotplug lockdep during init
  Revert "sched/core: Take the hotplug lock in sched_init_smp()"

 kernel/cpu.c        | 9 +++++++++
 kernel/sched/core.c | 5 +----
 2 files changed, 10 insertions(+), 4 deletions(-)

--
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] cpu/hotplug: Mute hotplug lockdep during init

[Valentin Schneider]

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@infradead.org>
Signed-off-by: Valentin Schneider <valentin.schneider@arm.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

[PATCH 1/2] cpu/hotplug: Mute hotplug lockdep during init

[Valentin Schneider]

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@infradead.org>
Signed-off-by: Valentin Schneider <valentin.schneider@arm.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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] Revert "sched/core: Take the hotplug lock in sched_init_smp()"

[Valentin Schneider]

This reverts commit 40fa3780bac2b654edf23f6b13f4e2dd550aea10.

Now that we have a system-wide muting of hotplug lockdep during init,
this is no longer needed.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/sched/core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index e4ca15d..144b778 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5854,14 +5854,11 @@ void __init sched_init_smp(void)
 	/*
 	 * There's no userspace yet to cause hotplug operations; hence all the
 	 * CPU masks are stable and all blatant races in the below code cannot
-	 * happen. The hotplug lock is nevertheless taken to satisfy lockdep,
-	 * but there won't be any contention on it.
+	 * happen.
 	 */
-	cpus_read_lock();
 	mutex_lock(&sched_domains_mutex);
 	sched_init_domains(cpu_active_mask);
 	mutex_unlock(&sched_domains_mutex);
-	cpus_read_unlock();
 
 	/* Move init over to a non-isolated CPU */
 	if (set_cpus_allowed_ptr(current, housekeeping_cpumask(HK_FLAG_DOMAIN)) < 0)
--
2.7.4

[PATCH 2/2] Revert "sched/core: Take the hotplug lock in sched_init_smp()"

[Valentin Schneider]

This reverts commit 40fa3780bac2b654edf23f6b13f4e2dd550aea10.

Now that we have a system-wide muting of hotplug lockdep during init,
this is no longer needed.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/sched/core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index e4ca15d..144b778 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5854,14 +5854,11 @@ void __init sched_init_smp(void)
 	/*
 	 * There's no userspace yet to cause hotplug operations; hence all the
 	 * CPU masks are stable and all blatant races in the below code cannot
-	 * happen. The hotplug lock is nevertheless taken to satisfy lockdep,
-	 * but there won't be any contention on it.
+	 * happen.
 	 */
-	cpus_read_lock();
 	mutex_lock(&sched_domains_mutex);
 	sched_init_domains(cpu_active_mask);
 	mutex_unlock(&sched_domains_mutex);
-	cpus_read_unlock();
 
 	/* Move init over to a non-isolated CPU */
 	if (set_cpus_allowed_ptr(current, housekeeping_cpumask(HK_FLAG_DOMAIN)) < 0)
--
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Marc Zyngier]

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@infradead.org>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.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@arm.com>

Thanks for having kept an eye on it!

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

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

[Marc Zyngier]

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@infradead.org>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.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@arm.com>

Thanks for having kept an eye on it!

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] System-wide mute of hotplug lockdep during init

[Valentin Schneider]

Hi,

On 19/12/2018 18:23, Valentin Schneider wrote:
> In [1] Peter had a suggestion to broadly mute hotplug lockdep warnings during
> init instead of going after every single warning and wrapping them with
> cpus_read_{lock, unlock}(). This is implemented in [PATCH 1/2].
> 
> [PATCH 2/2] is a logical follow-up as [PATCH 1/2] is the generalized version
> of the reverted patch.
> 
> [1]: https://lore.kernel.org/lkml/1543877121-4098-1-git-send-email-cai@gmx.us/#t
> 

Gentle ping - I suppose this flew under the holidays radar.

> Valentin Schneider (2):
>   cpu/hotplug: Mute hotplug lockdep during init
>   Revert "sched/core: Take the hotplug lock in sched_init_smp()"
> 
>  kernel/cpu.c        | 9 +++++++++
>  kernel/sched/core.c | 5 +----
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> --
> 2.7.4
> 

Re: [PATCH 0/2] System-wide mute of hotplug lockdep during init

[Valentin Schneider]

Hi,

On 19/12/2018 18:23, Valentin Schneider wrote:
> In [1] Peter had a suggestion to broadly mute hotplug lockdep warnings during
> init instead of going after every single warning and wrapping them with
> cpus_read_{lock, unlock}(). This is implemented in [PATCH 1/2].
> 
> [PATCH 2/2] is a logical follow-up as [PATCH 1/2] is the generalized version
> of the reverted patch.
> 
> [1]: https://lore.kernel.org/lkml/1543877121-4098-1-git-send-email-cai@gmx.us/#t
> 

Gentle ping - I suppose this flew under the holidays radar.

> Valentin Schneider (2):
>   cpu/hotplug: Mute hotplug lockdep during init
>   Revert "sched/core: Take the hotplug lock in sched_init_smp()"
> 
>  kernel/cpu.c        | 9 +++++++++
>  kernel/sched/core.c | 5 +----
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> --
> 2.7.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] System-wide mute of hotplug lockdep during init

[Peter Zijlstra]

On Fri, Jan 11, 2019 at 09:53:58AM +0000, Valentin Schneider wrote:
> Hi,
> 
> On 19/12/2018 18:23, Valentin Schneider wrote:
> > In [1] Peter had a suggestion to broadly mute hotplug lockdep warnings during
> > init instead of going after every single warning and wrapping them with
> > cpus_read_{lock, unlock}(). This is implemented in [PATCH 1/2].
> > 
> > [PATCH 2/2] is a logical follow-up as [PATCH 1/2] is the generalized version
> > of the reverted patch.
> > 
> > [1]: https://lore.kernel.org/lkml/1543877121-4098-1-git-send-email-cai@gmx.us/#t
> > 
> 
> Gentle ping - I suppose this flew under the holidays radar.

Indeed so... got them now. Thanks!

Re: [PATCH 0/2] System-wide mute of hotplug lockdep during init

[Peter Zijlstra]

On Fri, Jan 11, 2019 at 09:53:58AM +0000, Valentin Schneider wrote:
> Hi,
> 
> On 19/12/2018 18:23, Valentin Schneider wrote:
> > In [1] Peter had a suggestion to broadly mute hotplug lockdep warnings during
> > init instead of going after every single warning and wrapping them with
> > cpus_read_{lock, unlock}(). This is implemented in [PATCH 1/2].
> > 
> > [PATCH 2/2] is a logical follow-up as [PATCH 1/2] is the generalized version
> > of the reverted patch.
> > 
> > [1]: https://lore.kernel.org/lkml/1543877121-4098-1-git-send-email-cai@gmx.us/#t
> > 
> 
> Gentle ping - I suppose this flew under the holidays radar.

Indeed so... got them now. Thanks!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[tip:locking/core] cpu/hotplug: Mute hotplug lockdep during init

[tip-bot for Valentin Schneider]

Commit-ID:  ce48c457b95316b9a01b5aa9d4456ce820df94b4
Gitweb:     https://git.kernel.org/tip/ce48c457b95316b9a01b5aa9d4456ce820df94b4
Author:     Valentin Schneider <valentin.schneider@arm.com>
AuthorDate: Wed, 19 Dec 2018 18:23:15 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 21 Jan 2019 11:18:53 +0100

cpu/hotplug: Mute hotplug lockdep during init

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@infradead.org>
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: cai@gmx.us
Cc: daniel.lezcano@linaro.org
Cc: dietmar.eggemann@arm.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: longman@redhat.com
Cc: marc.zyngier@arm.com
Cc: mark.rutland@arm.com
Link: https://lkml.kernel.org/r/1545243796-23224-2-git-send-email-valentin.schneider@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/cpu.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 91d5c38eb7e5..34e40ef2f6e0 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);
 }
 

[tip:locking/core] Revert "sched/core: Take the hotplug lock in sched_init_smp()"

[tip-bot for Valentin Schneider]

Commit-ID:  b5a4e2bb0f4c86bfeb38df3e1d5b2f1272f0e673
Gitweb:     https://git.kernel.org/tip/b5a4e2bb0f4c86bfeb38df3e1d5b2f1272f0e673
Author:     Valentin Schneider <valentin.schneider@arm.com>
AuthorDate: Wed, 19 Dec 2018 18:23:16 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 21 Jan 2019 11:18:54 +0100

Revert "sched/core: Take the hotplug lock in sched_init_smp()"

This reverts commit 40fa3780bac2b654edf23f6b13f4e2dd550aea10.

Now that we have a system-wide muting of hotplug lockdep during init,
this is no longer needed.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: cai@gmx.us
Cc: daniel.lezcano@linaro.org
Cc: dietmar.eggemann@arm.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: longman@redhat.com
Cc: marc.zyngier@arm.com
Cc: mark.rutland@arm.com
Link: https://lkml.kernel.org/r/1545243796-23224-3-git-send-email-valentin.schneider@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index b05eef7d7a1f..3c8b4dba3d2d 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5867,14 +5867,11 @@ void __init sched_init_smp(void)
 	/*
 	 * There's no userspace yet to cause hotplug operations; hence all the
 	 * CPU masks are stable and all blatant races in the below code cannot
-	 * happen. The hotplug lock is nevertheless taken to satisfy lockdep,
-	 * but there won't be any contention on it.
+	 * happen.
 	 */
-	cpus_read_lock();
 	mutex_lock(&sched_domains_mutex);
 	sched_init_domains(cpu_active_mask);
 	mutex_unlock(&sched_domains_mutex);
-	cpus_read_unlock();
 
 	/* Move init over to a non-isolated CPU */
 	if (set_cpus_allowed_ptr(current, housekeeping_cpumask(HK_FLAG_DOMAIN)) < 0)