* [PATCHv3 1/2] jump_label: Provide static_key_[enable|/slow_inc]_cpuslocked()
2017-04-28 11:01 [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout Mark Rutland
@ 2017-04-28 11:01 ` Mark Rutland
2017-04-30 9:44 ` [tip:smp/hotplug] " tip-bot for Sebastian Andrzej Siewior
2017-04-28 11:01 ` [PATCHv3 2/2] arm64: cpufeature: use static_branch_enable_cpuslocked() Mark Rutland
2017-05-10 18:10 ` [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout Mark Rutland
2 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2017-04-28 11:01 UTC (permalink / raw)
To: tglx
Cc: linux-arm-kernel, linux-kernel, will.deacon, catalin.marinas,
bigeasy, jbaron, mark.rutland, peterz, rostedt, suzuki.poulose
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Provide static_key_[enable|slow_inc]_cpuslocked() variant that
don't take cpu_hotplug_lock().
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: jbaron@akamai.com
---
include/linux/jump_label.h | 7 +++++++
kernel/jump_label.c | 10 ++++++++++
2 files changed, 17 insertions(+)
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index d7b17d1..c80d8b1 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -164,6 +164,7 @@ extern void arch_jump_label_transform_static(struct jump_entry *entry,
extern void jump_label_apply_nops(struct module *mod);
extern int static_key_count(struct static_key *key);
extern void static_key_enable(struct static_key *key);
+extern void static_key_enable_cpuslocked(struct static_key *key);
extern void static_key_disable(struct static_key *key);
extern void static_key_disable_cpuslocked(struct static_key *key);
@@ -252,6 +253,11 @@ static inline void static_key_enable(struct static_key *key)
static_key_slow_inc(key);
}
+static inline void static_key_enable_cpuslocked(struct static_key *key)
+{
+ static_key_enable(key);
+}
+
static inline void static_key_disable(struct static_key *key)
{
int count = static_key_count(key);
@@ -429,6 +435,7 @@ struct static_key_false {
*/
#define static_branch_enable(x) static_key_enable(&(x)->key)
+#define static_branch_enable_cpuslocked(x) static_key_enable_cpuslocked(&(x)->key)
#define static_branch_disable(x) static_key_disable(&(x)->key)
#define static_branch_disable_cpuslocked(x) static_key_disable_cpuslocked(&(x)->key)
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d71124e..6343f4c 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -90,6 +90,16 @@ void static_key_enable(struct static_key *key)
}
EXPORT_SYMBOL_GPL(static_key_enable);
+void static_key_enable_cpuslocked(struct static_key *key)
+{
+ int count = static_key_count(key);
+
+ WARN_ON_ONCE(count < 0 || count > 1);
+
+ if (!count)
+ static_key_slow_inc_cpuslocked(key);
+}
+
void static_key_disable(struct static_key *key)
{
int count = static_key_count(key);
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [tip:smp/hotplug] jump_label: Provide static_key_[enable|/slow_inc]_cpuslocked()
2017-04-28 11:01 ` [PATCHv3 1/2] jump_label: Provide static_key_[enable|/slow_inc]_cpuslocked() Mark Rutland
@ 2017-04-30 9:44 ` tip-bot for Sebastian Andrzej Siewior
0 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Sebastian Andrzej Siewior @ 2017-04-30 9:44 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, mark.rutland, tglx, bigeasy, hpa, mingo
Commit-ID: 2006cddf3c2fd6b4114c704229552a903f180c71
Gitweb: http://git.kernel.org/tip/2006cddf3c2fd6b4114c704229552a903f180c71
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Fri, 28 Apr 2017 12:01:05 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 30 Apr 2017 11:19:01 +0200
jump_label: Provide static_key_[enable|/slow_inc]_cpuslocked()
Provide static_key_[enable|slow_inc]_cpuslocked() variant that
don't take cpu_hotplug_lock().
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: suzuki.poulose@arm.com
Cc: peterz@infradead.org
Cc: catalin.marinas@arm.com
Cc: will.deacon@arm.com
Cc: rostedt@goodmis.org
Cc: jbaron@akamai.com
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1493377266-2205-2-git-send-email-mark.rutland@arm.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
include/linux/jump_label.h | 7 +++++++
kernel/jump_label.c | 10 ++++++++++
2 files changed, 17 insertions(+)
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index d7b17d1..c80d8b1 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -164,6 +164,7 @@ extern void static_key_slow_dec_cpuslocked(struct static_key *key);
extern void jump_label_apply_nops(struct module *mod);
extern int static_key_count(struct static_key *key);
extern void static_key_enable(struct static_key *key);
+extern void static_key_enable_cpuslocked(struct static_key *key);
extern void static_key_disable(struct static_key *key);
extern void static_key_disable_cpuslocked(struct static_key *key);
@@ -252,6 +253,11 @@ static inline void static_key_enable(struct static_key *key)
static_key_slow_inc(key);
}
+static inline void static_key_enable_cpuslocked(struct static_key *key)
+{
+ static_key_enable(key);
+}
+
static inline void static_key_disable(struct static_key *key)
{
int count = static_key_count(key);
@@ -429,6 +435,7 @@ extern bool ____wrong_branch_error(void);
*/
#define static_branch_enable(x) static_key_enable(&(x)->key)
+#define static_branch_enable_cpuslocked(x) static_key_enable_cpuslocked(&(x)->key)
#define static_branch_disable(x) static_key_disable(&(x)->key)
#define static_branch_disable_cpuslocked(x) static_key_disable_cpuslocked(&(x)->key)
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d71124e..6343f4c 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -90,6 +90,16 @@ void static_key_enable(struct static_key *key)
}
EXPORT_SYMBOL_GPL(static_key_enable);
+void static_key_enable_cpuslocked(struct static_key *key)
+{
+ int count = static_key_count(key);
+
+ WARN_ON_ONCE(count < 0 || count > 1);
+
+ if (!count)
+ static_key_slow_inc_cpuslocked(key);
+}
+
void static_key_disable(struct static_key *key)
{
int count = static_key_count(key);
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCHv3 2/2] arm64: cpufeature: use static_branch_enable_cpuslocked()
2017-04-28 11:01 [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout Mark Rutland
2017-04-28 11:01 ` [PATCHv3 1/2] jump_label: Provide static_key_[enable|/slow_inc]_cpuslocked() Mark Rutland
@ 2017-04-28 11:01 ` Mark Rutland
2017-04-30 9:45 ` [tip:smp/hotplug] arm64/cpufeature: Use static_branch_enable_cpuslocked() tip-bot for Mark Rutland
2017-05-10 18:10 ` [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout Mark Rutland
2 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2017-04-28 11:01 UTC (permalink / raw)
To: tglx
Cc: linux-arm-kernel, linux-kernel, will.deacon, catalin.marinas,
bigeasy, jbaron, mark.rutland, peterz, rostedt, suzuki.poulose
Recently, the hotplug locking was conveted to use a percpu rwsem. Unlike
the existing {get,put}_online_cpus() logic, this can't nest.
Unfortunately, in arm64's secondary boot path we can end up nesting via
static_branch_enable() in cpus_set_cap() when we detect an erratum.
This leads to a stream of messages as below, where the secondary
attempts to schedule before it has been fully onlined. As the CPU
orchestrating the onlining holds the rswem, this hangs the system.
[ 0.250334] BUG: scheduling while atomic: swapper/1/0/0x00000002
[ 0.250337] Modules linked in:
[ 0.250346] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.11.0-rc7-next-20170424 #2
[ 0.250349] Hardware name: ARM Juno development board (r1) (DT)
[ 0.250353] Call trace:
[ 0.250365] [<ffff000008088510>] dump_backtrace+0x0/0x238
[ 0.250371] [<ffff00000808880c>] show_stack+0x14/0x20
[ 0.250377] [<ffff00000839d854>] dump_stack+0x9c/0xc0
[ 0.250384] [<ffff0000080e3540>] __schedule_bug+0x50/0x70
[ 0.250391] [<ffff000008932ecc>] __schedule+0x52c/0x5a8
[ 0.250395] [<ffff000008932f80>] schedule+0x38/0xa0
[ 0.250400] [<ffff000008935e8c>] rwsem_down_read_failed+0xc4/0x108
[ 0.250407] [<ffff0000080fe8e0>] __percpu_down_read+0x100/0x118
[ 0.250414] [<ffff0000080c0b60>] get_online_cpus+0x70/0x78
[ 0.250420] [<ffff0000081749e8>] static_key_enable+0x28/0x48
[ 0.250425] [<ffff00000808de90>] update_cpu_capabilities+0x78/0xf8
[ 0.250430] [<ffff00000808d14c>] update_cpu_errata_workarounds+0x1c/0x28
[ 0.250435] [<ffff00000808e004>] check_local_cpu_capabilities+0xf4/0x128
[ 0.250440] [<ffff00000808e894>] secondary_start_kernel+0x8c/0x118
[ 0.250444] [<000000008093d1b4>] 0x8093d1b4
We call cpus_set_cap() from update_cpu_capabilities(), which is called
from the secondary boot path (where the CPU orchestrating the onlining
holds the hotplug rwsem), and in the primary boot path, where this is
not held.
This patch makes cpus_set_cap() use static_branch_enable_cpuslocked(),
and updates all the callers of update_cpu_capabilities() consistent with
the change.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reported-by: Catalin Marinas <catalin.marinas@arm.com>
Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
[Mark: minor fixups]
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
arch/arm64/include/asm/cpufeature.h | 3 ++-
arch/arm64/kernel/cpu_errata.c | 9 ++++++++-
arch/arm64/kernel/cpufeature.c | 5 ++++-
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index f31c48d..a8d1b5b 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -145,7 +145,7 @@ static inline void cpus_set_cap(unsigned int num)
num, ARM64_NCAPS);
} else {
__set_bit(num, cpu_hwcaps);
- static_branch_enable(&cpu_hwcap_keys[num]);
+ static_branch_enable_cpuslocked(&cpu_hwcap_keys[num]);
}
}
@@ -223,6 +223,7 @@ void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
void check_local_cpu_capabilities(void);
void update_cpu_errata_workarounds(void);
+void update_cpu_errata_workarounds_cpuslocked(void);
void __init enable_errata_workarounds(void);
void verify_local_cpu_errata_workarounds(void);
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index f6cc67e..e650cf8 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -175,11 +175,18 @@ void verify_local_cpu_errata_workarounds(void)
}
}
-void update_cpu_errata_workarounds(void)
+void update_cpu_errata_workarounds_cpuslocked(void)
{
update_cpu_capabilities(arm64_errata, "enabling workaround for");
}
+void update_cpu_errata_workarounds(void)
+{
+ get_online_cpus();
+ update_cpu_errata_workarounds_cpuslocked();
+ put_online_cpus();
+}
+
void __init enable_errata_workarounds(void)
{
enable_cpu_capabilities(arm64_errata);
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index abda8e8..113c06d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -956,6 +956,7 @@ static void __init setup_elf_hwcaps(const struct arm64_cpu_capabilities *hwcaps)
cap_set_elf_hwcap(hwcaps);
}
+/* Should be called with CPU hotplug lock held */
void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
const char *info)
{
@@ -1075,14 +1076,16 @@ void check_local_cpu_capabilities(void)
* advertised capabilities.
*/
if (!sys_caps_initialised)
- update_cpu_errata_workarounds();
+ update_cpu_errata_workarounds_cpuslocked();
else
verify_local_cpu_capabilities();
}
static void __init setup_feature_capabilities(void)
{
+ get_online_cpus();
update_cpu_capabilities(arm64_features, "detected feature:");
+ put_online_cpus();
enable_cpu_capabilities(arm64_features);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [tip:smp/hotplug] arm64/cpufeature: Use static_branch_enable_cpuslocked()
2017-04-28 11:01 ` [PATCHv3 2/2] arm64: cpufeature: use static_branch_enable_cpuslocked() Mark Rutland
@ 2017-04-30 9:45 ` tip-bot for Mark Rutland
0 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Mark Rutland @ 2017-04-30 9:45 UTC (permalink / raw)
To: linux-tip-commits
Cc: mark.rutland, mingo, linux-kernel, hpa, suzuki.poulose, tglx,
bigeasy, catalin.marinas, will.deacon
Commit-ID: d54bb72551b999ddf7ceef87ed58fb4cb2ee6dfc
Gitweb: http://git.kernel.org/tip/d54bb72551b999ddf7ceef87ed58fb4cb2ee6dfc
Author: Mark Rutland <mark.rutland@arm.com>
AuthorDate: Fri, 28 Apr 2017 12:01:06 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 30 Apr 2017 11:19:01 +0200
arm64/cpufeature: Use static_branch_enable_cpuslocked()
Recently, the hotplug locking was conveted to use a percpu rwsem. Unlike
the existing {get,put}_online_cpus() logic, this can't nest.
Unfortunately, in arm64's secondary boot path we can end up nesting via
static_branch_enable() in cpus_set_cap() when we detect an erratum.
This leads to a stream of messages as below, where the secondary
attempts to schedule before it has been fully onlined. As the CPU
orchestrating the onlining holds the rswem, this hangs the system.
[ 0.250334] BUG: scheduling while atomic: swapper/1/0/0x00000002
[ 0.250337] Modules linked in:
[ 0.250346] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.11.0-rc7-next-20170424 #2
[ 0.250349] Hardware name: ARM Juno development board (r1) (DT)
[ 0.250353] Call trace:
[ 0.250365] [<ffff000008088510>] dump_backtrace+0x0/0x238
[ 0.250371] [<ffff00000808880c>] show_stack+0x14/0x20
[ 0.250377] [<ffff00000839d854>] dump_stack+0x9c/0xc0
[ 0.250384] [<ffff0000080e3540>] __schedule_bug+0x50/0x70
[ 0.250391] [<ffff000008932ecc>] __schedule+0x52c/0x5a8
[ 0.250395] [<ffff000008932f80>] schedule+0x38/0xa0
[ 0.250400] [<ffff000008935e8c>] rwsem_down_read_failed+0xc4/0x108
[ 0.250407] [<ffff0000080fe8e0>] __percpu_down_read+0x100/0x118
[ 0.250414] [<ffff0000080c0b60>] get_online_cpus+0x70/0x78
[ 0.250420] [<ffff0000081749e8>] static_key_enable+0x28/0x48
[ 0.250425] [<ffff00000808de90>] update_cpu_capabilities+0x78/0xf8
[ 0.250430] [<ffff00000808d14c>] update_cpu_errata_workarounds+0x1c/0x28
[ 0.250435] [<ffff00000808e004>] check_local_cpu_capabilities+0xf4/0x128
[ 0.250440] [<ffff00000808e894>] secondary_start_kernel+0x8c/0x118
[ 0.250444] [<000000008093d1b4>] 0x8093d1b4
We call cpus_set_cap() from update_cpu_capabilities(), which is called
from the secondary boot path (where the CPU orchestrating the onlining
holds the hotplug rwsem), and in the primary boot path, where this is
not held.
This patch makes cpus_set_cap() use static_branch_enable_cpuslocked(),
and updates all the callers of update_cpu_capabilities() consistent with
the change.
[Mark: minor fixups]
Reported-by: Catalin Marinas <catalin.marinas@arm.com>
Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: peterz@infradead.org
Cc: rostedt@goodmis.org
Cc: jbaron@akamai.com
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1493377266-2205-3-git-send-email-mark.rutland@arm.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
arch/arm64/include/asm/cpufeature.h | 3 ++-
arch/arm64/kernel/cpu_errata.c | 9 ++++++++-
arch/arm64/kernel/cpufeature.c | 5 ++++-
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index f31c48d..a8d1b5b 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -145,7 +145,7 @@ static inline void cpus_set_cap(unsigned int num)
num, ARM64_NCAPS);
} else {
__set_bit(num, cpu_hwcaps);
- static_branch_enable(&cpu_hwcap_keys[num]);
+ static_branch_enable_cpuslocked(&cpu_hwcap_keys[num]);
}
}
@@ -223,6 +223,7 @@ void enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps);
void check_local_cpu_capabilities(void);
void update_cpu_errata_workarounds(void);
+void update_cpu_errata_workarounds_cpuslocked(void);
void __init enable_errata_workarounds(void);
void verify_local_cpu_errata_workarounds(void);
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index f6cc67e..e650cf8 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -175,11 +175,18 @@ void verify_local_cpu_errata_workarounds(void)
}
}
-void update_cpu_errata_workarounds(void)
+void update_cpu_errata_workarounds_cpuslocked(void)
{
update_cpu_capabilities(arm64_errata, "enabling workaround for");
}
+void update_cpu_errata_workarounds(void)
+{
+ get_online_cpus();
+ update_cpu_errata_workarounds_cpuslocked();
+ put_online_cpus();
+}
+
void __init enable_errata_workarounds(void)
{
enable_cpu_capabilities(arm64_errata);
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index abda8e8..113c06d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -956,6 +956,7 @@ static void __init setup_elf_hwcaps(const struct arm64_cpu_capabilities *hwcaps)
cap_set_elf_hwcap(hwcaps);
}
+/* Should be called with CPU hotplug lock held */
void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
const char *info)
{
@@ -1075,14 +1076,16 @@ void check_local_cpu_capabilities(void)
* advertised capabilities.
*/
if (!sys_caps_initialised)
- update_cpu_errata_workarounds();
+ update_cpu_errata_workarounds_cpuslocked();
else
verify_local_cpu_capabilities();
}
static void __init setup_feature_capabilities(void)
{
+ get_online_cpus();
update_cpu_capabilities(arm64_features, "detected feature:");
+ put_online_cpus();
enable_cpu_capabilities(arm64_features);
}
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout
2017-04-28 11:01 [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout Mark Rutland
2017-04-28 11:01 ` [PATCHv3 1/2] jump_label: Provide static_key_[enable|/slow_inc]_cpuslocked() Mark Rutland
2017-04-28 11:01 ` [PATCHv3 2/2] arm64: cpufeature: use static_branch_enable_cpuslocked() Mark Rutland
@ 2017-05-10 18:10 ` Mark Rutland
2017-05-10 20:09 ` Thomas Gleixner
2 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2017-05-10 18:10 UTC (permalink / raw)
To: tglx
Cc: linux-arm-kernel, linux-kernel, will.deacon, catalin.marinas,
bigeasy, jbaron, peterz, rostedt, suzuki.poulose
On Fri, Apr 28, 2017 at 12:01:04PM +0100, Mark Rutland wrote:
> Hi,
>
> These patches fix a boot issue seen on some arm64 platforms as a result of the
> hotplug rwsem rework.
>
> Thomas, would you be able to take these into the tip smp/hotplug branch?
>
> Will has acked the arm64 part, and is happy for this to go via tip [1].
>
> I've tested this atop of the tip smp/hotplug branch, and with the arm64
> for-next/core branch merged in, which git handles automatically. In both cases,
> it builds cleanly and boots fine on Juno R1.
As a heads-up, with next-20170510 I'm seeing a warning here that I don't
recall seeing when I tested the above, and I'm not sure how to fix it:
[ 0.180998] CPU features: enabling workaround for ARM erratum 832075
[ 0.181032] ------------[ cut here ]------------
[ 0.181074] WARNING: CPU: 1 PID: 0 at kernel/cpu.c:234 lockdep_assert_hotplug_held+0x78/0x98
[ 0.181084] Modules linked in:
[ 0.181118] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.11.0-next-20170510 #1
[ 0.181132] Hardware name: ARM Juno development board (r1) (DT)
[ 0.181149] task: ffff80093590af00 task.stack: ffff800935938000
[ 0.181172] PC is at lockdep_assert_hotplug_held+0x78/0x98
[ 0.181194] LR is at lockdep_assert_hotplug_held+0x78/0x98
[ 0.181213] pc : [<ffff200008155218>] lr : [<ffff200008155218>] pstate: 800001c5
[ 0.181225] sp : ffff80093593bd80
[ 0.181239] x29: ffff80093593bd80 x28: 0000000000000006
[ 0.181274] x27: 0000000000000000 x26: 0000000000000008
[ 0.181309] x25: 1fffe40001aa0d96 x24: 0000000000000000
[ 0.181344] x23: ffff20000abf74a0 x22: 0000000000000000
[ 0.181379] x21: 1ffff00126b277be x20: ffff20000b054000
[ 0.181414] x19: ffff20000ba69000 x18: 0000000000000000
[ 0.181448] x17: 0000000000000000 x16: 0000000000000002
[ 0.181482] x15: 0000000000000000 x14: ffff20000d71af80
[ 0.181517] x13: 1ffff00126b216dd x12: ffff80093590b6c0
[ 0.181552] x11: 1ffff00126b216d8 x10: ffff80093590b6c8
[ 0.181587] x9 : ffff20000ae3b000 x8 : ffff20000d71a000
[ 0.181622] x7 : 1ffff00126b216dc x6 : 0000000041b58ab3
[ 0.181656] x5 : 0000000041b58ab3 x4 : 0000000000000000
[ 0.181690] x3 : 1ffff00126b216d8 x2 : 0000000000000000
[ 0.181724] x1 : 0000000000000001 x0 : 0000000000000000
[ 0.181801] ---[ end trace eee4d9a3d314f895 ]---
[ 0.181812] Call trace:
[ 0.182133] [<ffff200008155218>] lockdep_assert_hotplug_held+0x78/0x98
[ 0.182161] [<ffff20000840a36c>] __static_key_slow_inc+0x174/0x2e0
[ 0.182188] [<ffff20000840a654>] static_key_enable_cpuslocked+0x64/0xb0
[ 0.182215] [<ffff2000080a1120>] update_cpu_capabilities+0x178/0x2d8
[ 0.182243] [<ffff20000809e72c>] update_cpu_errata_workarounds_cpuslocked+0x1c/0x28
[ 0.182270] [<ffff2000080a1420>] check_local_cpu_capabilities+0x1a0/0x248
[ 0.182295] [<ffff2000080a2d18>] secondary_start_kernel+0x1e8/0x478
[ 0.182317] [<000000008219a1b4>] 0x8219a1b4
[ 0.182337] CPU features: enabling workaround for ARM erratum 834220
[ 0.182362] ------------[ cut here ]------------
The problem is that the secondary CPU doesn't hold the rwsem when it
calls __static_key_slow_inc() in its boot path. It cannot take the
rwsem, since the primaary CPU holds this for the duration of onlining
the secondary CPU.
So far, I haven't figured out how we can avoid this. Any ideas?
Thanks,
Mark.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout
2017-05-10 18:10 ` [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout Mark Rutland
@ 2017-05-10 20:09 ` Thomas Gleixner
2017-05-11 8:30 ` Thomas Gleixner
0 siblings, 1 reply; 11+ messages in thread
From: Thomas Gleixner @ 2017-05-10 20:09 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arm-kernel, linux-kernel, will.deacon, catalin.marinas,
bigeasy, jbaron, peterz, rostedt, suzuki.poulose
On Wed, 10 May 2017, Mark Rutland wrote:
> As a heads-up, with next-20170510 I'm seeing a warning here that I don't
> recall seeing when I tested the above, and I'm not sure how to fix it:
>
> [ 0.180998] CPU features: enabling workaround for ARM erratum 832075
> [ 0.181032] ------------[ cut here ]------------
> [ 0.181074] WARNING: CPU: 1 PID: 0 at kernel/cpu.c:234 lockdep_assert_hotplug_held+0x78/0x98
> [ 0.181084] Modules linked in:
> [ 0.181118] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.11.0-next-20170510 #1
> [ 0.181132] Hardware name: ARM Juno development board (r1) (DT)
> [ 0.181149] task: ffff80093590af00 task.stack: ffff800935938000
> [ 0.181172] PC is at lockdep_assert_hotplug_held+0x78/0x98
> [ 0.181194] LR is at lockdep_assert_hotplug_held+0x78/0x98
> [ 0.181213] pc : [<ffff200008155218>] lr : [<ffff200008155218>] pstate: 800001c5
> [ 0.181225] sp : ffff80093593bd80
> [ 0.181239] x29: ffff80093593bd80 x28: 0000000000000006
> [ 0.181274] x27: 0000000000000000 x26: 0000000000000008
> [ 0.181309] x25: 1fffe40001aa0d96 x24: 0000000000000000
> [ 0.181344] x23: ffff20000abf74a0 x22: 0000000000000000
> [ 0.181379] x21: 1ffff00126b277be x20: ffff20000b054000
> [ 0.181414] x19: ffff20000ba69000 x18: 0000000000000000
> [ 0.181448] x17: 0000000000000000 x16: 0000000000000002
> [ 0.181482] x15: 0000000000000000 x14: ffff20000d71af80
> [ 0.181517] x13: 1ffff00126b216dd x12: ffff80093590b6c0
> [ 0.181552] x11: 1ffff00126b216d8 x10: ffff80093590b6c8
> [ 0.181587] x9 : ffff20000ae3b000 x8 : ffff20000d71a000
> [ 0.181622] x7 : 1ffff00126b216dc x6 : 0000000041b58ab3
> [ 0.181656] x5 : 0000000041b58ab3 x4 : 0000000000000000
> [ 0.181690] x3 : 1ffff00126b216d8 x2 : 0000000000000000
> [ 0.181724] x1 : 0000000000000001 x0 : 0000000000000000
> [ 0.181801] ---[ end trace eee4d9a3d314f895 ]---
> [ 0.181812] Call trace:
> [ 0.182133] [<ffff200008155218>] lockdep_assert_hotplug_held+0x78/0x98
> [ 0.182161] [<ffff20000840a36c>] __static_key_slow_inc+0x174/0x2e0
> [ 0.182188] [<ffff20000840a654>] static_key_enable_cpuslocked+0x64/0xb0
> [ 0.182215] [<ffff2000080a1120>] update_cpu_capabilities+0x178/0x2d8
> [ 0.182243] [<ffff20000809e72c>] update_cpu_errata_workarounds_cpuslocked+0x1c/0x28
> [ 0.182270] [<ffff2000080a1420>] check_local_cpu_capabilities+0x1a0/0x248
> [ 0.182295] [<ffff2000080a2d18>] secondary_start_kernel+0x1e8/0x478
> [ 0.182317] [<000000008219a1b4>] 0x8219a1b4
> [ 0.182337] CPU features: enabling workaround for ARM erratum 834220
> [ 0.182362] ------------[ cut here ]------------
>
> The problem is that the secondary CPU doesn't hold the rwsem when it
> calls __static_key_slow_inc() in its boot path. It cannot take the
> rwsem, since the primaary CPU holds this for the duration of onlining
> the secondary CPU.
>
> So far, I haven't figured out how we can avoid this. Any ideas?
Not immediately, but I put it on the list of crap to look at....
Thanks,
tglx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout
2017-05-10 20:09 ` Thomas Gleixner
@ 2017-05-11 8:30 ` Thomas Gleixner
2017-05-11 9:37 ` Mark Rutland
0 siblings, 1 reply; 11+ messages in thread
From: Thomas Gleixner @ 2017-05-11 8:30 UTC (permalink / raw)
To: Mark Rutland
Cc: LAK, LKML, will.deacon, catalin.marinas, Sebastian Sewior,
jbaron, Peter Zijlstra, Steven Rostedt, suzuki.poulose
On Wed, 10 May 2017, Thomas Gleixner wrote:
> On Wed, 10 May 2017, Mark Rutland wrote:
> > [ 0.182133] [<ffff200008155218>] lockdep_assert_hotplug_held+0x78/0x98
> > [ 0.182161] [<ffff20000840a36c>] __static_key_slow_inc+0x174/0x2e0
> > [ 0.182188] [<ffff20000840a654>] static_key_enable_cpuslocked+0x64/0xb0
> > [ 0.182215] [<ffff2000080a1120>] update_cpu_capabilities+0x178/0x2d8
> > [ 0.182243] [<ffff20000809e72c>] update_cpu_errata_workarounds_cpuslocked+0x1c/0x28
> > [ 0.182270] [<ffff2000080a1420>] check_local_cpu_capabilities+0x1a0/0x248
> > [ 0.182295] [<ffff2000080a2d18>] secondary_start_kernel+0x1e8/0x478
> > [ 0.182317] [<000000008219a1b4>] 0x8219a1b4
> > [ 0.182337] CPU features: enabling workaround for ARM erratum 834220
> > [ 0.182362] ------------[ cut here ]------------
> >
> > The problem is that the secondary CPU doesn't hold the rwsem when it
> > calls __static_key_slow_inc() in its boot path. It cannot take the
> > rwsem, since the primaary CPU holds this for the duration of onlining
> > the secondary CPU.
Looking deeper into that:
secondary_start_kernel()
check_local_cpu_capabilities()
update_cpu_errata_workarounds()
update_cpu_capabilities()
static_key_enable()
__static_key_slow_inc()
jump_label_lock()
mutex_lock(&jump_label_mutex);
How is that supposed to work?
That call path is the low level CPU bringup, running in the context of the
idle task of that CPU with interrupts and preemption disabled. Taking a
mutex in that context, even if in that case the mutex is uncontended, is a
NONO.
Thanks,
tglx
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout
2017-05-11 8:30 ` Thomas Gleixner
@ 2017-05-11 9:37 ` Mark Rutland
2017-05-11 10:01 ` Thomas Gleixner
0 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2017-05-11 9:37 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LAK, LKML, will.deacon, catalin.marinas, Sebastian Sewior,
jbaron, Peter Zijlstra, Steven Rostedt, suzuki.poulose
On Thu, May 11, 2017 at 10:30:39AM +0200, Thomas Gleixner wrote:
> On Wed, 10 May 2017, Thomas Gleixner wrote:
> > On Wed, 10 May 2017, Mark Rutland wrote:
> > > [ 0.182133] [<ffff200008155218>] lockdep_assert_hotplug_held+0x78/0x98
> > > [ 0.182161] [<ffff20000840a36c>] __static_key_slow_inc+0x174/0x2e0
> > > [ 0.182188] [<ffff20000840a654>] static_key_enable_cpuslocked+0x64/0xb0
> > > [ 0.182215] [<ffff2000080a1120>] update_cpu_capabilities+0x178/0x2d8
> > > [ 0.182243] [<ffff20000809e72c>] update_cpu_errata_workarounds_cpuslocked+0x1c/0x28
> > > [ 0.182270] [<ffff2000080a1420>] check_local_cpu_capabilities+0x1a0/0x248
> > > [ 0.182295] [<ffff2000080a2d18>] secondary_start_kernel+0x1e8/0x478
> > > [ 0.182317] [<000000008219a1b4>] 0x8219a1b4
> > > [ 0.182337] CPU features: enabling workaround for ARM erratum 834220
> > > [ 0.182362] ------------[ cut here ]------------
> > >
> > > The problem is that the secondary CPU doesn't hold the rwsem when it
> > > calls __static_key_slow_inc() in its boot path. It cannot take the
> > > rwsem, since the primaary CPU holds this for the duration of onlining
> > > the secondary CPU.
>
> Looking deeper into that:
>
> secondary_start_kernel()
> check_local_cpu_capabilities()
> update_cpu_errata_workarounds()
> update_cpu_capabilities()
> static_key_enable()
> __static_key_slow_inc()
> jump_label_lock()
> mutex_lock(&jump_label_mutex);
>
> How is that supposed to work?
>
> That call path is the low level CPU bringup, running in the context of the
> idle task of that CPU with interrupts and preemption disabled. Taking a
> mutex in that context, even if in that case the mutex is uncontended, is a
> NONO.
Urgh; good point. Thanks for taking a look.
I think I can solve both issues by deferring poking the keys, so I'll
give that a go.
As an aside, do we have anything that should detect the broken mutex
usage? I've been testing kernels with LOCKDEP, PROVE_LOCKING,
DEBUG_ATOMIC_SLEEP, and friends, and nothing has complained so far.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout
2017-05-11 9:37 ` Mark Rutland
@ 2017-05-11 10:01 ` Thomas Gleixner
2017-05-11 12:54 ` Mark Rutland
0 siblings, 1 reply; 11+ messages in thread
From: Thomas Gleixner @ 2017-05-11 10:01 UTC (permalink / raw)
To: Mark Rutland
Cc: LAK, LKML, will.deacon, catalin.marinas, Sebastian Sewior,
jbaron, Peter Zijlstra, Steven Rostedt, suzuki.poulose
On Thu, 11 May 2017, Mark Rutland wrote:
> On Thu, May 11, 2017 at 10:30:39AM +0200, Thomas Gleixner wrote:
> > On Wed, 10 May 2017, Thomas Gleixner wrote:
> > > On Wed, 10 May 2017, Mark Rutland wrote:
> > > > [ 0.182133] [<ffff200008155218>] lockdep_assert_hotplug_held+0x78/0x98
> > > > [ 0.182161] [<ffff20000840a36c>] __static_key_slow_inc+0x174/0x2e0
> > > > [ 0.182188] [<ffff20000840a654>] static_key_enable_cpuslocked+0x64/0xb0
> > > > [ 0.182215] [<ffff2000080a1120>] update_cpu_capabilities+0x178/0x2d8
> > > > [ 0.182243] [<ffff20000809e72c>] update_cpu_errata_workarounds_cpuslocked+0x1c/0x28
> > > > [ 0.182270] [<ffff2000080a1420>] check_local_cpu_capabilities+0x1a0/0x248
> > > > [ 0.182295] [<ffff2000080a2d18>] secondary_start_kernel+0x1e8/0x478
> > > > [ 0.182317] [<000000008219a1b4>] 0x8219a1b4
> > > > [ 0.182337] CPU features: enabling workaround for ARM erratum 834220
> > > > [ 0.182362] ------------[ cut here ]------------
> > > >
> > > > The problem is that the secondary CPU doesn't hold the rwsem when it
> > > > calls __static_key_slow_inc() in its boot path. It cannot take the
> > > > rwsem, since the primaary CPU holds this for the duration of onlining
> > > > the secondary CPU.
> >
> > Looking deeper into that:
> >
> > secondary_start_kernel()
> > check_local_cpu_capabilities()
> > update_cpu_errata_workarounds()
> > update_cpu_capabilities()
> > static_key_enable()
> > __static_key_slow_inc()
> > jump_label_lock()
> > mutex_lock(&jump_label_mutex);
> >
> > How is that supposed to work?
> >
> > That call path is the low level CPU bringup, running in the context of the
> > idle task of that CPU with interrupts and preemption disabled. Taking a
> > mutex in that context, even if in that case the mutex is uncontended, is a
> > NONO.
>
> Urgh; good point. Thanks for taking a look.
>
> I think I can solve both issues by deferring poking the keys, so I'll
> give that a go.
>
> As an aside, do we have anything that should detect the broken mutex
> usage? I've been testing kernels with LOCKDEP, PROVE_LOCKING,
> DEBUG_ATOMIC_SLEEP, and friends, and nothing has complained so far.
Peter and myself were wondering about that already. No idea why that
doesn't yell at you.
Thanks,
tglx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout
2017-05-11 10:01 ` Thomas Gleixner
@ 2017-05-11 12:54 ` Mark Rutland
0 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2017-05-11 12:54 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LAK, LKML, will.deacon, catalin.marinas, Sebastian Sewior,
jbaron, Peter Zijlstra, Steven Rostedt, suzuki.poulose
On Thu, May 11, 2017 at 12:01:21PM +0200, Thomas Gleixner wrote:
> On Thu, 11 May 2017, Mark Rutland wrote:
> > On Thu, May 11, 2017 at 10:30:39AM +0200, Thomas Gleixner wrote:
> > > On Wed, 10 May 2017, Thomas Gleixner wrote:
> > > secondary_start_kernel()
> > > check_local_cpu_capabilities()
> > > update_cpu_errata_workarounds()
> > > update_cpu_capabilities()
> > > static_key_enable()
> > > __static_key_slow_inc()
> > > jump_label_lock()
> > > mutex_lock(&jump_label_mutex);
> > >
> > > How is that supposed to work?
> > >
> > > That call path is the low level CPU bringup, running in the context of the
> > > idle task of that CPU with interrupts and preemption disabled. Taking a
> > > mutex in that context, even if in that case the mutex is uncontended, is a
> > > NONO.
> > As an aside, do we have anything that should detect the broken mutex
> > usage? I've been testing kernels with LOCKDEP, PROVE_LOCKING,
> > DEBUG_ATOMIC_SLEEP, and friends, and nothing has complained so far.
>
> Peter and myself were wondering about that already. No idea why that
> doesn't yell at you.
AFAICT, this happens early enough that system_state is SYSTEM_BOOTING.
In ___might_sleep(), we see this and bail out without a splat.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 11+ messages in thread