LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v5 0/3] locking/rwsem: Add DEBUG_RWSEMS & restructure lock debugging menu
@ 2018-03-30 21:27 Waiman Long
  2018-03-30 21:27 ` [PATCH v5 1/3] locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches Waiman Long
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Waiman Long @ 2018-03-30 21:27 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Thomas Gleixner
  Cc: linux-kernel, Davidlohr Bueso, Waiman Long

v5:
 - Break patch 2 into 2 separate patches to make them easier to review.

v4:
 - Auto select DEBUG_RWSEMS & DEBUG_WW_MUTEX_SLOWPATH if PROVE_LOCKING
 - Modify patch 2 to do a restructuring of the lock debugging menu.

v3:
 - Add a new patch which adds a master LOCK_DEBUGGING option to turn
   on all lock debugging.

v2:
 - Fix typo in up_read_non_owner().

This patchset enhances lock debugging by adding rwsem lock/unlock
mismatches checking and restructure the lock debugging menu to make it
easier to select options that are likely to be most frequently used.

Waiman Long (3):
  locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches
  lib/Kconfig.debug: Add LOCK_DEBUGGING_SUPPORT to make it more readable
  lib/Kconfig.debug: Restructure the lock debugging menu

 kernel/locking/rwsem.c |   4 ++
 kernel/locking/rwsem.h |   8 ++-
 lib/Kconfig.debug      | 150 +++++++++++++++++++++++++++----------------------
 3 files changed, 93 insertions(+), 69 deletions(-)

-- 
1.8.3.1

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

* [PATCH v5 1/3] locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches
  2018-03-30 21:27 [PATCH v5 0/3] locking/rwsem: Add DEBUG_RWSEMS & restructure lock debugging menu Waiman Long
@ 2018-03-30 21:27 ` Waiman Long
  2018-03-31  7:59   ` [tip:locking/core] " tip-bot for Waiman Long
  2018-04-11 14:21   ` [PATCH v5 1/3] " Arnd Bergmann
  2018-03-30 21:27 ` [PATCH v5 2/3] lib/Kconfig.debug: Add LOCK_DEBUGGING_SUPPORT to make it more readable Waiman Long
  2018-03-30 21:28 ` [PATCH v5 3/3] lib/Kconfig.debug: Restructure the lock debugging menu Waiman Long
  2 siblings, 2 replies; 9+ messages in thread
From: Waiman Long @ 2018-03-30 21:27 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Thomas Gleixner
  Cc: linux-kernel, Davidlohr Bueso, Waiman Long

For a rwsem, locking can either be exclusive or shared. The corresponding
exclusive or shared unlock must be used. Otherwise, the protected data
structures may get corrupted or the lock may be in an inconsistent state.

In order to detect such anomaly, a new configuration option DEBUG_RWSEMS
is added which can be enabled to look for such mismatches and print
warnings that that happens.

Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Davidlohr Bueso <dave@stgolabs.net>
---
 kernel/locking/rwsem.c | 4 ++++
 kernel/locking/rwsem.h | 8 +++++++-
 lib/Kconfig.debug      | 8 ++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index f549c55..30465a2 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -117,6 +117,7 @@ int down_write_trylock(struct rw_semaphore *sem)
 void up_read(struct rw_semaphore *sem)
 {
 	rwsem_release(&sem->dep_map, 1, _RET_IP_);
+	DEBUG_RWSEMS_WARN_ON(sem->owner != RWSEM_READER_OWNED);
 
 	__up_read(sem);
 }
@@ -129,6 +130,7 @@ void up_read(struct rw_semaphore *sem)
 void up_write(struct rw_semaphore *sem)
 {
 	rwsem_release(&sem->dep_map, 1, _RET_IP_);
+	DEBUG_RWSEMS_WARN_ON(sem->owner != current);
 
 	rwsem_clear_owner(sem);
 	__up_write(sem);
@@ -142,6 +144,7 @@ void up_write(struct rw_semaphore *sem)
 void downgrade_write(struct rw_semaphore *sem)
 {
 	lock_downgrade(&sem->dep_map, _RET_IP_);
+	DEBUG_RWSEMS_WARN_ON(sem->owner != current);
 
 	rwsem_set_reader_owned(sem);
 	__downgrade_write(sem);
@@ -211,6 +214,7 @@ int __sched down_write_killable_nested(struct rw_semaphore *sem, int subclass)
 
 void up_read_non_owner(struct rw_semaphore *sem)
 {
+	DEBUG_RWSEMS_WARN_ON(sem->owner != RWSEM_READER_OWNED);
 	__up_read(sem);
 }
 
diff --git a/kernel/locking/rwsem.h b/kernel/locking/rwsem.h
index a883b8f..563a7bc 100644
--- a/kernel/locking/rwsem.h
+++ b/kernel/locking/rwsem.h
@@ -16,6 +16,12 @@
  */
 #define RWSEM_READER_OWNED	((struct task_struct *)1UL)
 
+#ifdef CONFIG_DEBUG_RWSEMS
+#define DEBUG_RWSEMS_WARN_ON(c)	DEBUG_LOCKS_WARN_ON(c)
+#else
+#define DEBUG_RWSEMS_WARN_ON(c)
+#endif
+
 #ifdef CONFIG_RWSEM_SPIN_ON_OWNER
 /*
  * All writes to owner are protected by WRITE_ONCE() to make sure that
@@ -41,7 +47,7 @@ static inline void rwsem_set_reader_owned(struct rw_semaphore *sem)
 	 * do a write to the rwsem cacheline when it is really necessary
 	 * to minimize cacheline contention.
 	 */
-	if (sem->owner != RWSEM_READER_OWNED)
+	if (READ_ONCE(sem->owner) != RWSEM_READER_OWNED)
 		WRITE_ONCE(sem->owner, RWSEM_READER_OWNED);
 }
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 64155e3..6aad28c 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1075,6 +1075,13 @@ config DEBUG_WW_MUTEX_SLOWPATH
 	 even a debug kernel.  If you are a driver writer, enable it.  If
 	 you are a distro, do not.
 
+config DEBUG_RWSEMS
+	bool "RW Semaphore debugging: basic checks"
+	depends on DEBUG_KERNEL && RWSEM_SPIN_ON_OWNER
+	help
+	  This feature allows mismatched rw semaphore locks and unlocks
+	  to be detected and reported.
+
 config DEBUG_LOCK_ALLOC
 	bool "Lock debugging: detect incorrect freeing of live locks"
 	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
@@ -1097,6 +1104,7 @@ config PROVE_LOCKING
 	select DEBUG_SPINLOCK
 	select DEBUG_MUTEXES
 	select DEBUG_RT_MUTEXES if RT_MUTEXES
+	select DEBUG_RWSEMS if RWSEM_SPIN_ON_OWNER
 	select DEBUG_LOCK_ALLOC
 	select TRACE_IRQFLAGS
 	default n
-- 
1.8.3.1

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

* [PATCH v5 2/3] lib/Kconfig.debug: Add LOCK_DEBUGGING_SUPPORT to make it more readable
  2018-03-30 21:27 [PATCH v5 0/3] locking/rwsem: Add DEBUG_RWSEMS & restructure lock debugging menu Waiman Long
  2018-03-30 21:27 ` [PATCH v5 1/3] locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches Waiman Long
@ 2018-03-30 21:27 ` Waiman Long
  2018-03-31  7:59   ` [tip:locking/core] locking/Kconfig: " tip-bot for Waiman Long
  2018-03-30 21:28 ` [PATCH v5 3/3] lib/Kconfig.debug: Restructure the lock debugging menu Waiman Long
  2 siblings, 1 reply; 9+ messages in thread
From: Waiman Long @ 2018-03-30 21:27 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Thomas Gleixner
  Cc: linux-kernel, Davidlohr Bueso, Waiman Long

There are a couples of lock debugging Kconfig options that depends on
the following support options:
 - TRACE_IRQFLAGS_SUPPORT
 - STACKTRACE_SUPPORT
 - LOCKDEP_SUPPORT

That makes those lock debugging options harder to read and understand.
So a new LOCK_DEBUGGING_SUPPORT option is added that is equivalent to
the above three options together. That makes the Kconfig.debug file
more readable.

Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Davidlohr Bueso <dave@stgolabs.net>
---
 lib/Kconfig.debug | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 6aad28c..0db85f5 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1034,6 +1034,11 @@ config DEBUG_PREEMPT
 
 menu "Lock Debugging (spinlocks, mutexes, etc...)"
 
+config LOCK_DEBUGGING_SUPPORT
+	bool
+	depends on TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	default y
+
 config DEBUG_RT_MUTEXES
 	bool "RT Mutex debugging, deadlock detection"
 	depends on DEBUG_KERNEL && RT_MUTEXES
@@ -1060,7 +1065,7 @@ config DEBUG_MUTEXES
 
 config DEBUG_WW_MUTEX_SLOWPATH
 	bool "Wait/wound mutex debugging: Slowpath testing"
-	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
 	select DEBUG_LOCK_ALLOC
 	select DEBUG_SPINLOCK
 	select DEBUG_MUTEXES
@@ -1084,7 +1089,7 @@ config DEBUG_RWSEMS
 
 config DEBUG_LOCK_ALLOC
 	bool "Lock debugging: detect incorrect freeing of live locks"
-	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
 	select DEBUG_SPINLOCK
 	select DEBUG_MUTEXES
 	select DEBUG_RT_MUTEXES if RT_MUTEXES
@@ -1099,7 +1104,7 @@ config DEBUG_LOCK_ALLOC
 
 config PROVE_LOCKING
 	bool "Lock debugging: prove locking correctness"
-	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
 	select LOCKDEP
 	select DEBUG_SPINLOCK
 	select DEBUG_MUTEXES
@@ -1144,7 +1149,7 @@ config PROVE_LOCKING
 
 config LOCKDEP
 	bool
-	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
 	select STACKTRACE
 	select FRAME_POINTER if !MIPS && !PPC && !ARM_UNWIND && !S390 && !MICROBLAZE && !ARC && !SCORE && !X86
 	select KALLSYMS
@@ -1155,7 +1160,7 @@ config LOCKDEP_SMALL
 
 config LOCK_STAT
 	bool "Lock usage statistics"
-	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
 	select LOCKDEP
 	select DEBUG_SPINLOCK
 	select DEBUG_MUTEXES
-- 
1.8.3.1

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

* [PATCH v5 3/3] lib/Kconfig.debug: Restructure the lock debugging menu
  2018-03-30 21:27 [PATCH v5 0/3] locking/rwsem: Add DEBUG_RWSEMS & restructure lock debugging menu Waiman Long
  2018-03-30 21:27 ` [PATCH v5 1/3] locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches Waiman Long
  2018-03-30 21:27 ` [PATCH v5 2/3] lib/Kconfig.debug: Add LOCK_DEBUGGING_SUPPORT to make it more readable Waiman Long
@ 2018-03-30 21:28 ` Waiman Long
  2018-03-31  8:00   ` [tip:locking/core] locking/Kconfig: " tip-bot for Waiman Long
  2 siblings, 1 reply; 9+ messages in thread
From: Waiman Long @ 2018-03-30 21:28 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Thomas Gleixner
  Cc: linux-kernel, Davidlohr Bueso, Waiman Long

Two config options in the lock debugging menu that are probably the most
frequently used, as far as I am concerned, is the PROVE_LOCKING and
LOCK_STAT. From a UI perspective, they should be front and center. So
these two options are now moved to the top of the lock debugging menu.

The DEBUG_WW_MUTEX_SLOWPATH option is also added to the PROVE_LOCKING
umbrella.

Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Davidlohr Bueso <dave@stgolabs.net>
---
 lib/Kconfig.debug | 135 +++++++++++++++++++++++++++---------------------------
 1 file changed, 68 insertions(+), 67 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 0db85f5..dc9ffe2 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1039,6 +1039,74 @@ config LOCK_DEBUGGING_SUPPORT
 	depends on TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
 	default y
 
+config PROVE_LOCKING
+	bool "Lock debugging: prove locking correctness"
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
+	select LOCKDEP
+	select DEBUG_SPINLOCK
+	select DEBUG_MUTEXES
+	select DEBUG_RT_MUTEXES if RT_MUTEXES
+	select DEBUG_RWSEMS if RWSEM_SPIN_ON_OWNER
+	select DEBUG_WW_MUTEX_SLOWPATH
+	select DEBUG_LOCK_ALLOC
+	select TRACE_IRQFLAGS
+	default n
+	help
+	 This feature enables the kernel to prove that all locking
+	 that occurs in the kernel runtime is mathematically
+	 correct: that under no circumstance could an arbitrary (and
+	 not yet triggered) combination of observed locking
+	 sequences (on an arbitrary number of CPUs, running an
+	 arbitrary number of tasks and interrupt contexts) cause a
+	 deadlock.
+
+	 In short, this feature enables the kernel to report locking
+	 related deadlocks before they actually occur.
+
+	 The proof does not depend on how hard and complex a
+	 deadlock scenario would be to trigger: how many
+	 participant CPUs, tasks and irq-contexts would be needed
+	 for it to trigger. The proof also does not depend on
+	 timing: if a race and a resulting deadlock is possible
+	 theoretically (no matter how unlikely the race scenario
+	 is), it will be proven so and will immediately be
+	 reported by the kernel (once the event is observed that
+	 makes the deadlock theoretically possible).
+
+	 If a deadlock is impossible (i.e. the locking rules, as
+	 observed by the kernel, are mathematically correct), the
+	 kernel reports nothing.
+
+	 NOTE: this feature can also be enabled for rwlocks, mutexes
+	 and rwsems - in which case all dependencies between these
+	 different locking variants are observed and mapped too, and
+	 the proof of observed correctness is also maintained for an
+	 arbitrary combination of these separate locking variants.
+
+	 For more details, see Documentation/locking/lockdep-design.txt.
+
+config LOCK_STAT
+	bool "Lock usage statistics"
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
+	select LOCKDEP
+	select DEBUG_SPINLOCK
+	select DEBUG_MUTEXES
+	select DEBUG_RT_MUTEXES if RT_MUTEXES
+	select DEBUG_LOCK_ALLOC
+	default n
+	help
+	 This feature enables tracking lock contention points
+
+	 For more details, see Documentation/locking/lockstat.txt
+
+	 This also enables lock events required by "perf lock",
+	 subcommand of perf.
+	 If you want to use "perf lock", you also need to turn on
+	 CONFIG_EVENT_TRACING.
+
+	 CONFIG_LOCK_STAT defines "contended" and "acquired" lock events.
+	 (CONFIG_LOCKDEP defines "acquire" and "release" events.)
+
 config DEBUG_RT_MUTEXES
 	bool "RT Mutex debugging, deadlock detection"
 	depends on DEBUG_KERNEL && RT_MUTEXES
@@ -1102,51 +1170,6 @@ config DEBUG_LOCK_ALLOC
 	 spin_lock_init()/mutex_init()/etc., or whether there is any lock
 	 held during task exit.
 
-config PROVE_LOCKING
-	bool "Lock debugging: prove locking correctness"
-	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
-	select LOCKDEP
-	select DEBUG_SPINLOCK
-	select DEBUG_MUTEXES
-	select DEBUG_RT_MUTEXES if RT_MUTEXES
-	select DEBUG_RWSEMS if RWSEM_SPIN_ON_OWNER
-	select DEBUG_LOCK_ALLOC
-	select TRACE_IRQFLAGS
-	default n
-	help
-	 This feature enables the kernel to prove that all locking
-	 that occurs in the kernel runtime is mathematically
-	 correct: that under no circumstance could an arbitrary (and
-	 not yet triggered) combination of observed locking
-	 sequences (on an arbitrary number of CPUs, running an
-	 arbitrary number of tasks and interrupt contexts) cause a
-	 deadlock.
-
-	 In short, this feature enables the kernel to report locking
-	 related deadlocks before they actually occur.
-
-	 The proof does not depend on how hard and complex a
-	 deadlock scenario would be to trigger: how many
-	 participant CPUs, tasks and irq-contexts would be needed
-	 for it to trigger. The proof also does not depend on
-	 timing: if a race and a resulting deadlock is possible
-	 theoretically (no matter how unlikely the race scenario
-	 is), it will be proven so and will immediately be
-	 reported by the kernel (once the event is observed that
-	 makes the deadlock theoretically possible).
-
-	 If a deadlock is impossible (i.e. the locking rules, as
-	 observed by the kernel, are mathematically correct), the
-	 kernel reports nothing.
-
-	 NOTE: this feature can also be enabled for rwlocks, mutexes
-	 and rwsems - in which case all dependencies between these
-	 different locking variants are observed and mapped too, and
-	 the proof of observed correctness is also maintained for an
-	 arbitrary combination of these separate locking variants.
-
-	 For more details, see Documentation/locking/lockdep-design.txt.
-
 config LOCKDEP
 	bool
 	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
@@ -1158,28 +1181,6 @@ config LOCKDEP
 config LOCKDEP_SMALL
 	bool
 
-config LOCK_STAT
-	bool "Lock usage statistics"
-	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
-	select LOCKDEP
-	select DEBUG_SPINLOCK
-	select DEBUG_MUTEXES
-	select DEBUG_RT_MUTEXES if RT_MUTEXES
-	select DEBUG_LOCK_ALLOC
-	default n
-	help
-	 This feature enables tracking lock contention points
-
-	 For more details, see Documentation/locking/lockstat.txt
-
-	 This also enables lock events required by "perf lock",
-	 subcommand of perf.
-	 If you want to use "perf lock", you also need to turn on
-	 CONFIG_EVENT_TRACING.
-
-	 CONFIG_LOCK_STAT defines "contended" and "acquired" lock events.
-	 (CONFIG_LOCKDEP defines "acquire" and "release" events.)
-
 config DEBUG_LOCKDEP
 	bool "Lock dependency engine debugging"
 	depends on DEBUG_KERNEL && LOCKDEP
-- 
1.8.3.1

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

* [tip:locking/core] locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches
  2018-03-30 21:27 ` [PATCH v5 1/3] locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches Waiman Long
@ 2018-03-31  7:59   ` tip-bot for Waiman Long
  2018-04-11 14:21   ` [PATCH v5 1/3] " Arnd Bergmann
  1 sibling, 0 replies; 9+ messages in thread
From: tip-bot for Waiman Long @ 2018-03-31  7:59 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, longman, dave, akpm, paulmck, peterz, hpa,
	torvalds, tglx, mingo

Commit-ID:  5149cbac4235e12a34cf089592a8bd1c9fcfa467
Gitweb:     https://git.kernel.org/tip/5149cbac4235e12a34cf089592a8bd1c9fcfa467
Author:     Waiman Long <longman@redhat.com>
AuthorDate: Fri, 30 Mar 2018 17:27:58 -0400
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 31 Mar 2018 07:30:50 +0200

locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches

For a rwsem, locking can either be exclusive or shared. The corresponding
exclusive or shared unlock must be used. Otherwise, the protected data
structures may get corrupted or the lock may be in an inconsistent state.

In order to detect such anomaly, a new configuration option DEBUG_RWSEMS
is added which can be enabled to look for such mismatches and print
warnings that that happens.

Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Davidlohr Bueso <dave@stgolabs.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1522445280-7767-2-git-send-email-longman@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/locking/rwsem.c | 4 ++++
 kernel/locking/rwsem.h | 8 +++++++-
 lib/Kconfig.debug      | 8 ++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index f549c552dbf1..30465a2f2b6c 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -117,6 +117,7 @@ EXPORT_SYMBOL(down_write_trylock);
 void up_read(struct rw_semaphore *sem)
 {
 	rwsem_release(&sem->dep_map, 1, _RET_IP_);
+	DEBUG_RWSEMS_WARN_ON(sem->owner != RWSEM_READER_OWNED);
 
 	__up_read(sem);
 }
@@ -129,6 +130,7 @@ EXPORT_SYMBOL(up_read);
 void up_write(struct rw_semaphore *sem)
 {
 	rwsem_release(&sem->dep_map, 1, _RET_IP_);
+	DEBUG_RWSEMS_WARN_ON(sem->owner != current);
 
 	rwsem_clear_owner(sem);
 	__up_write(sem);
@@ -142,6 +144,7 @@ EXPORT_SYMBOL(up_write);
 void downgrade_write(struct rw_semaphore *sem)
 {
 	lock_downgrade(&sem->dep_map, _RET_IP_);
+	DEBUG_RWSEMS_WARN_ON(sem->owner != current);
 
 	rwsem_set_reader_owned(sem);
 	__downgrade_write(sem);
@@ -211,6 +214,7 @@ EXPORT_SYMBOL(down_write_killable_nested);
 
 void up_read_non_owner(struct rw_semaphore *sem)
 {
+	DEBUG_RWSEMS_WARN_ON(sem->owner != RWSEM_READER_OWNED);
 	__up_read(sem);
 }
 
diff --git a/kernel/locking/rwsem.h b/kernel/locking/rwsem.h
index a883b8f1fdc6..a17cba8d94bb 100644
--- a/kernel/locking/rwsem.h
+++ b/kernel/locking/rwsem.h
@@ -16,6 +16,12 @@
  */
 #define RWSEM_READER_OWNED	((struct task_struct *)1UL)
 
+#ifdef CONFIG_DEBUG_RWSEMS
+# define DEBUG_RWSEMS_WARN_ON(c)	DEBUG_LOCKS_WARN_ON(c)
+#else
+# define DEBUG_RWSEMS_WARN_ON(c)
+#endif
+
 #ifdef CONFIG_RWSEM_SPIN_ON_OWNER
 /*
  * All writes to owner are protected by WRITE_ONCE() to make sure that
@@ -41,7 +47,7 @@ static inline void rwsem_set_reader_owned(struct rw_semaphore *sem)
 	 * do a write to the rwsem cacheline when it is really necessary
 	 * to minimize cacheline contention.
 	 */
-	if (sem->owner != RWSEM_READER_OWNED)
+	if (READ_ONCE(sem->owner) != RWSEM_READER_OWNED)
 		WRITE_ONCE(sem->owner, RWSEM_READER_OWNED);
 }
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 64155e310a9f..88650ab5cedb 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1075,6 +1075,13 @@ config DEBUG_WW_MUTEX_SLOWPATH
 	 even a debug kernel.  If you are a driver writer, enable it.  If
 	 you are a distro, do not.
 
+config DEBUG_RWSEMS
+	bool "RW Semaphore debugging: basic checks"
+	depends on DEBUG_KERNEL && RWSEM_SPIN_ON_OWNER
+	help
+	  This debugging feature allows mismatched rw semaphore locks and unlocks
+	  to be detected and reported.
+
 config DEBUG_LOCK_ALLOC
 	bool "Lock debugging: detect incorrect freeing of live locks"
 	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
@@ -1097,6 +1104,7 @@ config PROVE_LOCKING
 	select DEBUG_SPINLOCK
 	select DEBUG_MUTEXES
 	select DEBUG_RT_MUTEXES if RT_MUTEXES
+	select DEBUG_RWSEMS if RWSEM_SPIN_ON_OWNER
 	select DEBUG_LOCK_ALLOC
 	select TRACE_IRQFLAGS
 	default n

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

* [tip:locking/core] locking/Kconfig: Add LOCK_DEBUGGING_SUPPORT to make it more readable
  2018-03-30 21:27 ` [PATCH v5 2/3] lib/Kconfig.debug: Add LOCK_DEBUGGING_SUPPORT to make it more readable Waiman Long
@ 2018-03-31  7:59   ` tip-bot for Waiman Long
  0 siblings, 0 replies; 9+ messages in thread
From: tip-bot for Waiman Long @ 2018-03-31  7:59 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, linux-kernel, akpm, peterz, paulmck, hpa, dave, tglx,
	torvalds, longman

Commit-ID:  f07cbebb6daf04e5c9721e5be2737a6068c7e2a2
Gitweb:     https://git.kernel.org/tip/f07cbebb6daf04e5c9721e5be2737a6068c7e2a2
Author:     Waiman Long <longman@redhat.com>
AuthorDate: Fri, 30 Mar 2018 17:27:59 -0400
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 31 Mar 2018 07:30:50 +0200

locking/Kconfig: Add LOCK_DEBUGGING_SUPPORT to make it more readable

There are a couples of lock debugging Kconfig options that depends on
the following support options:

 - TRACE_IRQFLAGS_SUPPORT
 - STACKTRACE_SUPPORT
 - LOCKDEP_SUPPORT

That makes those lock debugging options harder to read and understand.
So a new LOCK_DEBUGGING_SUPPORT option is added that is equivalent to
the above three options together. That makes the Kconfig.debug file
more readable.

Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Davidlohr Bueso <dave@stgolabs.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1522445280-7767-3-git-send-email-longman@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 lib/Kconfig.debug | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 88650ab5cedb..ee7ca42e737e 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1034,6 +1034,11 @@ config DEBUG_PREEMPT
 
 menu "Lock Debugging (spinlocks, mutexes, etc...)"
 
+config LOCK_DEBUGGING_SUPPORT
+	bool
+	depends on TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	default y
+
 config DEBUG_RT_MUTEXES
 	bool "RT Mutex debugging, deadlock detection"
 	depends on DEBUG_KERNEL && RT_MUTEXES
@@ -1060,7 +1065,7 @@ config DEBUG_MUTEXES
 
 config DEBUG_WW_MUTEX_SLOWPATH
 	bool "Wait/wound mutex debugging: Slowpath testing"
-	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
 	select DEBUG_LOCK_ALLOC
 	select DEBUG_SPINLOCK
 	select DEBUG_MUTEXES
@@ -1084,7 +1089,7 @@ config DEBUG_RWSEMS
 
 config DEBUG_LOCK_ALLOC
 	bool "Lock debugging: detect incorrect freeing of live locks"
-	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
 	select DEBUG_SPINLOCK
 	select DEBUG_MUTEXES
 	select DEBUG_RT_MUTEXES if RT_MUTEXES
@@ -1099,7 +1104,7 @@ config DEBUG_LOCK_ALLOC
 
 config PROVE_LOCKING
 	bool "Lock debugging: prove locking correctness"
-	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
 	select LOCKDEP
 	select DEBUG_SPINLOCK
 	select DEBUG_MUTEXES
@@ -1144,7 +1149,7 @@ config PROVE_LOCKING
 
 config LOCKDEP
 	bool
-	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
 	select STACKTRACE
 	select FRAME_POINTER if !MIPS && !PPC && !ARM_UNWIND && !S390 && !MICROBLAZE && !ARC && !SCORE && !X86
 	select KALLSYMS
@@ -1155,7 +1160,7 @@ config LOCKDEP_SMALL
 
 config LOCK_STAT
 	bool "Lock usage statistics"
-	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
 	select LOCKDEP
 	select DEBUG_SPINLOCK
 	select DEBUG_MUTEXES

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

* [tip:locking/core] locking/Kconfig: Restructure the lock debugging menu
  2018-03-30 21:28 ` [PATCH v5 3/3] lib/Kconfig.debug: Restructure the lock debugging menu Waiman Long
@ 2018-03-31  8:00   ` tip-bot for Waiman Long
  0 siblings, 0 replies; 9+ messages in thread
From: tip-bot for Waiman Long @ 2018-03-31  8:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, linux-kernel, paulmck, mingo, akpm, hpa, dave, torvalds,
	longman, tglx

Commit-ID:  19193bcad8dced863f2f720b1a76110bda07c970
Gitweb:     https://git.kernel.org/tip/19193bcad8dced863f2f720b1a76110bda07c970
Author:     Waiman Long <longman@redhat.com>
AuthorDate: Fri, 30 Mar 2018 17:28:00 -0400
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 31 Mar 2018 07:30:51 +0200

locking/Kconfig: Restructure the lock debugging menu

Two config options in the lock debugging menu that are probably the most
frequently used, as far as I am concerned, is the PROVE_LOCKING and
LOCK_STAT. From a UI perspective, they should be front and center. So
these two options are now moved to the top of the lock debugging menu.

The DEBUG_WW_MUTEX_SLOWPATH option is also added to the PROVE_LOCKING
umbrella.

Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Davidlohr Bueso <dave@stgolabs.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1522445280-7767-4-git-send-email-longman@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 lib/Kconfig.debug | 135 +++++++++++++++++++++++++++---------------------------
 1 file changed, 68 insertions(+), 67 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ee7ca42e737e..4f7b3a11eb4d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1039,6 +1039,74 @@ config LOCK_DEBUGGING_SUPPORT
 	depends on TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
 	default y
 
+config PROVE_LOCKING
+	bool "Lock debugging: prove locking correctness"
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
+	select LOCKDEP
+	select DEBUG_SPINLOCK
+	select DEBUG_MUTEXES
+	select DEBUG_RT_MUTEXES if RT_MUTEXES
+	select DEBUG_RWSEMS if RWSEM_SPIN_ON_OWNER
+	select DEBUG_WW_MUTEX_SLOWPATH
+	select DEBUG_LOCK_ALLOC
+	select TRACE_IRQFLAGS
+	default n
+	help
+	 This feature enables the kernel to prove that all locking
+	 that occurs in the kernel runtime is mathematically
+	 correct: that under no circumstance could an arbitrary (and
+	 not yet triggered) combination of observed locking
+	 sequences (on an arbitrary number of CPUs, running an
+	 arbitrary number of tasks and interrupt contexts) cause a
+	 deadlock.
+
+	 In short, this feature enables the kernel to report locking
+	 related deadlocks before they actually occur.
+
+	 The proof does not depend on how hard and complex a
+	 deadlock scenario would be to trigger: how many
+	 participant CPUs, tasks and irq-contexts would be needed
+	 for it to trigger. The proof also does not depend on
+	 timing: if a race and a resulting deadlock is possible
+	 theoretically (no matter how unlikely the race scenario
+	 is), it will be proven so and will immediately be
+	 reported by the kernel (once the event is observed that
+	 makes the deadlock theoretically possible).
+
+	 If a deadlock is impossible (i.e. the locking rules, as
+	 observed by the kernel, are mathematically correct), the
+	 kernel reports nothing.
+
+	 NOTE: this feature can also be enabled for rwlocks, mutexes
+	 and rwsems - in which case all dependencies between these
+	 different locking variants are observed and mapped too, and
+	 the proof of observed correctness is also maintained for an
+	 arbitrary combination of these separate locking variants.
+
+	 For more details, see Documentation/locking/lockdep-design.txt.
+
+config LOCK_STAT
+	bool "Lock usage statistics"
+	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
+	select LOCKDEP
+	select DEBUG_SPINLOCK
+	select DEBUG_MUTEXES
+	select DEBUG_RT_MUTEXES if RT_MUTEXES
+	select DEBUG_LOCK_ALLOC
+	default n
+	help
+	 This feature enables tracking lock contention points
+
+	 For more details, see Documentation/locking/lockstat.txt
+
+	 This also enables lock events required by "perf lock",
+	 subcommand of perf.
+	 If you want to use "perf lock", you also need to turn on
+	 CONFIG_EVENT_TRACING.
+
+	 CONFIG_LOCK_STAT defines "contended" and "acquired" lock events.
+	 (CONFIG_LOCKDEP defines "acquire" and "release" events.)
+
 config DEBUG_RT_MUTEXES
 	bool "RT Mutex debugging, deadlock detection"
 	depends on DEBUG_KERNEL && RT_MUTEXES
@@ -1102,51 +1170,6 @@ config DEBUG_LOCK_ALLOC
 	 spin_lock_init()/mutex_init()/etc., or whether there is any lock
 	 held during task exit.
 
-config PROVE_LOCKING
-	bool "Lock debugging: prove locking correctness"
-	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
-	select LOCKDEP
-	select DEBUG_SPINLOCK
-	select DEBUG_MUTEXES
-	select DEBUG_RT_MUTEXES if RT_MUTEXES
-	select DEBUG_RWSEMS if RWSEM_SPIN_ON_OWNER
-	select DEBUG_LOCK_ALLOC
-	select TRACE_IRQFLAGS
-	default n
-	help
-	 This feature enables the kernel to prove that all locking
-	 that occurs in the kernel runtime is mathematically
-	 correct: that under no circumstance could an arbitrary (and
-	 not yet triggered) combination of observed locking
-	 sequences (on an arbitrary number of CPUs, running an
-	 arbitrary number of tasks and interrupt contexts) cause a
-	 deadlock.
-
-	 In short, this feature enables the kernel to report locking
-	 related deadlocks before they actually occur.
-
-	 The proof does not depend on how hard and complex a
-	 deadlock scenario would be to trigger: how many
-	 participant CPUs, tasks and irq-contexts would be needed
-	 for it to trigger. The proof also does not depend on
-	 timing: if a race and a resulting deadlock is possible
-	 theoretically (no matter how unlikely the race scenario
-	 is), it will be proven so and will immediately be
-	 reported by the kernel (once the event is observed that
-	 makes the deadlock theoretically possible).
-
-	 If a deadlock is impossible (i.e. the locking rules, as
-	 observed by the kernel, are mathematically correct), the
-	 kernel reports nothing.
-
-	 NOTE: this feature can also be enabled for rwlocks, mutexes
-	 and rwsems - in which case all dependencies between these
-	 different locking variants are observed and mapped too, and
-	 the proof of observed correctness is also maintained for an
-	 arbitrary combination of these separate locking variants.
-
-	 For more details, see Documentation/locking/lockdep-design.txt.
-
 config LOCKDEP
 	bool
 	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
@@ -1158,28 +1181,6 @@ config LOCKDEP
 config LOCKDEP_SMALL
 	bool
 
-config LOCK_STAT
-	bool "Lock usage statistics"
-	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
-	select LOCKDEP
-	select DEBUG_SPINLOCK
-	select DEBUG_MUTEXES
-	select DEBUG_RT_MUTEXES if RT_MUTEXES
-	select DEBUG_LOCK_ALLOC
-	default n
-	help
-	 This feature enables tracking lock contention points
-
-	 For more details, see Documentation/locking/lockstat.txt
-
-	 This also enables lock events required by "perf lock",
-	 subcommand of perf.
-	 If you want to use "perf lock", you also need to turn on
-	 CONFIG_EVENT_TRACING.
-
-	 CONFIG_LOCK_STAT defines "contended" and "acquired" lock events.
-	 (CONFIG_LOCKDEP defines "acquire" and "release" events.)
-
 config DEBUG_LOCKDEP
 	bool "Lock dependency engine debugging"
 	depends on DEBUG_KERNEL && LOCKDEP

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

* Re: [PATCH v5 1/3] locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches
  2018-03-30 21:27 ` [PATCH v5 1/3] locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches Waiman Long
  2018-03-31  7:59   ` [tip:locking/core] " tip-bot for Waiman Long
@ 2018-04-11 14:21   ` Arnd Bergmann
  2018-05-23  1:46     ` Dan Williams
  1 sibling, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2018-04-11 14:21 UTC (permalink / raw)
  To: Waiman Long
  Cc: Ingo Molnar, Peter Zijlstra, Thomas Gleixner,
	Linux Kernel Mailing List, Davidlohr Bueso,
	Linux NFS Mailing List, Trond Myklebust, Anna Schumaker

On Fri, Mar 30, 2018 at 11:27 PM, Waiman Long <longman@redhat.com> wrote:
> For a rwsem, locking can either be exclusive or shared. The corresponding
> exclusive or shared unlock must be used. Otherwise, the protected data
> structures may get corrupted or the lock may be in an inconsistent state.
>
> In order to detect such anomaly, a new configuration option DEBUG_RWSEMS
> is added which can be enabled to look for such mismatches and print
> warnings that that happens.
>
> Signed-off-by: Waiman Long <longman@redhat.com>
> Acked-by: Davidlohr Bueso <dave@stgolabs.net>

The new warning triggered in NFS, see
https://bugs.linaro.org/show_bug.cgi?id=3731

[   52.651490] DEBUG_LOCKS_WARN_ON(sem->owner != ((struct task_struct *)1UL))
[   52.651506] WARNING: CPU: 2 PID: 1457 at
/srv/oe/build/tmp-rpb-glibc/work-shared/intel-core2-32/kernel-source/kernel/locking/rwsem.c:217
up_read_non_owner+0x5d/0x70
[   52.674398] Modules linked in: x86_pkg_temp_thermal fuse
[   52.679719] CPU: 2 PID: 1457 Comm: kworker/2:2 Not tainted 4.16.0 #1
[   52.687448] Hardware name: Supermicro SYS-5019S-ML/X11SSH-F, BIOS
2.0b 07/27/2017
[   52.694922] Workqueue: nfsiod rpc_async_release
[   52.699454] RIP: 0010:up_read_non_owner+0x5d/0x70
[   52.704157] RSP: 0018:ffff9cbf81a23dd0 EFLAGS: 00010282
[   52.709376] RAX: 0000000000000000 RBX: ffff8dc1983c76c0 RCX: 0000000000000000
[   52.716500] RDX: ffffffffbd2d26c9 RSI: 0000000000000001 RDI: ffffffffbd2d2889
[   52.723652] RBP: ffff9cbf81a23dd8 R08: 0000000000000000 R09: 0000000000000000
[   52.730782] R10: ffff9cbf81a23dd0 R11: 0000000000000000 R12: ffff8dc19abf8600
[   52.737906] R13: ffff8dc19b6c0000 R14: 0000000000000000 R15: ffff8dc19bacad80
[   52.745029] FS:  0000000000000000(0000) GS:ffff8dc1afd00000(0000)
knlGS:0000000000000000
[   52.753108] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   52.758845] CR2: 00007f33794665d8 CR3: 000000016c41e006 CR4: 00000000003606e0
[   52.765968] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   52.773091] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   52.780215] Call Trace:
[   52.782695]  nfs_async_unlink_release+0x32/0x80
[   52.787220]  rpc_free_task+0x30/0x50
[   52.790789]  rpc_async_release+0x12/0x20
[   52.794707]  process_one_work+0x25e/0x660
[   52.798713]  worker_thread+0x4b/0x410
[   52.802377]  kthread+0x10d/0x140
[   52.805600]  ? rescuer_thread+0x3a0/0x3a0
[   52.809652]  ? kthread_create_worker_on_cpu+0x70/0x70
[   52.814702]  ? do_syscall_64+0x69/0x1b0
[   52.818540]  ret_from_fork+0x3a/0x50
[   52.822112] Code: ad 00 5b 5d c3 e8 74 ac 38 00 85 c0 74 de 8b 05
02 e3 48 02 85 c0 75 d4 48 c7 c6 c8 eb 4d be 48 c7 c7 5b ae 4c be e8
03 ae fa ff <0f> 0b eb bd 0f 1f 44 00 00 66 2e 0f 1f 84 00 00 00 00 00
0f 1f

I don't see anything immediately wrong with rmdir_sem (it's not that
complicated),
and there are only three users of up_read_non_owner(), so my first
suspicion would
be that something is wrong with the debug code, but please take a look
for yourself.

I also see that Bruce reported the same warning in
https://www.spinics.net/lists/linux-nfs/msg68064.html but apparently got no
reply.

      Arnd

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

* Re: [PATCH v5 1/3] locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches
  2018-04-11 14:21   ` [PATCH v5 1/3] " Arnd Bergmann
@ 2018-05-23  1:46     ` Dan Williams
  0 siblings, 0 replies; 9+ messages in thread
From: Dan Williams @ 2018-05-23  1:46 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: longman, Ingo Molnar, Peter Zijlstra, Thomas Gleixner,
	Linux Kernel Mailing List, dave, linux-nfs, trond.myklebust,
	anna.schumaker, Linus Torvalds

On Wed, Apr 11, 2018 at 7:24 AM Arnd Bergmann <arnd@arndb.de> wrote:

> On Fri, Mar 30, 2018 at 11:27 PM, Waiman Long <longman@redhat.com> wrote:
> > For a rwsem, locking can either be exclusive or shared. The
corresponding
> > exclusive or shared unlock must be used. Otherwise, the protected data
> > structures may get corrupted or the lock may be in an inconsistent
state.
> >
> > In order to detect such anomaly, a new configuration option DEBUG_RWSEMS
> > is added which can be enabled to look for such mismatches and print
> > warnings that that happens.
> >
> > Signed-off-by: Waiman Long <longman@redhat.com>
> > Acked-by: Davidlohr Bueso <dave@stgolabs.net>

> The new warning triggered in NFS, see
> https://bugs.linaro.org/show_bug.cgi?id=3731

> [   52.651490] DEBUG_LOCKS_WARN_ON(sem->owner != ((struct task_struct
*)1UL))
> [   52.651506] WARNING: CPU: 2 PID: 1457 at

/srv/oe/build/tmp-rpb-glibc/work-shared/intel-core2-32/kernel-source/kernel/locking/rwsem.c:217
> up_read_non_owner+0x5d/0x70
> [   52.674398] Modules linked in: x86_pkg_temp_thermal fuse
> [   52.679719] CPU: 2 PID: 1457 Comm: kworker/2:2 Not tainted 4.16.0 #1
> [   52.687448] Hardware name: Supermicro SYS-5019S-ML/X11SSH-F, BIOS
> 2.0b 07/27/2017
> [   52.694922] Workqueue: nfsiod rpc_async_release
> [   52.699454] RIP: 0010:up_read_non_owner+0x5d/0x70
> [   52.704157] RSP: 0018:ffff9cbf81a23dd0 EFLAGS: 00010282
> [   52.709376] RAX: 0000000000000000 RBX: ffff8dc1983c76c0 RCX:
0000000000000000
> [   52.716500] RDX: ffffffffbd2d26c9 RSI: 0000000000000001 RDI:
ffffffffbd2d2889
> [   52.723652] RBP: ffff9cbf81a23dd8 R08: 0000000000000000 R09:
0000000000000000
> [   52.730782] R10: ffff9cbf81a23dd0 R11: 0000000000000000 R12:
ffff8dc19abf8600
> [   52.737906] R13: ffff8dc19b6c0000 R14: 0000000000000000 R15:
ffff8dc19bacad80
> [   52.745029] FS:  0000000000000000(0000) GS:ffff8dc1afd00000(0000)
> knlGS:0000000000000000
> [   52.753108] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   52.758845] CR2: 00007f33794665d8 CR3: 000000016c41e006 CR4:
00000000003606e0
> [   52.765968] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
> [   52.773091] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
> [   52.780215] Call Trace:
> [   52.782695]  nfs_async_unlink_release+0x32/0x80
> [   52.787220]  rpc_free_task+0x30/0x50
> [   52.790789]  rpc_async_release+0x12/0x20
> [   52.794707]  process_one_work+0x25e/0x660
> [   52.798713]  worker_thread+0x4b/0x410
> [   52.802377]  kthread+0x10d/0x140
> [   52.805600]  ? rescuer_thread+0x3a0/0x3a0
> [   52.809652]  ? kthread_create_worker_on_cpu+0x70/0x70
> [   52.814702]  ? do_syscall_64+0x69/0x1b0
> [   52.818540]  ret_from_fork+0x3a/0x50
> [   52.822112] Code: ad 00 5b 5d c3 e8 74 ac 38 00 85 c0 74 de 8b 05
> 02 e3 48 02 85 c0 75 d4 48 c7 c6 c8 eb 4d be 48 c7 c7 5b ae 4c be e8
> 03 ae fa ff <0f> 0b eb bd 0f 1f 44 00 00 66 2e 0f 1f 84 00 00 00 00 00
> 0f 1f

> I don't see anything immediately wrong with rmdir_sem (it's not that
> complicated),
> and there are only three users of up_read_non_owner(), so my first
> suspicion would
> be that something is wrong with the debug code, but please take a look
> for yourself.

> I also see that Bruce reported the same warning in
> https://www.spinics.net/lists/linux-nfs/msg68064.html but apparently got
no
> reply.

I'm still seeing this regression on 4.17-rc6

[   93.481301] DEBUG_LOCKS_WARN_ON(sem->owner != ((struct task_struct
*)(1UL << 0)))
[   93.481318] WARNING: CPU: 14 PID: 320 at kernel/locking/rwsem.c:217
up_read_non_owner+0x58/0x60
[   93.486308] Modules linked in: ip6t_rpfilter(E) ip6t_REJECT(E)
nf_reject_ipv6(E) xt_conntrack(E) ebtable_nat(E) ebtable_broute(E)
bridge(E) stp(E) llc(E) ip6table_nat(E) nf_conntrack_ipv6(E)
nf_defrag_ipv6(E) nf_nat_ipv6(E) ip6table_mangle(E) ip6table_raw(E)
ip6table_security(E) iptable_nat(E) nf_conntrack_ipv4(E) nf_defrag_ipv4(E)
nf_nat_ipv4(E) nf_nat(E) nf_conntrack(E) iptable_mangle(E) iptable_raw(E)
iptable_security(E) ebtable_filter(E) ebtables(E) ip6table_filter(E)
ip6_tables(E) crct10dif_pclmul(E) crc32_pclmul(E) crc32c_intel(E)
ghash_clmulni_intel(E) dax_pmem(OE) nd_pmem(OE) device_dax(OE) nd_btt(OE)
serio_raw(E) nd_e820(OE) nfit(OE) libnvdimm(OE) nfit_test_iomap(OE)
[   93.499172] CPU: 14 PID: 320 Comm: kworker/14:1 Tainted: G           OE
     4.17.0-rc6+ #1921
[   93.500958] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.11.1-0-g0551a4be2c-prebuilt.qemu-project.org 04/01/2014
[   93.503714] Workqueue: nfsiod rpc_async_release
[   93.504619] RIP: 0010:up_read_non_owner+0x58/0x60
[   93.505492] RSP: 0018:ffffc90004fbbe18 EFLAGS: 00010286
[   93.506658] RAX: 0000000000000000 RBX: ffff88040f339548 RCX:
0000000000000000
[   93.508135] RDX: 0000000000000002 RSI: 0000000000000001 RDI:
0000000000000247
[   93.509651] RBP: ffff88042ab5f308 R08: 0000000000000000 R09:
0000000000000001
[   93.511166] R10: ffffc90004fbbdf8 R11: ffff88042a84af40 R12:
ffff880429a94088
[   93.512630] R13: ffff8804317abd00 R14: 0000000000000000 R15:
0000000000000000
[   93.514215] FS:  0000000000000000(0000) GS:ffff880431780000(0000)
knlGS:0000000000000000
[   93.516016] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   93.517414] CR2: 00007fd78ad448a0 CR3: 0000000002610000 CR4:
00000000000406e0
[   93.518930] Call Trace:
[   93.519612]  nfs_async_unlink_release+0x2d/0x80
[   93.520600]  rpc_free_task+0x2d/0x70
[   93.521495]  process_one_work+0x212/0x660
[   93.522553]  worker_thread+0x3a/0x390
[   93.523612]  ? process_one_work+0x660/0x660
[   93.524660]  kthread+0x11e/0x140
[   93.525766]  ? kthread_create_worker_on_cpu+0x70/0x70

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

end of thread, other threads:[~2018-05-23  1:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-30 21:27 [PATCH v5 0/3] locking/rwsem: Add DEBUG_RWSEMS & restructure lock debugging menu Waiman Long
2018-03-30 21:27 ` [PATCH v5 1/3] locking/rwsem: Add DEBUG_RWSEMS to look for lock/unlock mismatches Waiman Long
2018-03-31  7:59   ` [tip:locking/core] " tip-bot for Waiman Long
2018-04-11 14:21   ` [PATCH v5 1/3] " Arnd Bergmann
2018-05-23  1:46     ` Dan Williams
2018-03-30 21:27 ` [PATCH v5 2/3] lib/Kconfig.debug: Add LOCK_DEBUGGING_SUPPORT to make it more readable Waiman Long
2018-03-31  7:59   ` [tip:locking/core] locking/Kconfig: " tip-bot for Waiman Long
2018-03-30 21:28 ` [PATCH v5 3/3] lib/Kconfig.debug: Restructure the lock debugging menu Waiman Long
2018-03-31  8:00   ` [tip:locking/core] locking/Kconfig: " tip-bot for Waiman Long

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).