For fixing cpu_hotplug_lock recursion in tg_set_cfs_bandwidth().
Signed-off-by: Konstantin Khlebnikov <[email protected]>
---
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();
}
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 <[email protected]>
---
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);
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?
On Thu, Jan 4, 2018 at 2:20 AM, Peter Zijlstra <[email protected]> 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.
On Thu, Jan 04, 2018 at 10:36:50AM +0300, Konstantin Khlebnikov wrote:
> On Thu, Jan 4, 2018 at 2:20 AM, Peter Zijlstra <[email protected]> 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.
For fixing cpu_hotplug_lock recursion in tg_set_cfs_bandwidth().
Signed-off-by: Konstantin Khlebnikov <[email protected]>
---
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();
}