[PATCH v2 0/4] Working around CPU hotplug and static keys locking

[PATCH v2 0/4] Working around CPU hotplug and static keys locking

[Marc Zyngier]

Since f2545b2d4ce1 ("jump_label: Reorder hotplug lock and
jump_label_lock"), it has become impossible to switch a static key
from a CPU hotplug notifier:

- On the primary CPU, cpu_hotplug_lock is taken by
  __cpuhp_setup_state(), and then again by static_key_slow_inc(). The
  lock being taken as a reader, so it is OK so far.

- On a secondary CPU, _cpu_up takes the lock *as a writer* on the boot
  CPU, and the secondary tries to switch the static key, taking the
  lock as well (as a reader). In that case, we're toasted.

I couldn't find an elegant solution to this, so this series works
around the issue in the most disgusting way, adding a _nolock version
of the static key API to be used in CPU hotplug situations.

The last patch uses this API to work around the issue that Leo
reported, where the static key flipped on a secondary CPU brings the
box down in flames.

Marc Zyngier (4):
  jump_label: Move cpu hotplug locking
  jump_label: Split out code under the hotplug lock
  jump_label: Provide hotplug context variants
  clocksource/arm_arch_timer: Use static_branch_enable_cpuslocked()

 Documentation/static-keys.txt        | 15 ++++++++++++
 drivers/clocksource/arm_arch_timer.c |  6 ++++-
 include/linux/jump_label.h           | 11 +++++++--
 kernel/jump_label.c                  | 44 +++++++++++++++++++++++++++++++-----
 4 files changed, 67 insertions(+), 9 deletions(-)

-- 
2.11.0

[PATCH v2 0/4] Working around CPU hotplug and static keys locking

[Marc Zyngier]

Since f2545b2d4ce1 ("jump_label: Reorder hotplug lock and
jump_label_lock"), it has become impossible to switch a static key
from a CPU hotplug notifier:

- On the primary CPU, cpu_hotplug_lock is taken by
  __cpuhp_setup_state(), and then again by static_key_slow_inc(). The
  lock being taken as a reader, so it is OK so far.

- On a secondary CPU, _cpu_up takes the lock *as a writer* on the boot
  CPU, and the secondary tries to switch the static key, taking the
  lock as well (as a reader). In that case, we're toasted.

I couldn't find an elegant solution to this, so this series works
around the issue in the most disgusting way, adding a _nolock version
of the static key API to be used in CPU hotplug situations.

The last patch uses this API to work around the issue that Leo
reported, where the static key flipped on a secondary CPU brings the
box down in flames.

Marc Zyngier (4):
  jump_label: Move cpu hotplug locking
  jump_label: Split out code under the hotplug lock
  jump_label: Provide hotplug context variants
  clocksource/arm_arch_timer: Use static_branch_enable_cpuslocked()

 Documentation/static-keys.txt        | 15 ++++++++++++
 drivers/clocksource/arm_arch_timer.c |  6 ++++-
 include/linux/jump_label.h           | 11 +++++++--
 kernel/jump_label.c                  | 44 +++++++++++++++++++++++++++++++-----
 4 files changed, 67 insertions(+), 9 deletions(-)

-- 
2.11.0

[PATCH v2 1/4] jump_label: Move cpu hotplug locking

[Marc Zyngier]

As we're about to rework the locking, let's move the taking and
release of the CPU hotplug lock to locations that will make its
reworking completely obvious.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 kernel/jump_label.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d11c506a6ac3..f11b10091100 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -105,6 +105,7 @@ void static_key_slow_inc(struct static_key *key)
 {
 	int v, v1;
 
+	cpus_read_lock();
 	STATIC_KEY_CHECK_USE();
 
 	/*
@@ -121,11 +122,12 @@ void static_key_slow_inc(struct static_key *key)
 	 */
 	for (v = atomic_read(&key->enabled); v > 0; v = v1) {
 		v1 = atomic_cmpxchg(&key->enabled, v, v + 1);
-		if (likely(v1 == v))
+		if (likely(v1 == v)) {
+			cpus_read_unlock();
 			return;
+		}
 	}
 
-	cpus_read_lock();
 	jump_label_lock();
 	if (atomic_read(&key->enabled) == 0) {
 		atomic_set(&key->enabled, -1);
-- 
2.11.0

[PATCH v2 1/4] jump_label: Move cpu hotplug locking

[Marc Zyngier]

As we're about to rework the locking, let's move the taking and
release of the CPU hotplug lock to locations that will make its
reworking completely obvious.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 kernel/jump_label.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d11c506a6ac3..f11b10091100 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -105,6 +105,7 @@ void static_key_slow_inc(struct static_key *key)
 {
 	int v, v1;
 
+	cpus_read_lock();
 	STATIC_KEY_CHECK_USE();
 
 	/*
@@ -121,11 +122,12 @@ void static_key_slow_inc(struct static_key *key)
 	 */
 	for (v = atomic_read(&key->enabled); v > 0; v = v1) {
 		v1 = atomic_cmpxchg(&key->enabled, v, v + 1);
-		if (likely(v1 == v))
+		if (likely(v1 == v)) {
+			cpus_read_unlock();
 			return;
+		}
 	}
 
-	cpus_read_lock();
 	jump_label_lock();
 	if (atomic_read(&key->enabled) == 0) {
 		atomic_set(&key->enabled, -1);
-- 
2.11.0

[PATCH v2 2/4] jump_label: Split out code under the hotplug lock

[Marc Zyngier]

In order to later introduce an "already locked" version of some
of the static key funcions, let's split the code into the core stuff
(the *_cpuslocked functions) and the usual helpers, which now
take/release the hotplug lock and call into the _cpuslocked
versions.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 kernel/jump_label.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index f11b10091100..0ab8b35daa54 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -101,11 +101,10 @@ void static_key_disable(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_disable);
 
-void static_key_slow_inc(struct static_key *key)
+static void static_key_slow_inc_cpuslocked(struct static_key *key)
 {
 	int v, v1;
 
-	cpus_read_lock();
 	STATIC_KEY_CHECK_USE();
 
 	/*
@@ -122,10 +121,8 @@ void static_key_slow_inc(struct static_key *key)
 	 */
 	for (v = atomic_read(&key->enabled); v > 0; v = v1) {
 		v1 = atomic_cmpxchg(&key->enabled, v, v + 1);
-		if (likely(v1 == v)) {
-			cpus_read_unlock();
+		if (likely(v1 == v))
 			return;
-		}
 	}
 
 	jump_label_lock();
@@ -137,14 +134,20 @@ void static_key_slow_inc(struct static_key *key)
 		atomic_inc(&key->enabled);
 	}
 	jump_label_unlock();
+}
+
+void static_key_slow_inc(struct static_key *key)
+{
+	cpus_read_lock();
+	static_key_slow_inc_cpuslocked(key);
 	cpus_read_unlock();
 }
 EXPORT_SYMBOL_GPL(static_key_slow_inc);
 
-static void __static_key_slow_dec(struct static_key *key,
-		unsigned long rate_limit, struct delayed_work *work)
+static void static_key_slow_dec_cpuslocked(struct static_key *key,
+					   unsigned long rate_limit,
+					   struct delayed_work *work)
 {
-	cpus_read_lock();
 	/*
 	 * The negative count check is valid even when a negative
 	 * key->enabled is in use by static_key_slow_inc(); a
@@ -155,7 +158,6 @@ static void __static_key_slow_dec(struct static_key *key,
 	if (!atomic_dec_and_mutex_lock(&key->enabled, &jump_label_mutex)) {
 		WARN(atomic_read(&key->enabled) < 0,
 		     "jump label: negative count!\n");
-		cpus_read_unlock();
 		return;
 	}
 
@@ -166,6 +168,14 @@ static void __static_key_slow_dec(struct static_key *key,
 		jump_label_update(key);
 	}
 	jump_label_unlock();
+}
+
+static void __static_key_slow_dec(struct static_key *key,
+				  unsigned long rate_limit,
+				  struct delayed_work *work)
+{
+	cpus_read_lock();
+	static_key_slow_dec_cpuslocked(key, rate_limit, work);
 	cpus_read_unlock();
 }
 
-- 
2.11.0

[PATCH v2 2/4] jump_label: Split out code under the hotplug lock

[Marc Zyngier]

In order to later introduce an "already locked" version of some
of the static key funcions, let's split the code into the core stuff
(the *_cpuslocked functions) and the usual helpers, which now
take/release the hotplug lock and call into the _cpuslocked
versions.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 kernel/jump_label.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index f11b10091100..0ab8b35daa54 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -101,11 +101,10 @@ void static_key_disable(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_disable);
 
-void static_key_slow_inc(struct static_key *key)
+static void static_key_slow_inc_cpuslocked(struct static_key *key)
 {
 	int v, v1;
 
-	cpus_read_lock();
 	STATIC_KEY_CHECK_USE();
 
 	/*
@@ -122,10 +121,8 @@ void static_key_slow_inc(struct static_key *key)
 	 */
 	for (v = atomic_read(&key->enabled); v > 0; v = v1) {
 		v1 = atomic_cmpxchg(&key->enabled, v, v + 1);
-		if (likely(v1 == v)) {
-			cpus_read_unlock();
+		if (likely(v1 == v))
 			return;
-		}
 	}
 
 	jump_label_lock();
@@ -137,14 +134,20 @@ void static_key_slow_inc(struct static_key *key)
 		atomic_inc(&key->enabled);
 	}
 	jump_label_unlock();
+}
+
+void static_key_slow_inc(struct static_key *key)
+{
+	cpus_read_lock();
+	static_key_slow_inc_cpuslocked(key);
 	cpus_read_unlock();
 }
 EXPORT_SYMBOL_GPL(static_key_slow_inc);
 
-static void __static_key_slow_dec(struct static_key *key,
-		unsigned long rate_limit, struct delayed_work *work)
+static void static_key_slow_dec_cpuslocked(struct static_key *key,
+					   unsigned long rate_limit,
+					   struct delayed_work *work)
 {
-	cpus_read_lock();
 	/*
 	 * The negative count check is valid even when a negative
 	 * key->enabled is in use by static_key_slow_inc(); a
@@ -155,7 +158,6 @@ static void __static_key_slow_dec(struct static_key *key,
 	if (!atomic_dec_and_mutex_lock(&key->enabled, &jump_label_mutex)) {
 		WARN(atomic_read(&key->enabled) < 0,
 		     "jump label: negative count!\n");
-		cpus_read_unlock();
 		return;
 	}
 
@@ -166,6 +168,14 @@ static void __static_key_slow_dec(struct static_key *key,
 		jump_label_update(key);
 	}
 	jump_label_unlock();
+}
+
+static void __static_key_slow_dec(struct static_key *key,
+				  unsigned long rate_limit,
+				  struct delayed_work *work)
+{
+	cpus_read_lock();
+	static_key_slow_dec_cpuslocked(key, rate_limit, work);
 	cpus_read_unlock();
 }
 
-- 
2.11.0

[PATCH v2 3/4] jump_label: Provide hotplug context variants

[Marc Zyngier]

As using the normal static key API under the hotplug lock is
pretty much impossible, let's provide a variant of some of them
that require the hotplug lock to have already been taken.

These function are only meant to be used in CPU hotplug callbacks.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 Documentation/static-keys.txt | 15 +++++++++++++++
 include/linux/jump_label.h    | 11 +++++++++--
 kernel/jump_label.c           | 20 ++++++++++++++++++++
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/static-keys.txt b/Documentation/static-keys.txt
index b83dfa1c0602..6d0a181e74d2 100644
--- a/Documentation/static-keys.txt
+++ b/Documentation/static-keys.txt
@@ -149,6 +149,21 @@ static_branch_inc(), will change the branch back to true. Likewise, if the
 key is initialized false, a 'static_branch_inc()', will change the branch to
 true. And then a 'static_branch_dec()', will again make the branch false.
 
+Note that switching branches results in some locks being taken,
+particularly the CPU hotplug lock (in order to avoid races against
+CPUs being brought in the kernel whilst the kernel is getting
+patched). Calling the static key API from within a hotplug notifier is
+thus a sure deadlock recipe. In order to still allow use of the
+functionnality, the following functions are provided:
+
+	static_key_enable_cpuslocked()
+	static_key_disable_cpuslocked()
+	static_branch_enable_cpuslocked()
+	static_branch_disable_cpuslocked()
+
+These functions are *not* general purpose, and must only be used when
+you really know that you're in the above context, and no other.
+
 Where an array of keys is required, it can be defined as::
 
 	DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 2afd74b9d844..c5e5d2ffce58 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -163,6 +163,8 @@ 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_disable(struct static_key *key);
+extern void static_key_enable_cpuslocked(struct static_key *key);
+extern void static_key_disable_cpuslocked(struct static_key *key);
 
 /*
  * We should be using ATOMIC_INIT() for initializing .enabled, but
@@ -252,6 +254,9 @@ static inline void static_key_disable(struct static_key *key)
 		static_key_slow_dec(key);
 }
 
+#define static_key_enable_cpuslocked(k)		static_key_enable((k))
+#define static_key_disable_cpuslocked(k)	static_key_disable((k))
+
 #define STATIC_KEY_INIT_TRUE	{ .enabled = ATOMIC_INIT(1) }
 #define STATIC_KEY_INIT_FALSE	{ .enabled = ATOMIC_INIT(0) }
 
@@ -413,8 +418,10 @@ extern bool ____wrong_branch_error(void);
  * Normal usage; boolean enable/disable.
  */
 
-#define static_branch_enable(x)		static_key_enable(&(x)->key)
-#define static_branch_disable(x)	static_key_disable(&(x)->key)
+#define static_branch_enable(x)			static_key_enable(&(x)->key)
+#define static_branch_disable(x)		static_key_disable(&(x)->key)
+#define static_branch_enable_cpuslocked(x)	static_key_enable_cpuslocked(&(x)->key)
+#define static_branch_disable_cpuslocked(x)	static_key_disable_cpuslocked(&(x)->key)
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 0ab8b35daa54..4e7fa067ca17 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -170,6 +170,26 @@ static void static_key_slow_dec_cpuslocked(struct static_key *key,
 	jump_label_unlock();
 }
 
+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_cpuslocked(struct static_key *key)
+{
+	int count = static_key_count(key);
+
+	WARN_ON_ONCE(count < 0 || count > 1);
+
+	if (count)
+		static_key_slow_dec_cpuslocked(key, 0, NULL);
+}
+
 static void __static_key_slow_dec(struct static_key *key,
 				  unsigned long rate_limit,
 				  struct delayed_work *work)
-- 
2.11.0

[PATCH v2 3/4] jump_label: Provide hotplug context variants

[Marc Zyngier]

As using the normal static key API under the hotplug lock is
pretty much impossible, let's provide a variant of some of them
that require the hotplug lock to have already been taken.

These function are only meant to be used in CPU hotplug callbacks.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 Documentation/static-keys.txt | 15 +++++++++++++++
 include/linux/jump_label.h    | 11 +++++++++--
 kernel/jump_label.c           | 20 ++++++++++++++++++++
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/static-keys.txt b/Documentation/static-keys.txt
index b83dfa1c0602..6d0a181e74d2 100644
--- a/Documentation/static-keys.txt
+++ b/Documentation/static-keys.txt
@@ -149,6 +149,21 @@ static_branch_inc(), will change the branch back to true. Likewise, if the
 key is initialized false, a 'static_branch_inc()', will change the branch to
 true. And then a 'static_branch_dec()', will again make the branch false.
 
+Note that switching branches results in some locks being taken,
+particularly the CPU hotplug lock (in order to avoid races against
+CPUs being brought in the kernel whilst the kernel is getting
+patched). Calling the static key API from within a hotplug notifier is
+thus a sure deadlock recipe. In order to still allow use of the
+functionnality, the following functions are provided:
+
+	static_key_enable_cpuslocked()
+	static_key_disable_cpuslocked()
+	static_branch_enable_cpuslocked()
+	static_branch_disable_cpuslocked()
+
+These functions are *not* general purpose, and must only be used when
+you really know that you're in the above context, and no other.
+
 Where an array of keys is required, it can be defined as::
 
 	DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 2afd74b9d844..c5e5d2ffce58 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -163,6 +163,8 @@ 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_disable(struct static_key *key);
+extern void static_key_enable_cpuslocked(struct static_key *key);
+extern void static_key_disable_cpuslocked(struct static_key *key);
 
 /*
  * We should be using ATOMIC_INIT() for initializing .enabled, but
@@ -252,6 +254,9 @@ static inline void static_key_disable(struct static_key *key)
 		static_key_slow_dec(key);
 }
 
+#define static_key_enable_cpuslocked(k)		static_key_enable((k))
+#define static_key_disable_cpuslocked(k)	static_key_disable((k))
+
 #define STATIC_KEY_INIT_TRUE	{ .enabled = ATOMIC_INIT(1) }
 #define STATIC_KEY_INIT_FALSE	{ .enabled = ATOMIC_INIT(0) }
 
@@ -413,8 +418,10 @@ extern bool ____wrong_branch_error(void);
  * Normal usage; boolean enable/disable.
  */
 
-#define static_branch_enable(x)		static_key_enable(&(x)->key)
-#define static_branch_disable(x)	static_key_disable(&(x)->key)
+#define static_branch_enable(x)			static_key_enable(&(x)->key)
+#define static_branch_disable(x)		static_key_disable(&(x)->key)
+#define static_branch_enable_cpuslocked(x)	static_key_enable_cpuslocked(&(x)->key)
+#define static_branch_disable_cpuslocked(x)	static_key_disable_cpuslocked(&(x)->key)
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 0ab8b35daa54..4e7fa067ca17 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -170,6 +170,26 @@ static void static_key_slow_dec_cpuslocked(struct static_key *key,
 	jump_label_unlock();
 }
 
+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_cpuslocked(struct static_key *key)
+{
+	int count = static_key_count(key);
+
+	WARN_ON_ONCE(count < 0 || count > 1);
+
+	if (count)
+		static_key_slow_dec_cpuslocked(key, 0, NULL);
+}
+
 static void __static_key_slow_dec(struct static_key *key,
 				  unsigned long rate_limit,
 				  struct delayed_work *work)
-- 
2.11.0

[PATCH v2 4/4] clocksource/arm_arch_timer: Use static_branch_enable_cpuslocked()

[Marc Zyngier]

Use the new static_branch_enable_cpuslocked function to switch
the workaround static key on the CPU hotplug path.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index aae87c4c546e..c62e71614c75 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -455,7 +455,11 @@ void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa
 			per_cpu(timer_unstable_counter_workaround, i) = wa;
 	}
 
-	static_branch_enable(&arch_timer_read_ool_enabled);
+	/*
+	 * 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
-- 
2.11.0

[PATCH v2 4/4] clocksource/arm_arch_timer: Use static_branch_enable_cpuslocked()

[Marc Zyngier]

Use the new static_branch_enable_cpuslocked function to switch
the workaround static key on the CPU hotplug path.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index aae87c4c546e..c62e71614c75 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -455,7 +455,11 @@ void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa
 			per_cpu(timer_unstable_counter_workaround, i) = wa;
 	}
 
-	static_branch_enable(&arch_timer_read_ool_enabled);
+	/*
+	 * 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
-- 
2.11.0

Re: [PATCH v2 3/4] jump_label: Provide hotplug context variants

[Peter Zijlstra]

On Tue, Aug 01, 2017 at 09:02:56AM +0100, Marc Zyngier wrote:
> As using the normal static key API under the hotplug lock is
> pretty much impossible, let's provide a variant of some of them
> that require the hotplug lock to have already been taken.
> 
> These function are only meant to be used in CPU hotplug callbacks.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  Documentation/static-keys.txt | 15 +++++++++++++++
>  include/linux/jump_label.h    | 11 +++++++++--
>  kernel/jump_label.c           | 20 ++++++++++++++++++++
>  3 files changed, 44 insertions(+), 2 deletions(-)

I stuffed them in my locking/core branch on top of Paolo's patches:

  https://lkml.kernel.org/r/1501601046-35683-1-git-send-email-pbonzini@redhat.com

This patch now looks like so.

---
Subject: jump_label: Provide hotplug context variants
From:   Marc Zyngier <marc.zyngier@arm.com>
Date: Tue, 1 Aug 2017 09:02:56 +0100

As using the normal static key API under the hotplug lock is
pretty much impossible, let's provide a variant of some of them
that require the hotplug lock to have already been taken.

These function are only meant to be used in CPU hotplug callbacks.

Cc: linux-arm-kernel@lists.infradead.org
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20170801080257.5056-4-marc.zyngier@arm.com
---
 Documentation/static-keys.txt |   15 +++++++++++++++
 include/linux/jump_label.h    |   11 +++++++++--
 kernel/jump_label.c           |   22 ++++++++++++++++++----
 3 files changed, 42 insertions(+), 6 deletions(-)

--- a/Documentation/static-keys.txt
+++ b/Documentation/static-keys.txt
@@ -154,6 +154,21 @@ and 'static_key_count()'.  In general, i
 should be protected with the same mutex used around the enable/disable
 or increment/decrement function.
 
+Note that switching branches results in some locks being taken,
+particularly the CPU hotplug lock (in order to avoid races against
+CPUs being brought in the kernel whilst the kernel is getting
+patched). Calling the static key API from within a hotplug notifier is
+thus a sure deadlock recipe. In order to still allow use of the
+functionnality, the following functions are provided:
+
+	static_key_enable_cpuslocked()
+	static_key_disable_cpuslocked()
+	static_branch_enable_cpuslocked()
+	static_branch_disable_cpuslocked()
+
+These functions are *not* general purpose, and must only be used when
+you really know that you're in the above context, and no other.
+
 Where an array of keys is required, it can be defined as::
 
 	DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -163,6 +163,8 @@ extern void jump_label_apply_nops(struct
 extern int static_key_count(struct static_key *key);
 extern void static_key_enable(struct static_key *key);
 extern void static_key_disable(struct static_key *key);
+extern void static_key_enable_cpuslocked(struct static_key *key);
+extern void static_key_disable_cpuslocked(struct static_key *key);
 
 /*
  * We should be using ATOMIC_INIT() for initializing .enabled, but
@@ -254,6 +256,9 @@ static inline void static_key_disable(st
 	atomic_set(&key->enabled, 0);
 }
 
+#define static_key_enable_cpuslocked(k)		static_key_enable((k))
+#define static_key_disable_cpuslocked(k)	static_key_disable((k))
+
 #define STATIC_KEY_INIT_TRUE	{ .enabled = ATOMIC_INIT(1) }
 #define STATIC_KEY_INIT_FALSE	{ .enabled = ATOMIC_INIT(0) }
 
@@ -415,8 +420,10 @@ extern bool ____wrong_branch_error(void)
  * Normal usage; boolean enable/disable.
  */
 
-#define static_branch_enable(x)		static_key_enable(&(x)->key)
-#define static_branch_disable(x)	static_key_disable(&(x)->key)
+#define static_branch_enable(x)			static_key_enable(&(x)->key)
+#define static_branch_disable(x)		static_key_disable(&(x)->key)
+#define static_branch_enable_cpuslocked(x)	static_key_enable_cpuslocked(&(x)->key)
+#define static_branch_disable_cpuslocked(x)	static_key_disable_cpuslocked(&(x)->key)
 
 #endif /* __ASSEMBLY__ */
 
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -126,15 +126,15 @@ void static_key_slow_inc(struct static_k
 }
 EXPORT_SYMBOL_GPL(static_key_slow_inc);
 
-void static_key_enable(struct static_key *key)
+void static_key_enable_cpuslocked(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE();
+
 	if (atomic_read(&key->enabled) > 0) {
 		WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
 		return;
 	}
 
-	cpus_read_lock();
 	jump_label_lock();
 	if (atomic_read(&key->enabled) == 0) {
 		atomic_set(&key->enabled, -1);
@@ -145,23 +145,37 @@ void static_key_enable(struct static_key
 		atomic_set_release(&key->enabled, 1);
 	}
 	jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_enable_cpuslocked);
+
+void static_key_enable(struct static_key *key)
+{
+	cpus_read_lock();
+	static_key_enable_cpuslocked(key);
 	cpus_read_unlock();
 }
 EXPORT_SYMBOL_GPL(static_key_enable);
 
-void static_key_disable(struct static_key *key)
+void static_key_disable_cpuslocked(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE();
+
 	if (atomic_read(&key->enabled) != 1) {
 		WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
 		return;
 	}
 
-	cpus_read_lock();
 	jump_label_lock();
 	if (atomic_cmpxchg(&key->enabled, 1, 0))
 		jump_label_update(key);
 	jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_disable_cpuslocked);
+
+void static_key_disable(struct static_key *key)
+{
+	cpus_read_lock();
+	static_key_disable_cpuslocked(key);
 	cpus_read_unlock();
 }
 EXPORT_SYMBOL_GPL(static_key_disable);

[PATCH v2 3/4] jump_label: Provide hotplug context variants

[Peter Zijlstra]

On Tue, Aug 01, 2017 at 09:02:56AM +0100, Marc Zyngier wrote:
> As using the normal static key API under the hotplug lock is
> pretty much impossible, let's provide a variant of some of them
> that require the hotplug lock to have already been taken.
> 
> These function are only meant to be used in CPU hotplug callbacks.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  Documentation/static-keys.txt | 15 +++++++++++++++
>  include/linux/jump_label.h    | 11 +++++++++--
>  kernel/jump_label.c           | 20 ++++++++++++++++++++
>  3 files changed, 44 insertions(+), 2 deletions(-)

I stuffed them in my locking/core branch on top of Paolo's patches:

  https://lkml.kernel.org/r/1501601046-35683-1-git-send-email-pbonzini at redhat.com

This patch now looks like so.

---
Subject: jump_label: Provide hotplug context variants
From:   Marc Zyngier <marc.zyngier@arm.com>
Date: Tue, 1 Aug 2017 09:02:56 +0100

As using the normal static key API under the hotplug lock is
pretty much impossible, let's provide a variant of some of them
that require the hotplug lock to have already been taken.

These function are only meant to be used in CPU hotplug callbacks.

Cc: linux-arm-kernel at lists.infradead.org
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20170801080257.5056-4-marc.zyngier at arm.com
---
 Documentation/static-keys.txt |   15 +++++++++++++++
 include/linux/jump_label.h    |   11 +++++++++--
 kernel/jump_label.c           |   22 ++++++++++++++++++----
 3 files changed, 42 insertions(+), 6 deletions(-)

--- a/Documentation/static-keys.txt
+++ b/Documentation/static-keys.txt
@@ -154,6 +154,21 @@ and 'static_key_count()'.  In general, i
 should be protected with the same mutex used around the enable/disable
 or increment/decrement function.
 
+Note that switching branches results in some locks being taken,
+particularly the CPU hotplug lock (in order to avoid races against
+CPUs being brought in the kernel whilst the kernel is getting
+patched). Calling the static key API from within a hotplug notifier is
+thus a sure deadlock recipe. In order to still allow use of the
+functionnality, the following functions are provided:
+
+	static_key_enable_cpuslocked()
+	static_key_disable_cpuslocked()
+	static_branch_enable_cpuslocked()
+	static_branch_disable_cpuslocked()
+
+These functions are *not* general purpose, and must only be used when
+you really know that you're in the above context, and no other.
+
 Where an array of keys is required, it can be defined as::
 
 	DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -163,6 +163,8 @@ extern void jump_label_apply_nops(struct
 extern int static_key_count(struct static_key *key);
 extern void static_key_enable(struct static_key *key);
 extern void static_key_disable(struct static_key *key);
+extern void static_key_enable_cpuslocked(struct static_key *key);
+extern void static_key_disable_cpuslocked(struct static_key *key);
 
 /*
  * We should be using ATOMIC_INIT() for initializing .enabled, but
@@ -254,6 +256,9 @@ static inline void static_key_disable(st
 	atomic_set(&key->enabled, 0);
 }
 
+#define static_key_enable_cpuslocked(k)		static_key_enable((k))
+#define static_key_disable_cpuslocked(k)	static_key_disable((k))
+
 #define STATIC_KEY_INIT_TRUE	{ .enabled = ATOMIC_INIT(1) }
 #define STATIC_KEY_INIT_FALSE	{ .enabled = ATOMIC_INIT(0) }
 
@@ -415,8 +420,10 @@ extern bool ____wrong_branch_error(void)
  * Normal usage; boolean enable/disable.
  */
 
-#define static_branch_enable(x)		static_key_enable(&(x)->key)
-#define static_branch_disable(x)	static_key_disable(&(x)->key)
+#define static_branch_enable(x)			static_key_enable(&(x)->key)
+#define static_branch_disable(x)		static_key_disable(&(x)->key)
+#define static_branch_enable_cpuslocked(x)	static_key_enable_cpuslocked(&(x)->key)
+#define static_branch_disable_cpuslocked(x)	static_key_disable_cpuslocked(&(x)->key)
 
 #endif /* __ASSEMBLY__ */
 
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -126,15 +126,15 @@ void static_key_slow_inc(struct static_k
 }
 EXPORT_SYMBOL_GPL(static_key_slow_inc);
 
-void static_key_enable(struct static_key *key)
+void static_key_enable_cpuslocked(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE();
+
 	if (atomic_read(&key->enabled) > 0) {
 		WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
 		return;
 	}
 
-	cpus_read_lock();
 	jump_label_lock();
 	if (atomic_read(&key->enabled) == 0) {
 		atomic_set(&key->enabled, -1);
@@ -145,23 +145,37 @@ void static_key_enable(struct static_key
 		atomic_set_release(&key->enabled, 1);
 	}
 	jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_enable_cpuslocked);
+
+void static_key_enable(struct static_key *key)
+{
+	cpus_read_lock();
+	static_key_enable_cpuslocked(key);
 	cpus_read_unlock();
 }
 EXPORT_SYMBOL_GPL(static_key_enable);
 
-void static_key_disable(struct static_key *key)
+void static_key_disable_cpuslocked(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE();
+
 	if (atomic_read(&key->enabled) != 1) {
 		WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
 		return;
 	}
 
-	cpus_read_lock();
 	jump_label_lock();
 	if (atomic_cmpxchg(&key->enabled, 1, 0))
 		jump_label_update(key);
 	jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_disable_cpuslocked);
+
+void static_key_disable(struct static_key *key)
+{
+	cpus_read_lock();
+	static_key_disable_cpuslocked(key);
 	cpus_read_unlock();
 }
 EXPORT_SYMBOL_GPL(static_key_disable);

Re: [PATCH v2 3/4] jump_label: Provide hotplug context variants

[Marc Zyngier]

On 02/08/17 15:37, Peter Zijlstra wrote:
> On Tue, Aug 01, 2017 at 09:02:56AM +0100, Marc Zyngier wrote:
>> As using the normal static key API under the hotplug lock is
>> pretty much impossible, let's provide a variant of some of them
>> that require the hotplug lock to have already been taken.
>>
>> These function are only meant to be used in CPU hotplug callbacks.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  Documentation/static-keys.txt | 15 +++++++++++++++
>>  include/linux/jump_label.h    | 11 +++++++++--
>>  kernel/jump_label.c           | 20 ++++++++++++++++++++
>>  3 files changed, 44 insertions(+), 2 deletions(-)
> 
> I stuffed them in my locking/core branch on top of Paolo's patches:
> 
>   https://lkml.kernel.org/r/1501601046-35683-1-git-send-email-pbonzini@redhat.com
> 
> This patch now looks like so.
> 
> ---
> Subject: jump_label: Provide hotplug context variants
> From:   Marc Zyngier <marc.zyngier@arm.com>
> Date: Tue, 1 Aug 2017 09:02:56 +0100
> 
> As using the normal static key API under the hotplug lock is
> pretty much impossible, let's provide a variant of some of them
> that require the hotplug lock to have already been taken.
> 
> These function are only meant to be used in CPU hotplug callbacks.
> 
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: http://lkml.kernel.org/r/20170801080257.5056-4-marc.zyngier@arm.com
> ---
>  Documentation/static-keys.txt |   15 +++++++++++++++
>  include/linux/jump_label.h    |   11 +++++++++--
>  kernel/jump_label.c           |   22 ++++++++++++++++++----
>  3 files changed, 42 insertions(+), 6 deletions(-)
> 
> --- a/Documentation/static-keys.txt
> +++ b/Documentation/static-keys.txt
> @@ -154,6 +154,21 @@ and 'static_key_count()'.  In general, i
>  should be protected with the same mutex used around the enable/disable
>  or increment/decrement function.
>  
> +Note that switching branches results in some locks being taken,
> +particularly the CPU hotplug lock (in order to avoid races against
> +CPUs being brought in the kernel whilst the kernel is getting
> +patched). Calling the static key API from within a hotplug notifier is
> +thus a sure deadlock recipe. In order to still allow use of the
> +functionnality, the following functions are provided:
> +
> +	static_key_enable_cpuslocked()
> +	static_key_disable_cpuslocked()
> +	static_branch_enable_cpuslocked()
> +	static_branch_disable_cpuslocked()
> +
> +These functions are *not* general purpose, and must only be used when
> +you really know that you're in the above context, and no other.
> +
>  Where an array of keys is required, it can be defined as::
>  
>  	DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -163,6 +163,8 @@ extern void jump_label_apply_nops(struct
>  extern int static_key_count(struct static_key *key);
>  extern void static_key_enable(struct static_key *key);
>  extern void static_key_disable(struct static_key *key);
> +extern void static_key_enable_cpuslocked(struct static_key *key);
> +extern void static_key_disable_cpuslocked(struct static_key *key);
>  
>  /*
>   * We should be using ATOMIC_INIT() for initializing .enabled, but
> @@ -254,6 +256,9 @@ static inline void static_key_disable(st
>  	atomic_set(&key->enabled, 0);
>  }
>  
> +#define static_key_enable_cpuslocked(k)		static_key_enable((k))
> +#define static_key_disable_cpuslocked(k)	static_key_disable((k))
> +
>  #define STATIC_KEY_INIT_TRUE	{ .enabled = ATOMIC_INIT(1) }
>  #define STATIC_KEY_INIT_FALSE	{ .enabled = ATOMIC_INIT(0) }
>  
> @@ -415,8 +420,10 @@ extern bool ____wrong_branch_error(void)
>   * Normal usage; boolean enable/disable.
>   */
>  
> -#define static_branch_enable(x)		static_key_enable(&(x)->key)
> -#define static_branch_disable(x)	static_key_disable(&(x)->key)
> +#define static_branch_enable(x)			static_key_enable(&(x)->key)
> +#define static_branch_disable(x)		static_key_disable(&(x)->key)
> +#define static_branch_enable_cpuslocked(x)	static_key_enable_cpuslocked(&(x)->key)
> +#define static_branch_disable_cpuslocked(x)	static_key_disable_cpuslocked(&(x)->key)
>  
>  #endif /* __ASSEMBLY__ */
>  
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -126,15 +126,15 @@ void static_key_slow_inc(struct static_k
>  }
>  EXPORT_SYMBOL_GPL(static_key_slow_inc);
>  
> -void static_key_enable(struct static_key *key)
> +void static_key_enable_cpuslocked(struct static_key *key)
>  {
>  	STATIC_KEY_CHECK_USE();
> +
>  	if (atomic_read(&key->enabled) > 0) {
>  		WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
>  		return;
>  	}
>  
> -	cpus_read_lock();
>  	jump_label_lock();
>  	if (atomic_read(&key->enabled) == 0) {
>  		atomic_set(&key->enabled, -1);
> @@ -145,23 +145,37 @@ void static_key_enable(struct static_key
>  		atomic_set_release(&key->enabled, 1);
>  	}
>  	jump_label_unlock();
> +}
> +EXPORT_SYMBOL_GPL(static_key_enable_cpuslocked);
> +
> +void static_key_enable(struct static_key *key)
> +{
> +	cpus_read_lock();
> +	static_key_enable_cpuslocked(key);
>  	cpus_read_unlock();
>  }
>  EXPORT_SYMBOL_GPL(static_key_enable);
>  
> -void static_key_disable(struct static_key *key)
> +void static_key_disable_cpuslocked(struct static_key *key)
>  {
>  	STATIC_KEY_CHECK_USE();
> +
>  	if (atomic_read(&key->enabled) != 1) {
>  		WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
>  		return;
>  	}
>  
> -	cpus_read_lock();
>  	jump_label_lock();
>  	if (atomic_cmpxchg(&key->enabled, 1, 0))
>  		jump_label_update(key);
>  	jump_label_unlock();
> +}
> +EXPORT_SYMBOL_GPL(static_key_disable_cpuslocked);
> +
> +void static_key_disable(struct static_key *key)
> +{
> +	cpus_read_lock();
> +	static_key_disable_cpuslocked(key);
>  	cpus_read_unlock();
>  }
>  EXPORT_SYMBOL_GPL(static_key_disable);
> 

I've just tried this one of the affected systems, and it booted just
fine. So thumbs up from me.

Thanks again,

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

[PATCH v2 3/4] jump_label: Provide hotplug context variants

[Marc Zyngier]

On 02/08/17 15:37, Peter Zijlstra wrote:
> On Tue, Aug 01, 2017 at 09:02:56AM +0100, Marc Zyngier wrote:
>> As using the normal static key API under the hotplug lock is
>> pretty much impossible, let's provide a variant of some of them
>> that require the hotplug lock to have already been taken.
>>
>> These function are only meant to be used in CPU hotplug callbacks.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  Documentation/static-keys.txt | 15 +++++++++++++++
>>  include/linux/jump_label.h    | 11 +++++++++--
>>  kernel/jump_label.c           | 20 ++++++++++++++++++++
>>  3 files changed, 44 insertions(+), 2 deletions(-)
> 
> I stuffed them in my locking/core branch on top of Paolo's patches:
> 
>   https://lkml.kernel.org/r/1501601046-35683-1-git-send-email-pbonzini at redhat.com
> 
> This patch now looks like so.
> 
> ---
> Subject: jump_label: Provide hotplug context variants
> From:   Marc Zyngier <marc.zyngier@arm.com>
> Date: Tue, 1 Aug 2017 09:02:56 +0100
> 
> As using the normal static key API under the hotplug lock is
> pretty much impossible, let's provide a variant of some of them
> that require the hotplug lock to have already been taken.
> 
> These function are only meant to be used in CPU hotplug callbacks.
> 
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: http://lkml.kernel.org/r/20170801080257.5056-4-marc.zyngier at arm.com
> ---
>  Documentation/static-keys.txt |   15 +++++++++++++++
>  include/linux/jump_label.h    |   11 +++++++++--
>  kernel/jump_label.c           |   22 ++++++++++++++++++----
>  3 files changed, 42 insertions(+), 6 deletions(-)
> 
> --- a/Documentation/static-keys.txt
> +++ b/Documentation/static-keys.txt
> @@ -154,6 +154,21 @@ and 'static_key_count()'.  In general, i
>  should be protected with the same mutex used around the enable/disable
>  or increment/decrement function.
>  
> +Note that switching branches results in some locks being taken,
> +particularly the CPU hotplug lock (in order to avoid races against
> +CPUs being brought in the kernel whilst the kernel is getting
> +patched). Calling the static key API from within a hotplug notifier is
> +thus a sure deadlock recipe. In order to still allow use of the
> +functionnality, the following functions are provided:
> +
> +	static_key_enable_cpuslocked()
> +	static_key_disable_cpuslocked()
> +	static_branch_enable_cpuslocked()
> +	static_branch_disable_cpuslocked()
> +
> +These functions are *not* general purpose, and must only be used when
> +you really know that you're in the above context, and no other.
> +
>  Where an array of keys is required, it can be defined as::
>  
>  	DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -163,6 +163,8 @@ extern void jump_label_apply_nops(struct
>  extern int static_key_count(struct static_key *key);
>  extern void static_key_enable(struct static_key *key);
>  extern void static_key_disable(struct static_key *key);
> +extern void static_key_enable_cpuslocked(struct static_key *key);
> +extern void static_key_disable_cpuslocked(struct static_key *key);
>  
>  /*
>   * We should be using ATOMIC_INIT() for initializing .enabled, but
> @@ -254,6 +256,9 @@ static inline void static_key_disable(st
>  	atomic_set(&key->enabled, 0);
>  }
>  
> +#define static_key_enable_cpuslocked(k)		static_key_enable((k))
> +#define static_key_disable_cpuslocked(k)	static_key_disable((k))
> +
>  #define STATIC_KEY_INIT_TRUE	{ .enabled = ATOMIC_INIT(1) }
>  #define STATIC_KEY_INIT_FALSE	{ .enabled = ATOMIC_INIT(0) }
>  
> @@ -415,8 +420,10 @@ extern bool ____wrong_branch_error(void)
>   * Normal usage; boolean enable/disable.
>   */
>  
> -#define static_branch_enable(x)		static_key_enable(&(x)->key)
> -#define static_branch_disable(x)	static_key_disable(&(x)->key)
> +#define static_branch_enable(x)			static_key_enable(&(x)->key)
> +#define static_branch_disable(x)		static_key_disable(&(x)->key)
> +#define static_branch_enable_cpuslocked(x)	static_key_enable_cpuslocked(&(x)->key)
> +#define static_branch_disable_cpuslocked(x)	static_key_disable_cpuslocked(&(x)->key)
>  
>  #endif /* __ASSEMBLY__ */
>  
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -126,15 +126,15 @@ void static_key_slow_inc(struct static_k
>  }
>  EXPORT_SYMBOL_GPL(static_key_slow_inc);
>  
> -void static_key_enable(struct static_key *key)
> +void static_key_enable_cpuslocked(struct static_key *key)
>  {
>  	STATIC_KEY_CHECK_USE();
> +
>  	if (atomic_read(&key->enabled) > 0) {
>  		WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
>  		return;
>  	}
>  
> -	cpus_read_lock();
>  	jump_label_lock();
>  	if (atomic_read(&key->enabled) == 0) {
>  		atomic_set(&key->enabled, -1);
> @@ -145,23 +145,37 @@ void static_key_enable(struct static_key
>  		atomic_set_release(&key->enabled, 1);
>  	}
>  	jump_label_unlock();
> +}
> +EXPORT_SYMBOL_GPL(static_key_enable_cpuslocked);
> +
> +void static_key_enable(struct static_key *key)
> +{
> +	cpus_read_lock();
> +	static_key_enable_cpuslocked(key);
>  	cpus_read_unlock();
>  }
>  EXPORT_SYMBOL_GPL(static_key_enable);
>  
> -void static_key_disable(struct static_key *key)
> +void static_key_disable_cpuslocked(struct static_key *key)
>  {
>  	STATIC_KEY_CHECK_USE();
> +
>  	if (atomic_read(&key->enabled) != 1) {
>  		WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
>  		return;
>  	}
>  
> -	cpus_read_lock();
>  	jump_label_lock();
>  	if (atomic_cmpxchg(&key->enabled, 1, 0))
>  		jump_label_update(key);
>  	jump_label_unlock();
> +}
> +EXPORT_SYMBOL_GPL(static_key_disable_cpuslocked);
> +
> +void static_key_disable(struct static_key *key)
> +{
> +	cpus_read_lock();
> +	static_key_disable_cpuslocked(key);
>  	cpus_read_unlock();
>  }
>  EXPORT_SYMBOL_GPL(static_key_disable);
> 

I've just tried this one of the affected systems, and it booted just
fine. So thumbs up from me.

Thanks again,

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

[tip:locking/core] jump_label: Move CPU hotplug locking

[tip-bot for Marc Zyngier]

Commit-ID:  b70cecf4b6b72a9977576ab32cca0e24f286f517
Gitweb:     http://git.kernel.org/tip/b70cecf4b6b72a9977576ab32cca0e24f286f517
Author:     Marc Zyngier <marc.zyngier@arm.com>
AuthorDate: Tue, 1 Aug 2017 09:02:54 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 10 Aug 2017 12:28:58 +0200

jump_label: Move CPU hotplug locking

As we're about to rework the locking, let's move the taking and
release of the CPU hotplug lock to locations that will make its
reworking completely obvious.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20170801080257.5056-2-marc.zyngier@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/jump_label.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index f2ea678..161301f 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -83,6 +83,7 @@ void static_key_slow_inc(struct static_key *key)
 {
 	int v, v1;
 
+	cpus_read_lock();
 	STATIC_KEY_CHECK_USE();
 
 	/*
@@ -99,11 +100,12 @@ void static_key_slow_inc(struct static_key *key)
 	 */
 	for (v = atomic_read(&key->enabled); v > 0; v = v1) {
 		v1 = atomic_cmpxchg(&key->enabled, v, v + 1);
-		if (likely(v1 == v))
+		if (likely(v1 == v)) {
+			cpus_read_unlock();
 			return;
+		}
 	}
 
-	cpus_read_lock();
 	jump_label_lock();
 	if (atomic_read(&key->enabled) == 0) {
 		atomic_set(&key->enabled, -1);

[tip:locking/core] jump_label: Split out code under the hotplug lock

[tip-bot for Marc Zyngier]

Commit-ID:  8b7b412807053ab5f059ffae426a280e769a5bda
Gitweb:     http://git.kernel.org/tip/8b7b412807053ab5f059ffae426a280e769a5bda
Author:     Marc Zyngier <marc.zyngier@arm.com>
AuthorDate: Tue, 1 Aug 2017 09:02:55 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 10 Aug 2017 12:28:58 +0200

jump_label: Split out code under the hotplug lock

In order to later introduce an "already locked" version of some
of the static key funcions, let's split the code into the core stuff
(the *_cpuslocked functions) and the usual helpers, which now
take/release the hotplug lock and call into the _cpuslocked
versions.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20170801080257.5056-3-marc.zyngier@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/jump_label.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 161301f..cc6d815 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -79,11 +79,10 @@ int static_key_count(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_count);
 
-void static_key_slow_inc(struct static_key *key)
+static void static_key_slow_inc_cpuslocked(struct static_key *key)
 {
 	int v, v1;
 
-	cpus_read_lock();
 	STATIC_KEY_CHECK_USE();
 
 	/*
@@ -100,10 +99,8 @@ void static_key_slow_inc(struct static_key *key)
 	 */
 	for (v = atomic_read(&key->enabled); v > 0; v = v1) {
 		v1 = atomic_cmpxchg(&key->enabled, v, v + 1);
-		if (likely(v1 == v)) {
-			cpus_read_unlock();
+		if (likely(v1 == v))
 			return;
-		}
 	}
 
 	jump_label_lock();
@@ -119,6 +116,12 @@ void static_key_slow_inc(struct static_key *key)
 		atomic_inc(&key->enabled);
 	}
 	jump_label_unlock();
+}
+
+void static_key_slow_inc(struct static_key *key)
+{
+	cpus_read_lock();
+	static_key_slow_inc_cpuslocked(key);
 	cpus_read_unlock();
 }
 EXPORT_SYMBOL_GPL(static_key_slow_inc);
@@ -163,10 +166,10 @@ void static_key_disable(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_disable);
 
-static void __static_key_slow_dec(struct static_key *key,
-		unsigned long rate_limit, struct delayed_work *work)
+static void static_key_slow_dec_cpuslocked(struct static_key *key,
+					   unsigned long rate_limit,
+					   struct delayed_work *work)
 {
-	cpus_read_lock();
 	/*
 	 * The negative count check is valid even when a negative
 	 * key->enabled is in use by static_key_slow_inc(); a
@@ -177,7 +180,6 @@ static void __static_key_slow_dec(struct static_key *key,
 	if (!atomic_dec_and_mutex_lock(&key->enabled, &jump_label_mutex)) {
 		WARN(atomic_read(&key->enabled) < 0,
 		     "jump label: negative count!\n");
-		cpus_read_unlock();
 		return;
 	}
 
@@ -188,6 +190,14 @@ static void __static_key_slow_dec(struct static_key *key,
 		jump_label_update(key);
 	}
 	jump_label_unlock();
+}
+
+static void __static_key_slow_dec(struct static_key *key,
+				  unsigned long rate_limit,
+				  struct delayed_work *work)
+{
+	cpus_read_lock();
+	static_key_slow_dec_cpuslocked(key, rate_limit, work);
 	cpus_read_unlock();
 }
 

[tip:locking/core] jump_label: Provide hotplug context variants

[tip-bot for Marc Zyngier]

Commit-ID:  5a40527f8f0798553764fc8db4111d7d9c33ea51
Gitweb:     http://git.kernel.org/tip/5a40527f8f0798553764fc8db4111d7d9c33ea51
Author:     Marc Zyngier <marc.zyngier@arm.com>
AuthorDate: Tue, 1 Aug 2017 09:02:56 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 10 Aug 2017 12:28:59 +0200

jump_label: Provide hotplug context variants

As using the normal static key API under the hotplug lock is
pretty much impossible, let's provide a variant of some of them
that require the hotplug lock to have already been taken.

These function are only meant to be used in CPU hotplug callbacks.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20170801080257.5056-4-marc.zyngier@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 Documentation/static-keys.txt | 15 +++++++++++++++
 include/linux/jump_label.h    | 11 +++++++++--
 kernel/jump_label.c           | 22 ++++++++++++++++++----
 3 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/Documentation/static-keys.txt b/Documentation/static-keys.txt
index 870b4be..ab16efe 100644
--- a/Documentation/static-keys.txt
+++ b/Documentation/static-keys.txt
@@ -154,6 +154,21 @@ and 'static_key_count()'.  In general, if you use these functions, they
 should be protected with the same mutex used around the enable/disable
 or increment/decrement function.
 
+Note that switching branches results in some locks being taken,
+particularly the CPU hotplug lock (in order to avoid races against
+CPUs being brought in the kernel whilst the kernel is getting
+patched). Calling the static key API from within a hotplug notifier is
+thus a sure deadlock recipe. In order to still allow use of the
+functionnality, the following functions are provided:
+
+	static_key_enable_cpuslocked()
+	static_key_disable_cpuslocked()
+	static_branch_enable_cpuslocked()
+	static_branch_disable_cpuslocked()
+
+These functions are *not* general purpose, and must only be used when
+you really know that you're in the above context, and no other.
+
 Where an array of keys is required, it can be defined as::
 
 	DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 740a42e..cd58616 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -163,6 +163,8 @@ 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_disable(struct static_key *key);
+extern void static_key_enable_cpuslocked(struct static_key *key);
+extern void static_key_disable_cpuslocked(struct static_key *key);
 
 /*
  * We should be using ATOMIC_INIT() for initializing .enabled, but
@@ -254,6 +256,9 @@ static inline void static_key_disable(struct static_key *key)
 	atomic_set(&key->enabled, 0);
 }
 
+#define static_key_enable_cpuslocked(k)		static_key_enable((k))
+#define static_key_disable_cpuslocked(k)	static_key_disable((k))
+
 #define STATIC_KEY_INIT_TRUE	{ .enabled = ATOMIC_INIT(1) }
 #define STATIC_KEY_INIT_FALSE	{ .enabled = ATOMIC_INIT(0) }
 
@@ -415,8 +420,10 @@ extern bool ____wrong_branch_error(void);
  * Normal usage; boolean enable/disable.
  */
 
-#define static_branch_enable(x)		static_key_enable(&(x)->key)
-#define static_branch_disable(x)	static_key_disable(&(x)->key)
+#define static_branch_enable(x)			static_key_enable(&(x)->key)
+#define static_branch_disable(x)		static_key_disable(&(x)->key)
+#define static_branch_enable_cpuslocked(x)	static_key_enable_cpuslocked(&(x)->key)
+#define static_branch_disable_cpuslocked(x)	static_key_disable_cpuslocked(&(x)->key)
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index cc6d815..0bf2e8f5 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -126,15 +126,15 @@ void static_key_slow_inc(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_slow_inc);
 
-void static_key_enable(struct static_key *key)
+void static_key_enable_cpuslocked(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE();
+
 	if (atomic_read(&key->enabled) > 0) {
 		WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
 		return;
 	}
 
-	cpus_read_lock();
 	jump_label_lock();
 	if (atomic_read(&key->enabled) == 0) {
 		atomic_set(&key->enabled, -1);
@@ -145,23 +145,37 @@ void static_key_enable(struct static_key *key)
 		atomic_set_release(&key->enabled, 1);
 	}
 	jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_enable_cpuslocked);
+
+void static_key_enable(struct static_key *key)
+{
+	cpus_read_lock();
+	static_key_enable_cpuslocked(key);
 	cpus_read_unlock();
 }
 EXPORT_SYMBOL_GPL(static_key_enable);
 
-void static_key_disable(struct static_key *key)
+void static_key_disable_cpuslocked(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE();
+
 	if (atomic_read(&key->enabled) != 1) {
 		WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
 		return;
 	}
 
-	cpus_read_lock();
 	jump_label_lock();
 	if (atomic_cmpxchg(&key->enabled, 1, 0))
 		jump_label_update(key);
 	jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_disable_cpuslocked);
+
+void static_key_disable(struct static_key *key)
+{
+	cpus_read_lock();
+	static_key_disable_cpuslocked(key);
 	cpus_read_unlock();
 }
 EXPORT_SYMBOL_GPL(static_key_disable);

[tip:locking/core] clocksource/arm_arch_timer: Use static_branch_enable_cpuslocked()

[tip-bot for Marc Zyngier]

Commit-ID:  450f9689f294c331c56ec37d68302ccc19c7caa2
Gitweb:     http://git.kernel.org/tip/450f9689f294c331c56ec37d68302ccc19c7caa2
Author:     Marc Zyngier <marc.zyngier@arm.com>
AuthorDate: Tue, 1 Aug 2017 09:02:57 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 10 Aug 2017 12:29:00 +0200

clocksource/arm_arch_timer: Use static_branch_enable_cpuslocked()

Use the new static_branch_enable_cpuslocked() function to switch
the workaround static key on the CPU hotplug path.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20170801080257.5056-5-marc.zyngier@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/clocksource/arm_arch_timer.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index aae87c4..c62e716 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -455,7 +455,11 @@ void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa
 			per_cpu(timer_unstable_counter_workaround, i) = wa;
 	}
 
-	static_branch_enable(&arch_timer_read_ool_enabled);
+	/*
+	 * 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