All of lore.kernel.org
 help / color / mirror / Atom feed
* Spurious lockdep splat in v4.15-rc9
@ 2018-01-22 17:10 Tejun Heo
  2018-01-22 17:40 ` Peter Zijlstra
  2018-01-22 21:53 ` Peter Zijlstra
  0 siblings, 2 replies; 5+ messages in thread
From: Tejun Heo @ 2018-01-22 17:10 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar; +Cc: linux-kernel

Hello, Peter, Ingo.

I get the below lockdep warning if I try to write a config into cgroup
cpu.max file.  It's warning about A-A deadlock, but it's obviously
spurious - the system doesn't lock up and the warning is about two
get_online_cpus() calls nesting.

Thanks.

[   79.106704]
[   79.106886] ============================================
[   79.107319] WARNING: possible recursive locking detected
[   79.107741] 4.15.0-rc9-work+ #61 Not tainted
[   79.108080] --------------------------------------------
[   79.108505] bash/2133 is trying to acquire lock:
[   79.108872]  (cpu_hotplug_lock.rw_sem){++++}, at: [<00000000b3203afd>] static_key_slow_inc+0xe/0xa0
[   79.109593]
[   79.109593] but task is already holding lock:
[   79.110058]  (cpu_hotplug_lock.rw_sem){++++}, at: [<00000000748e6cec>] tg_set_cfs_bandwidth+0x51/0x330
[   79.110801]
[   79.110801] other info that might help us debug this:
[   79.111322]  Possible unsafe locking scenario:
[   79.111322]
[   79.111792]        CPU0
[   79.111992]        ----
[   79.112197]   lock(cpu_hotplug_lock.rw_sem);
[   79.112537]   lock(cpu_hotplug_lock.rw_sem);
[   79.112880]
[   79.112880]  *** DEADLOCK ***
[   79.112880]
[   79.113355]  May be due to missing lock nesting notation
[   79.113355]
[   79.113893] 5 locks held by bash/2133:
[   79.114199]  #0:  (sb_writers#7){.+.+}, at: [<00000000259a9362>] vfs_write+0x18a/0x1c0
[   79.114830]  #1:  (&of->mutex){+.+.}, at: [<00000000b1a2a028>] kernfs_fop_write+0xde/0x1a0
[   79.115492]  #2:  (kn->count#182){.+.+}, at: [<000000008f74a9a4>] kernfs_fop_write+0xe6/0x1a0
[   79.116182]  #3:  (cpu_hotplug_lock.rw_sem){++++}, at: [<00000000748e6cec>] tg_set_cfs_bandwidth+0x51/0x330
[   79.116956]  #4:  (cfs_constraints_mutex){+.+.}, at: [<000000007a63f0e9>] tg_set_cfs_bandwidth+0x5f/0x330
[   79.117717]
[   79.117717] stack backtrace:
[   79.118072] CPU: 13 PID: 2133 Comm: bash Not tainted 4.15.0-rc9-work+ #61
[   79.118616] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-3.el7_4.1 04/01/2014
[   79.119331] Call Trace:
[   79.119539]  dump_stack+0x5e/0x8f
[   79.119809]  __lock_acquire+0x150e/0x15f0
[   79.120136]  ? tg_set_cfs_bandwidth+0x5f/0x330
[   79.120497]  ? __mutex_lock+0x204/0x930
[   79.120805]  ? tg_set_cfs_bandwidth+0x5f/0x330
[   79.121164]  lock_acquire+0xb0/0x200
[   79.121454]  ? static_key_slow_inc+0xe/0xa0
[   79.121798]  cpus_read_lock+0x43/0xb0
[   79.122095]  ? static_key_slow_inc+0xe/0xa0
[   79.122438]  static_key_slow_inc+0xe/0xa0
[   79.122763]  tg_set_cfs_bandwidth+0x30e/0x330
[   79.123112]  ? tg_set_cfs_bandwidth+0xaa/0x330
[   79.123473]  cpu_max_write+0xb8/0x100
[   79.123773]  cgroup_file_write+0x69/0x200
[   79.124100]  kernfs_fop_write+0x10e/0x1a0
[   79.124430]  __vfs_write+0x23/0x130
[   79.124721]  ? rcu_read_lock_sched_held+0x96/0xa0
[   79.125100]  ? rcu_sync_lockdep_assert+0x2a/0x50
[   79.125475]  ? __sb_start_write+0x194/0x230
[   79.125812]  ? vfs_write+0x18a/0x1c0
[   79.126109]  ? __close_fd+0x66/0xd0
[   79.126392]  vfs_write+0xbf/0x1c0
[   79.126660]  SyS_write+0x45/0xa0
[   79.126928]  entry_SYSCALL_64_fastpath+0x18/0x85
[   79.127306] RIP: 0033:0x7f3b040e21f0

-- 
tejun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Spurious lockdep splat in v4.15-rc9
  2018-01-22 17:10 Spurious lockdep splat in v4.15-rc9 Tejun Heo
@ 2018-01-22 17:40 ` Peter Zijlstra
  2018-01-22 21:53 ` Peter Zijlstra
  1 sibling, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2018-01-22 17:40 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Ingo Molnar, linux-kernel

On Mon, Jan 22, 2018 at 09:10:18AM -0800, Tejun Heo wrote:
> Hello, Peter, Ingo.
> 
> I get the below lockdep warning if I try to write a config into cgroup
> cpu.max file.  It's warning about A-A deadlock, but it's obviously
> spurious - the system doesn't lock up and the warning is about two
> get_online_cpus() calls nesting.

Looks real, just not instantly fatal. It would generate an actual
deadlock the moment there is a contending hotplug operation around
though.

I'll have a wee look.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Spurious lockdep splat in v4.15-rc9
  2018-01-22 17:10 Spurious lockdep splat in v4.15-rc9 Tejun Heo
  2018-01-22 17:40 ` Peter Zijlstra
@ 2018-01-22 21:53 ` Peter Zijlstra
  2018-01-22 22:03   ` Tejun Heo
  2018-01-24 10:38   ` [tip:sched/urgent] sched/core: Fix cpu.max vs. cpuhotplug deadlock tip-bot for Peter Zijlstra
  1 sibling, 2 replies; 5+ messages in thread
From: Peter Zijlstra @ 2018-01-22 21:53 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Ingo Molnar, linux-kernel, Thomas Gleixner


Tejun, does the below work for you (compile tested only).

---
Subject: sched: Fix cpu.max vs cpuhotplug deadlock

Tejun reported the following cpu-hotplug lock (percpu-rwsem) read recursion:

  tg_set_cfs_bandwidth()
    get_online_cpus()
      cpus_read_lock()

    cfs_bandwidth_usage_inc()
      static_key_slow_inc()
        cpus_read_lock()

Reported-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/jump_label.h |  7 +++++++
 kernel/jump_label.c        | 12 +++++++++---
 kernel/sched/fair.c        |  4 ++--
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index c7b368c734af..e0340ca08d98 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,9 @@ static inline void static_key_slow_dec(struct static_key *key)
 	atomic_dec(&key->enabled);
 }
 
+#define static_key_slow_inc_cpuslocked(key) static_key_slow_inc(key)
+#define static_key_slow_dec_cpuslocked(key) static_key_slow_dec(key)
+
 static inline int jump_label_text_reserved(void *start, void *end)
 {
 	return 0;
@@ -416,6 +421,8 @@ struct static_key_false {
 
 #define static_branch_inc(x)		static_key_slow_inc(&(x)->key)
 #define static_branch_dec(x)		static_key_slow_dec(&(x)->key)
+#define static_branch_inc_cpuslocked(x)	static_key_slow_inc_cpuslocked(&(x)->key)
+#define static_branch_dec_cpuslocked(x)	static_key_slow_dec_cpuslocked(&(x)->key)
 
 /*
  * Normal usage; boolean enable/disable.
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 8594d24e4adc..b4517095db6a 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,7 +180,7 @@ 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,
+static void __static_key_slow_dec_cpuslocked(struct static_key *key,
 					   unsigned long rate_limit,
 					   struct delayed_work *work)
 {
@@ -211,7 +211,7 @@ static void __static_key_slow_dec(struct static_key *key,
 				  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();
 }
 
@@ -229,6 +229,12 @@ void static_key_slow_dec(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_slow_dec);
 
+void static_key_slow_dec_cpuslocked(struct static_key *key)
+{
+	STATIC_KEY_CHECK_USE(key);
+	__static_key_slow_dec_cpuslocked(key, 0, NULL);
+}
+
 void static_key_slow_dec_deferred(struct static_key_deferred *key)
 {
 	STATIC_KEY_CHECK_USE(key);
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1070803cb423..7b6535987500 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4361,12 +4361,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)

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: Spurious lockdep splat in v4.15-rc9
  2018-01-22 21:53 ` Peter Zijlstra
@ 2018-01-22 22:03   ` Tejun Heo
  2018-01-24 10:38   ` [tip:sched/urgent] sched/core: Fix cpu.max vs. cpuhotplug deadlock tip-bot for Peter Zijlstra
  1 sibling, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2018-01-22 22:03 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Ingo Molnar, linux-kernel, Thomas Gleixner

Hello, Peter.

On Mon, Jan 22, 2018 at 10:53:29PM +0100, Peter Zijlstra wrote:
> 
> Tejun, does the below work for you (compile tested only).

Yeap, that gets rid of the lockdep warning.

Thanks a lot.

-- 
tejun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip:sched/urgent] sched/core: Fix cpu.max vs. cpuhotplug deadlock
  2018-01-22 21:53 ` Peter Zijlstra
  2018-01-22 22:03   ` Tejun Heo
@ 2018-01-24 10:38   ` tip-bot for Peter Zijlstra
  1 sibling, 0 replies; 5+ messages in thread
From: tip-bot for Peter Zijlstra @ 2018-01-24 10:38 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: tglx, tj, peterz, hpa, linux-kernel, torvalds, mingo

Commit-ID:  ce48c146495a1a50e48cdbfbfaba3e708be7c07c
Gitweb:     https://git.kernel.org/tip/ce48c146495a1a50e48cdbfbfaba3e708be7c07c
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Mon, 22 Jan 2018 22:53:28 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 24 Jan 2018 10:03:44 +0100

sched/core: Fix cpu.max vs. cpuhotplug deadlock

Tejun reported the following cpu-hotplug lock (percpu-rwsem) read recursion:

  tg_set_cfs_bandwidth()
    get_online_cpus()
      cpus_read_lock()

    cfs_bandwidth_usage_inc()
      static_key_slow_inc()
        cpus_read_lock()

Reported-by: Tejun Heo <tj@kernel.org>
Tested-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20180122215328.GP3397@worktop
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/jump_label.h |  7 +++++++
 kernel/jump_label.c        | 12 +++++++++---
 kernel/sched/fair.c        |  4 ++--
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index c7b368c..e0340ca 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,9 @@ static inline void static_key_slow_dec(struct static_key *key)
 	atomic_dec(&key->enabled);
 }
 
+#define static_key_slow_inc_cpuslocked(key) static_key_slow_inc(key)
+#define static_key_slow_dec_cpuslocked(key) static_key_slow_dec(key)
+
 static inline int jump_label_text_reserved(void *start, void *end)
 {
 	return 0;
@@ -416,6 +421,8 @@ extern bool ____wrong_branch_error(void);
 
 #define static_branch_inc(x)		static_key_slow_inc(&(x)->key)
 #define static_branch_dec(x)		static_key_slow_dec(&(x)->key)
+#define static_branch_inc_cpuslocked(x)	static_key_slow_inc_cpuslocked(&(x)->key)
+#define static_branch_dec_cpuslocked(x)	static_key_slow_dec_cpuslocked(&(x)->key)
 
 /*
  * Normal usage; boolean enable/disable.
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 8594d24..b451709 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,7 +180,7 @@ 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,
+static void __static_key_slow_dec_cpuslocked(struct static_key *key,
 					   unsigned long rate_limit,
 					   struct delayed_work *work)
 {
@@ -211,7 +211,7 @@ static void __static_key_slow_dec(struct static_key *key,
 				  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();
 }
 
@@ -229,6 +229,12 @@ void static_key_slow_dec(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_slow_dec);
 
+void static_key_slow_dec_cpuslocked(struct static_key *key)
+{
+	STATIC_KEY_CHECK_USE(key);
+	__static_key_slow_dec_cpuslocked(key, 0, NULL);
+}
+
 void static_key_slow_dec_deferred(struct static_key_deferred *key)
 {
 	STATIC_KEY_CHECK_USE(key);
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 2fe3aa8..26a71eb 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)

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-01-24 10:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-22 17:10 Spurious lockdep splat in v4.15-rc9 Tejun Heo
2018-01-22 17:40 ` Peter Zijlstra
2018-01-22 21:53 ` Peter Zijlstra
2018-01-22 22:03   ` Tejun Heo
2018-01-24 10:38   ` [tip:sched/urgent] sched/core: Fix cpu.max vs. cpuhotplug deadlock tip-bot for Peter Zijlstra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.