[PATCH 1/2] jump_label: export static_key_slow_inc/dec_cpuslocked()

[PATCH 1/2] jump_label: export static_key_slow_inc/dec_cpuslocked()

[Konstantin Khlebnikov]

For fixing cpu_hotplug_lock recursion in tg_set_cfs_bandwidth().

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 include/linux/jump_label.h |   12 ++++++++++++
 kernel/jump_label.c        |   18 +++++++++++++-----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index c7b368c734af..db10e6a9d315 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -160,6 +160,8 @@ extern void arch_jump_label_transform_static(struct jump_entry *entry,
 extern int jump_label_text_reserved(void *start, void *end);
 extern void static_key_slow_inc(struct static_key *key);
 extern void static_key_slow_dec(struct static_key *key);
+extern void static_key_slow_inc_cpuslocked(struct static_key *key);
+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);
@@ -222,6 +224,16 @@ static inline void static_key_slow_dec(struct static_key *key)
 	atomic_dec(&key->enabled);
 }
 
+static inline void static_key_slow_inc_cpuslocked(struct static_key *key)
+{
+	static_key_slow_inc(key);
+}
+
+static inline void static_key_slow_dec_cpuslocked(struct static_key *key)
+{
+	static_key_slow_dec(key);
+}
+
 static inline int jump_label_text_reserved(void *start, void *end)
 {
 	return 0;
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 8594d24e4adc..9813895231cc 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -79,7 +79,7 @@ int static_key_count(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_count);
 
-static void static_key_slow_inc_cpuslocked(struct static_key *key)
+void static_key_slow_inc_cpuslocked(struct static_key *key)
 {
 	int v, v1;
 
@@ -117,6 +117,7 @@ static void static_key_slow_inc_cpuslocked(struct static_key *key)
 	}
 	jump_label_unlock();
 }
+EXPORT_SYMBOL_GPL(static_key_slow_inc_cpuslocked);
 
 void static_key_slow_inc(struct static_key *key)
 {
@@ -180,9 +181,9 @@ void static_key_disable(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_disable);
 
-static void static_key_slow_dec_cpuslocked(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)
 {
 	/*
 	 * The negative count check is valid even when a negative
@@ -206,12 +207,19 @@ static void static_key_slow_dec_cpuslocked(struct static_key *key,
 	jump_label_unlock();
 }
 
+void static_key_slow_dec_cpuslocked(struct static_key *key)
+{
+	STATIC_KEY_CHECK_USE(key);
+	__static_key_slow_dec_cpuslocked(key, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(static_key_slow_dec_cpuslocked);
+
 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);
+	__static_key_slow_dec_cpuslocked(key, rate_limit, work);
 	cpus_read_unlock();
 }
 

[PATCH 2/2] sched/core: fix cpu_hotplug_lock recursion in tg_set_cfs_bandwidth()

[Konstantin Khlebnikov]

After commit fc8dffd379ca ("cpu/hotplug: Convert hotplug locking to
percpu rwsem") get_online_cpus() must be non-recursive.

Cpu hotplug is already locked for read in tg_set_cfs_bandwidth() and
static_key_slow_inc() in cfs_bandwidth_usage_inc() locks it again.

Switch to cpus_read_[un]lock() and static_key_slow_inc/dec_cpuslocked().

This fixes lockdep warning:

============================================
WARNING: possible recursive locking detected
4.14.11-debug-test #5 Not tainted
--------------------------------------------
portod-worker27/4773 is trying to acquire lock:
 (cpu_hotplug_lock.rw_sem){++++}, at: static_key_slow_inc+0xe/0x170

but task is already holding lock:
 (cpu_hotplug_lock.rw_sem){++++}, at: tg_set_cfs_bandwidth+0xc6/0x890

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock(cpu_hotplug_lock.rw_sem);
  lock(cpu_hotplug_lock.rw_sem);

 *** DEADLOCK ***

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 kernel/sched/core.c  |    4 ++--
 kernel/sched/fair.c  |    4 ++--
 kernel/sched/sched.h |    1 +
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 644fa2e3d993..584832b49fdc 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6448,7 +6448,7 @@ static int tg_set_cfs_bandwidth(struct task_group *tg, u64 period, u64 quota)
 	 * Prevent race between setting of cfs_rq->runtime_enabled and
 	 * unthrottle_offline_cfs_rqs().
 	 */
-	get_online_cpus();
+	cpus_read_lock();
 	mutex_lock(&cfs_constraints_mutex);
 	ret = __cfs_schedulable(tg, period, quota);
 	if (ret)
@@ -6491,7 +6491,7 @@ static int tg_set_cfs_bandwidth(struct task_group *tg, u64 period, u64 quota)
 		cfs_bandwidth_usage_dec();
 out_unlock:
 	mutex_unlock(&cfs_constraints_mutex);
-	put_online_cpus();
+	cpus_read_unlock();
 
 	return ret;
 }
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 2fe3aa853e4d..26a71ebcd3c2 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4365,12 +4365,12 @@ static inline bool cfs_bandwidth_used(void)
 
 void cfs_bandwidth_usage_inc(void)
 {
-	static_key_slow_inc(&__cfs_bandwidth_used);
+	static_key_slow_inc_cpuslocked(&__cfs_bandwidth_used);
 }
 
 void cfs_bandwidth_usage_dec(void)
 {
-	static_key_slow_dec(&__cfs_bandwidth_used);
+	static_key_slow_dec_cpuslocked(&__cfs_bandwidth_used);
 }
 #else /* HAVE_JUMP_LABEL */
 static bool cfs_bandwidth_used(void)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index b19552a212de..7dddc531ba63 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2007,6 +2007,7 @@ extern void init_cfs_rq(struct cfs_rq *cfs_rq);
 extern void init_rt_rq(struct rt_rq *rt_rq);
 extern void init_dl_rq(struct dl_rq *dl_rq);
 
+/* Must be called under cpus_read_lock() */
 extern void cfs_bandwidth_usage_inc(void);
 extern void cfs_bandwidth_usage_dec(void);
 

Re: [PATCH 1/2] jump_label: export static_key_slow_inc/dec_cpuslocked()

[Peter Zijlstra]

On Wed, Jan 03, 2018 at 11:29:50PM +0300, Konstantin Khlebnikov wrote:
> For fixing cpu_hotplug_lock recursion in tg_set_cfs_bandwidth().

Why would that need module exports?

Re: [PATCH 1/2] jump_label: export static_key_slow_inc/dec_cpuslocked()

[Konstantin Khlebnikov]

On Thu, Jan 4, 2018 at 2:20 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Jan 03, 2018 at 11:29:50PM +0300, Konstantin Khlebnikov wrote:
>> For fixing cpu_hotplug_lock recursion in tg_set_cfs_bandwidth().
>
> Why would that need module exports?

Just for symmetry with other functions.

Re: [PATCH 1/2] jump_label: export static_key_slow_inc/dec_cpuslocked()

[Peter Zijlstra]

On Thu, Jan 04, 2018 at 10:36:50AM +0300, Konstantin Khlebnikov wrote:
> On Thu, Jan 4, 2018 at 2:20 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Wed, Jan 03, 2018 at 11:29:50PM +0300, Konstantin Khlebnikov wrote:
> >> For fixing cpu_hotplug_lock recursion in tg_set_cfs_bandwidth().
> >
> > Why would that need module exports?
> 
> Just for symmetry with other functions.

No exports unless there are in-tree users.

[PATCH 1/2 v2] jump_label: export static_key_slow_inc/dec_cpuslocked()

[Konstantin Khlebnikov]

For fixing cpu_hotplug_lock recursion in tg_set_cfs_bandwidth().

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>

---

v2: remove EXPORT_SYMBOL_GPL, second patch unchanged
---
 include/linux/jump_label.h |   12 ++++++++++++
 kernel/jump_label.c        |   16 +++++++++++-----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index c7b368c734af..db10e6a9d315 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -160,6 +160,8 @@ extern void arch_jump_label_transform_static(struct jump_entry *entry,
 extern int jump_label_text_reserved(void *start, void *end);
 extern void static_key_slow_inc(struct static_key *key);
 extern void static_key_slow_dec(struct static_key *key);
+extern void static_key_slow_inc_cpuslocked(struct static_key *key);
+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);
@@ -222,6 +224,16 @@ static inline void static_key_slow_dec(struct static_key *key)
 	atomic_dec(&key->enabled);
 }
 
+static inline void static_key_slow_inc_cpuslocked(struct static_key *key)
+{
+	static_key_slow_inc(key);
+}
+
+static inline void static_key_slow_dec_cpuslocked(struct static_key *key)
+{
+	static_key_slow_dec(key);
+}
+
 static inline int jump_label_text_reserved(void *start, void *end)
 {
 	return 0;
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 8594d24e4adc..934ef0bb6c0d 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -79,7 +79,7 @@ int static_key_count(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_count);
 
-static void static_key_slow_inc_cpuslocked(struct static_key *key)
+void static_key_slow_inc_cpuslocked(struct static_key *key)
 {
 	int v, v1;
 
@@ -180,9 +180,9 @@ void static_key_disable(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_disable);
 
-static void static_key_slow_dec_cpuslocked(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)
 {
 	/*
 	 * The negative count check is valid even when a negative
@@ -206,12 +206,18 @@ static void static_key_slow_dec_cpuslocked(struct static_key *key,
 	jump_label_unlock();
 }
 
+void static_key_slow_dec_cpuslocked(struct static_key *key)
+{
+	STATIC_KEY_CHECK_USE(key);
+	__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)
 {
 	cpus_read_lock();
-	static_key_slow_dec_cpuslocked(key, rate_limit, work);
+	__static_key_slow_dec_cpuslocked(key, rate_limit, work);
 	cpus_read_unlock();
 }