kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH rcu 0/7] RCU-related lockdep changes for v6.4
@ 2023-03-17  3:13 Boqun Feng
  2023-03-17  3:13 ` [PATCH rcu 1/7] locking/lockdep: Introduce lock_sync() Boqun Feng
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Boqun Feng @ 2023-03-17  3:13 UTC (permalink / raw)
  To: rcu
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Boqun Feng, Lai Jiangshan, Paul E. McKenney, Josh Triplett,
	Steven Rostedt, Mathieu Desnoyers, Davidlohr Bueso,
	Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes, Shuah Khan,
	David Woodhouse, Paolo Bonzini, kvm, seanjc, linux-kernel,
	linux-kselftest

Hi,

This series enables deadlock detection for srcu_read_lock() vs
synchronize_srcu().

Again, my first time helping prepare PR, so please take a careful look
and yell at me if there is something wrong. Thanks a lot!

You will also be able to find the series at:

	https://github/fbq/linux rcu/lockdep.2023.03.12a

top commit is:

	24390de55773	

List of changes:

Boqun Feng (4):
  locking/lockdep: Introduce lock_sync()
  rcu: Annotate SRCU's update-side lockdep dependencies
  locking: Reduce the number of locks in ww_mutex stress tests
  locking/lockdep: Improve the deadlock scenario print for sync and read
    lock

Paul E. McKenney (3):
  rcutorture: Add SRCU deadlock scenarios
  rcutorture: Add RCU Tasks Trace and SRCU deadlock scenarios
  rcutorture: Add srcu_lockdep.sh

 include/linux/lockdep.h                       |   8 +-
 include/linux/srcu.h                          |  34 +++-
 kernel/locking/lockdep.c                      |  64 +++++-
 kernel/locking/test-ww_mutex.c                |   2 +-
 kernel/rcu/rcutorture.c                       | 185 ++++++++++++++++++
 kernel/rcu/srcutiny.c                         |   2 +
 kernel/rcu/srcutree.c                         |   2 +
 .../selftests/rcutorture/bin/srcu_lockdep.sh  |  73 +++++++
 8 files changed, 359 insertions(+), 11 deletions(-)
 create mode 100755 tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh

-- 
2.39.2


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

* [PATCH rcu 1/7] locking/lockdep: Introduce lock_sync()
  2023-03-17  3:13 [PATCH rcu 0/7] RCU-related lockdep changes for v6.4 Boqun Feng
@ 2023-03-17  3:13 ` Boqun Feng
  2023-03-20 17:06   ` Davidlohr Bueso
  2023-03-17  3:13 ` [PATCH rcu 2/7] rcu: Annotate SRCU's update-side lockdep dependencies Boqun Feng
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Boqun Feng @ 2023-03-17  3:13 UTC (permalink / raw)
  To: rcu
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Boqun Feng, Lai Jiangshan, Paul E. McKenney, Josh Triplett,
	Steven Rostedt, Mathieu Desnoyers, Davidlohr Bueso,
	Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes, Shuah Khan,
	David Woodhouse, Paolo Bonzini, kvm, seanjc, linux-kernel,
	linux-kselftest

Currently, functions like synchronize_srcu() do not have lockdep
annotations resembling those of other write-side locking primitives.
Such annotations might look as follows:

	lock_acquire();
	lock_release();

Such annotations would tell lockdep that synchronize_srcu() acts like
an empty critical section that waits for other (read-side) critical
sections to finish.  This would definitely catch some deadlock, but
as pointed out by Paul Mckenney [1], this could also introduce false
positives because of irq-safe/unsafe detection.  Of course, there are
tricks could help with this:

	might_sleep(); // Existing statement in __synchronize_srcu().
	if (IS_ENABLED(CONFIG_PROVE_LOCKING)) {
		local_irq_disable();
		lock_acquire();
		lock_release();
		local_irq_enable();
	}

But it would be better for lockdep to provide a separate annonation for
functions like synchronize_srcu(), so that people won't need to repeat
the ugly tricks above.

Therefore introduce lock_sync(), which is simply an lock+unlock
pair with no irq safe/unsafe deadlock check.  This works because the
to-be-annontated functions do not create real critical sections, and
there is therefore no way that irq can create extra dependencies.

[1]: https://lore.kernel.org/lkml/20180412021233.ewncg5jjuzjw3x62@tardis/

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Acked-by: Waiman Long <longman@redhat.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 include/linux/lockdep.h  |  5 +++++
 kernel/locking/lockdep.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 1023f349af71..14d9dbedc6c1 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -268,6 +268,10 @@ extern void lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 
 extern void lock_release(struct lockdep_map *lock, unsigned long ip);
 
+extern void lock_sync(struct lockdep_map *lock, unsigned int subclass,
+		      int read, int check, struct lockdep_map *nest_lock,
+		      unsigned long ip);
+
 /* lock_is_held_type() returns */
 #define LOCK_STATE_UNKNOWN	-1
 #define LOCK_STATE_NOT_HELD	0
@@ -554,6 +558,7 @@ do {									\
 #define lock_map_acquire_read(l)		lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)
 #define lock_map_acquire_tryread(l)		lock_acquire_shared_recursive(l, 0, 1, NULL, _THIS_IP_)
 #define lock_map_release(l)			lock_release(l, _THIS_IP_)
+#define lock_map_sync(l)			lock_sync(l, 0, 0, 1, NULL, _THIS_IP_)
 
 #ifdef CONFIG_PROVE_LOCKING
 # define might_lock(lock)						\
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 50d4863974e7..36430cf8e407 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -5693,6 +5693,40 @@ void lock_release(struct lockdep_map *lock, unsigned long ip)
 }
 EXPORT_SYMBOL_GPL(lock_release);
 
+/*
+ * lock_sync() - A special annotation for synchronize_{s,}rcu()-like API.
+ *
+ * No actual critical section is created by the APIs annotated with this: these
+ * APIs are used to wait for one or multiple critical sections (on other CPUs
+ * or threads), and it means that calling these APIs inside these critical
+ * sections is potential deadlock.
+ *
+ * This annotation acts as an acqurie+release anontation pair with hardirqoff
+ * being 1. Since there's no critical section, no interrupt can create extra
+ * dependencies "inside" the annotation, hardirqoff == 1 allows us to avoid
+ * false positives.
+ */
+void lock_sync(struct lockdep_map *lock, unsigned subclass, int read,
+	       int check, struct lockdep_map *nest_lock, unsigned long ip)
+{
+	unsigned long flags;
+
+	if (unlikely(!lockdep_enabled()))
+		return;
+
+	raw_local_irq_save(flags);
+	check_flags(flags);
+
+	lockdep_recursion_inc();
+	__lock_acquire(lock, subclass, 0, read, check, 1, nest_lock, ip, 0, 0);
+
+	if (__lock_release(lock, ip))
+		check_chain_key(current);
+	lockdep_recursion_finish();
+	raw_local_irq_restore(flags);
+}
+EXPORT_SYMBOL_GPL(lock_sync);
+
 noinstr int lock_is_held_type(const struct lockdep_map *lock, int read)
 {
 	unsigned long flags;
-- 
2.39.2


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

* [PATCH rcu 2/7] rcu: Annotate SRCU's update-side lockdep dependencies
  2023-03-17  3:13 [PATCH rcu 0/7] RCU-related lockdep changes for v6.4 Boqun Feng
  2023-03-17  3:13 ` [PATCH rcu 1/7] locking/lockdep: Introduce lock_sync() Boqun Feng
@ 2023-03-17  3:13 ` Boqun Feng
  2023-03-17  3:13 ` [PATCH rcu 3/7] locking: Reduce the number of locks in ww_mutex stress tests Boqun Feng
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Boqun Feng @ 2023-03-17  3:13 UTC (permalink / raw)
  To: rcu
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Boqun Feng, Lai Jiangshan, Paul E. McKenney, Josh Triplett,
	Steven Rostedt, Mathieu Desnoyers, Davidlohr Bueso,
	Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes, Shuah Khan,
	David Woodhouse, Paolo Bonzini, kvm, seanjc, linux-kernel,
	linux-kselftest

Although all flavors of RCU readers are annotated correctly with
lockdep as recursive read locks, they do not set the lock_acquire
'check' parameter.  This means that RCU read locks are not added to
the lockdep dependency graph, which in turn means that lockdep cannot
detect RCU-based deadlocks.  This is not a problem for RCU flavors having
atomic read-side critical sections because context-based annotations can
catch these deadlocks, see for example the RCU_LOCKDEP_WARN() statement
in synchronize_rcu().  But context-based annotations are not helpful
for sleepable RCU, especially given that it is perfectly legal to do
synchronize_srcu(&srcu1) within an srcu_read_lock(&srcu2).

However, we can detect SRCU-based by: (1) Making srcu_read_lock() a
'check'ed recursive read lock and (2) Making synchronize_srcu() a empty
write lock critical section.  Even better, with the newly introduced
lock_sync(), we can avoid false positives about irq-unsafe/safe.
This commit therefore makes it so.

Note that NMI-safe SRCU read side critical sections are currently not
annotated, but might be annotated in the future.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
[ boqun: Add comments for annotation per Waiman's suggestion ]
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 include/linux/srcu.h  | 34 ++++++++++++++++++++++++++++++++--
 kernel/rcu/srcutiny.c |  2 ++
 kernel/rcu/srcutree.c |  2 ++
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 74796cd7e7a9..7a745169b79a 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -102,6 +102,32 @@ static inline int srcu_read_lock_held(const struct srcu_struct *ssp)
 	return lock_is_held(&ssp->dep_map);
 }
 
+/**
+ * Annotations provide deadlock detection for SRCU.
+ *
+ * Similar to other lockdep annotations, except there is an additional
+ * srcu_lock_sync(), which is basically an empty *write*-side critical section,
+ * see lock_sync() for more information.
+ */
+
+/* Annotates a srcu_read_lock() */
+static inline void srcu_lock_acquire(struct lockdep_map *map)
+{
+	lock_map_acquire_read(map);
+}
+
+/* Annotates a srcu_read_lock() */
+static inline void srcu_lock_release(struct lockdep_map *map)
+{
+	lock_map_release(map);
+}
+
+/* Annotates a synchronize_srcu() */
+static inline void srcu_lock_sync(struct lockdep_map *map)
+{
+	lock_map_sync(map);
+}
+
 #else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
 
 static inline int srcu_read_lock_held(const struct srcu_struct *ssp)
@@ -109,6 +135,10 @@ static inline int srcu_read_lock_held(const struct srcu_struct *ssp)
 	return 1;
 }
 
+#define srcu_lock_acquire(m) do { } while (0)
+#define srcu_lock_release(m) do { } while (0)
+#define srcu_lock_sync(m) do { } while (0)
+
 #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
 
 #define SRCU_NMI_UNKNOWN	0x0
@@ -182,7 +212,7 @@ static inline int srcu_read_lock(struct srcu_struct *ssp) __acquires(ssp)
 
 	srcu_check_nmi_safety(ssp, false);
 	retval = __srcu_read_lock(ssp);
-	rcu_lock_acquire(&(ssp)->dep_map);
+	srcu_lock_acquire(&(ssp)->dep_map);
 	return retval;
 }
 
@@ -254,7 +284,7 @@ static inline void srcu_read_unlock(struct srcu_struct *ssp, int idx)
 {
 	WARN_ON_ONCE(idx & ~0x1);
 	srcu_check_nmi_safety(ssp, false);
-	rcu_lock_release(&(ssp)->dep_map);
+	srcu_lock_release(&(ssp)->dep_map);
 	__srcu_read_unlock(ssp, idx);
 }
 
diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index b12fb0cec44d..336af24e0fe3 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -197,6 +197,8 @@ void synchronize_srcu(struct srcu_struct *ssp)
 {
 	struct rcu_synchronize rs;
 
+	srcu_lock_sync(&ssp->dep_map);
+
 	RCU_LOCKDEP_WARN(lockdep_is_held(ssp) ||
 			lock_is_held(&rcu_bh_lock_map) ||
 			lock_is_held(&rcu_lock_map) ||
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index ab4ee58af84b..c541b82646b6 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -1307,6 +1307,8 @@ static void __synchronize_srcu(struct srcu_struct *ssp, bool do_norm)
 {
 	struct rcu_synchronize rcu;
 
+	srcu_lock_sync(&ssp->dep_map);
+
 	RCU_LOCKDEP_WARN(lockdep_is_held(ssp) ||
 			 lock_is_held(&rcu_bh_lock_map) ||
 			 lock_is_held(&rcu_lock_map) ||
-- 
2.39.2


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

* [PATCH rcu 3/7] locking: Reduce the number of locks in ww_mutex stress tests
  2023-03-17  3:13 [PATCH rcu 0/7] RCU-related lockdep changes for v6.4 Boqun Feng
  2023-03-17  3:13 ` [PATCH rcu 1/7] locking/lockdep: Introduce lock_sync() Boqun Feng
  2023-03-17  3:13 ` [PATCH rcu 2/7] rcu: Annotate SRCU's update-side lockdep dependencies Boqun Feng
@ 2023-03-17  3:13 ` Boqun Feng
  2023-03-17 18:38   ` Paul E. McKenney
  2023-03-17  3:13 ` [PATCH rcu 4/7] locking/lockdep: Improve the deadlock scenario print for sync and read lock Boqun Feng
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Boqun Feng @ 2023-03-17  3:13 UTC (permalink / raw)
  To: rcu
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Boqun Feng, Lai Jiangshan, Paul E. McKenney, Josh Triplett,
	Steven Rostedt, Mathieu Desnoyers, Davidlohr Bueso,
	Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes, Shuah Khan,
	David Woodhouse, Paolo Bonzini, kvm, seanjc, linux-kernel,
	linux-kselftest, kernel test robot

The stress test in test_ww_mutex_init() uses 4095 locks since
lockdep::reference has 12 bits, and since we are going to reduce it to
11 bits to support lock_sync(), and 2047 is still a reasonable number of
the max nesting level for locks, so adjust the test.

Reported-by: kernel test robot <oliver.sang@intel.com>
Link: https://lore.kernel.org/oe-lkp/202302011445.9d99dae2-oliver.sang@intel.com
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 kernel/locking/test-ww_mutex.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/locking/test-ww_mutex.c b/kernel/locking/test-ww_mutex.c
index 29dc253d03af..93cca6e69860 100644
--- a/kernel/locking/test-ww_mutex.c
+++ b/kernel/locking/test-ww_mutex.c
@@ -659,7 +659,7 @@ static int __init test_ww_mutex_init(void)
 	if (ret)
 		return ret;
 
-	ret = stress(4095, hweight32(STRESS_ALL)*ncpus, STRESS_ALL);
+	ret = stress(2047, hweight32(STRESS_ALL)*ncpus, STRESS_ALL);
 	if (ret)
 		return ret;
 
-- 
2.39.2


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

* [PATCH rcu 4/7] locking/lockdep: Improve the deadlock scenario print for sync and read lock
  2023-03-17  3:13 [PATCH rcu 0/7] RCU-related lockdep changes for v6.4 Boqun Feng
                   ` (2 preceding siblings ...)
  2023-03-17  3:13 ` [PATCH rcu 3/7] locking: Reduce the number of locks in ww_mutex stress tests Boqun Feng
@ 2023-03-17  3:13 ` Boqun Feng
  2023-03-20 12:13   ` Peter Zijlstra
  2023-03-17  3:13 ` [PATCH rcu 5/7] rcutorture: Add SRCU deadlock scenarios Boqun Feng
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Boqun Feng @ 2023-03-17  3:13 UTC (permalink / raw)
  To: rcu
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Boqun Feng, Lai Jiangshan, Paul E. McKenney, Josh Triplett,
	Steven Rostedt, Mathieu Desnoyers, Davidlohr Bueso,
	Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes, Shuah Khan,
	David Woodhouse, Paolo Bonzini, kvm, seanjc, linux-kernel,
	linux-kselftest

Lock scenario print is always a weak spot of lockdep splats. Improvement
can be made if we rework the dependency search and the error printing.

However without touching the graph search, we can improve a little for
the circular deadlock case, since we have the to-be-added lock
dependency, and know whether these two locks are read/write/sync.

In order to know whether a held_lock is sync or not, a bit was
"stolen" from ->references, which reduce our limit for the same lock
class nesting from 2^12 to 2^11, and it should still be good enough.

Besides, since we now have bit in held_lock for sync, we don't need the
"hardirqoffs being 1" trick, and also we can avoid the __lock_release()
if we jump out of __lock_acquire() before the held_lock stored.

With these changes, a deadlock case evolved with read lock and sync gets
a better print-out from:

	[...]  Possible unsafe locking scenario:
	[...]
	[...]        CPU0                    CPU1
	[...]        ----                    ----
	[...]   lock(srcuA);
	[...]                                lock(srcuB);
	[...]                                lock(srcuA);
	[...]   lock(srcuB);

to

	[...]  Possible unsafe locking scenario:
	[...]
	[...]        CPU0                    CPU1
	[...]        ----                    ----
	[...]   rlock(srcuA);
	[...]                                lock(srcuB);
	[...]                                lock(srcuA);
	[...]   sync(srcuB);

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 include/linux/lockdep.h  |  3 ++-
 kernel/locking/lockdep.c | 48 ++++++++++++++++++++++++++--------------
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 14d9dbedc6c1..b32256e9e944 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -134,7 +134,8 @@ struct held_lock {
 	unsigned int read:2;        /* see lock_acquire() comment */
 	unsigned int check:1;       /* see lock_acquire() comment */
 	unsigned int hardirqs_off:1;
-	unsigned int references:12;					/* 32 bits */
+	unsigned int sync:1;
+	unsigned int references:11;					/* 32 bits */
 	unsigned int pin_count;
 };
 
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 36430cf8e407..dcd1d5bfc1e0 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1881,6 +1881,8 @@ print_circular_lock_scenario(struct held_lock *src,
 	struct lock_class *source = hlock_class(src);
 	struct lock_class *target = hlock_class(tgt);
 	struct lock_class *parent = prt->class;
+	int src_read = src->read;
+	int tgt_read = tgt->read;
 
 	/*
 	 * A direct locking problem where unsafe_class lock is taken
@@ -1908,7 +1910,10 @@ print_circular_lock_scenario(struct held_lock *src,
 	printk(" Possible unsafe locking scenario:\n\n");
 	printk("       CPU0                    CPU1\n");
 	printk("       ----                    ----\n");
-	printk("  lock(");
+	if (tgt_read != 0)
+		printk("  rlock(");
+	else
+		printk("  lock(");
 	__print_lock_name(target);
 	printk(KERN_CONT ");\n");
 	printk("                               lock(");
@@ -1917,7 +1922,12 @@ print_circular_lock_scenario(struct held_lock *src,
 	printk("                               lock(");
 	__print_lock_name(target);
 	printk(KERN_CONT ");\n");
-	printk("  lock(");
+	if (src_read != 0)
+		printk("  rlock(");
+	else if (src->sync)
+		printk("  sync(");
+	else
+		printk("  lock(");
 	__print_lock_name(source);
 	printk(KERN_CONT ");\n");
 	printk("\n *** DEADLOCK ***\n\n");
@@ -4531,7 +4541,13 @@ mark_usage(struct task_struct *curr, struct held_lock *hlock, int check)
 					return 0;
 		}
 	}
-	if (!hlock->hardirqs_off) {
+
+	/*
+	 * For lock_sync(), don't mark the ENABLED usage, since lock_sync()
+	 * creates no critical section and no extra dependency can be introduced
+	 * by interrupts
+	 */
+	if (!hlock->hardirqs_off && !hlock->sync) {
 		if (hlock->read) {
 			if (!mark_lock(curr, hlock,
 					LOCK_ENABLED_HARDIRQ_READ))
@@ -4910,7 +4926,7 @@ static int __lock_is_held(const struct lockdep_map *lock, int read);
 static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 			  int trylock, int read, int check, int hardirqs_off,
 			  struct lockdep_map *nest_lock, unsigned long ip,
-			  int references, int pin_count)
+			  int references, int pin_count, int sync)
 {
 	struct task_struct *curr = current;
 	struct lock_class *class = NULL;
@@ -4961,7 +4977,8 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 
 	class_idx = class - lock_classes;
 
-	if (depth) { /* we're holding locks */
+	if (depth && !sync) {
+		/* we're holding locks and the new held lock is not a sync */
 		hlock = curr->held_locks + depth - 1;
 		if (hlock->class_idx == class_idx && nest_lock) {
 			if (!references)
@@ -4995,6 +5012,7 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 	hlock->trylock = trylock;
 	hlock->read = read;
 	hlock->check = check;
+	hlock->sync = !!sync;
 	hlock->hardirqs_off = !!hardirqs_off;
 	hlock->references = references;
 #ifdef CONFIG_LOCK_STAT
@@ -5056,6 +5074,10 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 	if (!validate_chain(curr, hlock, chain_head, chain_key))
 		return 0;
 
+	/* For lock_sync(), we are done here since no actual critical section */
+	if (hlock->sync)
+		return 1;
+
 	curr->curr_chain_key = chain_key;
 	curr->lockdep_depth++;
 	check_chain_key(curr);
@@ -5197,7 +5219,7 @@ static int reacquire_held_locks(struct task_struct *curr, unsigned int depth,
 				    hlock->read, hlock->check,
 				    hlock->hardirqs_off,
 				    hlock->nest_lock, hlock->acquire_ip,
-				    hlock->references, hlock->pin_count)) {
+				    hlock->references, hlock->pin_count, 0)) {
 		case 0:
 			return 1;
 		case 1:
@@ -5667,7 +5689,7 @@ void lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 
 	lockdep_recursion_inc();
 	__lock_acquire(lock, subclass, trylock, read, check,
-		       irqs_disabled_flags(flags), nest_lock, ip, 0, 0);
+		       irqs_disabled_flags(flags), nest_lock, ip, 0, 0, 0);
 	lockdep_recursion_finish();
 	raw_local_irq_restore(flags);
 }
@@ -5700,11 +5722,6 @@ EXPORT_SYMBOL_GPL(lock_release);
  * APIs are used to wait for one or multiple critical sections (on other CPUs
  * or threads), and it means that calling these APIs inside these critical
  * sections is potential deadlock.
- *
- * This annotation acts as an acqurie+release anontation pair with hardirqoff
- * being 1. Since there's no critical section, no interrupt can create extra
- * dependencies "inside" the annotation, hardirqoff == 1 allows us to avoid
- * false positives.
  */
 void lock_sync(struct lockdep_map *lock, unsigned subclass, int read,
 	       int check, struct lockdep_map *nest_lock, unsigned long ip)
@@ -5718,10 +5735,9 @@ void lock_sync(struct lockdep_map *lock, unsigned subclass, int read,
 	check_flags(flags);
 
 	lockdep_recursion_inc();
-	__lock_acquire(lock, subclass, 0, read, check, 1, nest_lock, ip, 0, 0);
-
-	if (__lock_release(lock, ip))
-		check_chain_key(current);
+	__lock_acquire(lock, subclass, 0, read, check,
+		       irqs_disabled_flags(flags), nest_lock, ip, 0, 0, 1);
+	check_chain_key(current);
 	lockdep_recursion_finish();
 	raw_local_irq_restore(flags);
 }
-- 
2.39.2


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

* [PATCH rcu 5/7] rcutorture: Add SRCU deadlock scenarios
  2023-03-17  3:13 [PATCH rcu 0/7] RCU-related lockdep changes for v6.4 Boqun Feng
                   ` (3 preceding siblings ...)
  2023-03-17  3:13 ` [PATCH rcu 4/7] locking/lockdep: Improve the deadlock scenario print for sync and read lock Boqun Feng
@ 2023-03-17  3:13 ` Boqun Feng
  2023-03-17  3:13 ` [PATCH rcu 6/7] rcutorture: Add RCU Tasks Trace and " Boqun Feng
  2023-03-17  3:13 ` [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh Boqun Feng
  6 siblings, 0 replies; 20+ messages in thread
From: Boqun Feng @ 2023-03-17  3:13 UTC (permalink / raw)
  To: rcu
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Boqun Feng, Lai Jiangshan, Paul E. McKenney, Josh Triplett,
	Steven Rostedt, Mathieu Desnoyers, Davidlohr Bueso,
	Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes, Shuah Khan,
	David Woodhouse, Paolo Bonzini, kvm, seanjc, linux-kernel,
	linux-kselftest

From: "Paul E. McKenney" <paulmck@kernel.org>

In order to test the new SRCU-lockdep functionality, this commit adds
an rcutorture.test_srcu_lockdep module parameter that, when non-zero,
selects an SRCU deadlock scenario to execute.  This parameter is a
five-digit number formatted as DNNL, where "D" is 1 to force a deadlock
and 0 to avoid doing so; "NN" is the test number, 0 for SRCU-based, 1
for SRCU/mutex-based, and 2 for SRCU/rwsem-based; and "L" is the number
of steps in the deadlock cycle.

Note that rcutorture.test_srcu_lockdep=1 will also force a hard hang.

If a non-zero value of rcutorture.test_srcu_lockdep does not select a
deadlock scenario, a console message is printed and testing continues.

[ paulmck: Apply kernel test robot feedback, add rwsem support. ]
[ paulmck: Apply Dan Carpenter feedback. ]

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 kernel/rcu/rcutorture.c | 151 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 151 insertions(+)

diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 8e6c023212cb..80ff9a743d31 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -120,6 +120,7 @@ torture_param(int, test_boost, 1, "Test RCU prio boost: 0=no, 1=maybe, 2=yes.");
 torture_param(int, test_boost_duration, 4, "Duration of each boost test, seconds.");
 torture_param(int, test_boost_interval, 7, "Interval between boost tests, seconds.");
 torture_param(bool, test_no_idle_hz, true, "Test support for tickless idle CPUs");
+torture_param(int, test_srcu_lockdep, 0, "Test specified SRCU deadlock scenario.");
 torture_param(int, verbose, 1, "Enable verbose debugging printk()s");
 
 static char *torture_type = "rcu";
@@ -3463,6 +3464,154 @@ static void rcutorture_sync(void)
 		cur_ops->sync();
 }
 
+static DEFINE_MUTEX(mut0);
+static DEFINE_MUTEX(mut1);
+static DEFINE_MUTEX(mut2);
+static DEFINE_MUTEX(mut3);
+static DEFINE_MUTEX(mut4);
+static DEFINE_MUTEX(mut5);
+static DEFINE_MUTEX(mut6);
+static DEFINE_MUTEX(mut7);
+static DEFINE_MUTEX(mut8);
+static DEFINE_MUTEX(mut9);
+
+static DECLARE_RWSEM(rwsem0);
+static DECLARE_RWSEM(rwsem1);
+static DECLARE_RWSEM(rwsem2);
+static DECLARE_RWSEM(rwsem3);
+static DECLARE_RWSEM(rwsem4);
+static DECLARE_RWSEM(rwsem5);
+static DECLARE_RWSEM(rwsem6);
+static DECLARE_RWSEM(rwsem7);
+static DECLARE_RWSEM(rwsem8);
+static DECLARE_RWSEM(rwsem9);
+
+DEFINE_STATIC_SRCU(srcu0);
+DEFINE_STATIC_SRCU(srcu1);
+DEFINE_STATIC_SRCU(srcu2);
+DEFINE_STATIC_SRCU(srcu3);
+DEFINE_STATIC_SRCU(srcu4);
+DEFINE_STATIC_SRCU(srcu5);
+DEFINE_STATIC_SRCU(srcu6);
+DEFINE_STATIC_SRCU(srcu7);
+DEFINE_STATIC_SRCU(srcu8);
+DEFINE_STATIC_SRCU(srcu9);
+
+static int srcu_lockdep_next(const char *f, const char *fl, const char *fs, const char *fu, int i,
+			     int cyclelen, int deadlock)
+{
+	int j = i + 1;
+
+	if (j >= cyclelen)
+		j = deadlock ? 0 : -1;
+	if (j >= 0)
+		pr_info("%s: %s(%d), %s(%d), %s(%d)\n", f, fl, i, fs, j, fu, i);
+	else
+		pr_info("%s: %s(%d), %s(%d)\n", f, fl, i, fu, i);
+	return j;
+}
+
+// Test lockdep on SRCU-based deadlock scenarios.
+static void rcu_torture_init_srcu_lockdep(void)
+{
+	int cyclelen;
+	int deadlock;
+	bool err = false;
+	int i;
+	int j;
+	int idx;
+	struct mutex *muts[] = { &mut0, &mut1, &mut2, &mut3, &mut4,
+				 &mut5, &mut6, &mut7, &mut8, &mut9 };
+	struct rw_semaphore *rwsems[] = { &rwsem0, &rwsem1, &rwsem2, &rwsem3, &rwsem4,
+					  &rwsem5, &rwsem6, &rwsem7, &rwsem8, &rwsem9 };
+	struct srcu_struct *srcus[] = { &srcu0, &srcu1, &srcu2, &srcu3, &srcu4,
+					&srcu5, &srcu6, &srcu7, &srcu8, &srcu9 };
+	int testtype;
+
+	if (!test_srcu_lockdep)
+		return;
+
+	deadlock = test_srcu_lockdep / 1000;
+	testtype = (test_srcu_lockdep / 10) % 100;
+	cyclelen = test_srcu_lockdep % 10;
+	WARN_ON_ONCE(ARRAY_SIZE(muts) != ARRAY_SIZE(srcus));
+	if (WARN_ONCE(deadlock != !!deadlock,
+		      "%s: test_srcu_lockdep=%d and deadlock digit %d must be zero or one.\n",
+		      __func__, test_srcu_lockdep, deadlock))
+		err = true;
+	if (WARN_ONCE(cyclelen <= 0,
+		      "%s: test_srcu_lockdep=%d and cycle-length digit %d must be greater than zero.\n",
+		      __func__, test_srcu_lockdep, cyclelen))
+		err = true;
+	if (err)
+		goto err_out;
+
+	if (testtype == 0) {
+		pr_info("%s: test_srcu_lockdep = %05d: SRCU %d-way %sdeadlock.\n",
+			__func__, test_srcu_lockdep, cyclelen, deadlock ? "" : "non-");
+		if (deadlock && cyclelen == 1)
+			pr_info("%s: Expect hang.\n", __func__);
+		for (i = 0; i < cyclelen; i++) {
+			j = srcu_lockdep_next(__func__, "srcu_read_lock", "synchronize_srcu",
+					      "srcu_read_unlock", i, cyclelen, deadlock);
+			idx = srcu_read_lock(srcus[i]);
+			if (j >= 0)
+				synchronize_srcu(srcus[j]);
+			srcu_read_unlock(srcus[i], idx);
+		}
+		return;
+	}
+
+	if (testtype == 1) {
+		pr_info("%s: test_srcu_lockdep = %05d: SRCU/mutex %d-way %sdeadlock.\n",
+			__func__, test_srcu_lockdep, cyclelen, deadlock ? "" : "non-");
+		for (i = 0; i < cyclelen; i++) {
+			pr_info("%s: srcu_read_lock(%d), mutex_lock(%d), mutex_unlock(%d), srcu_read_unlock(%d)\n",
+				__func__, i, i, i, i);
+			idx = srcu_read_lock(srcus[i]);
+			mutex_lock(muts[i]);
+			mutex_unlock(muts[i]);
+			srcu_read_unlock(srcus[i], idx);
+
+			j = srcu_lockdep_next(__func__, "mutex_lock", "synchronize_srcu",
+					      "mutex_unlock", i, cyclelen, deadlock);
+			mutex_lock(muts[i]);
+			if (j >= 0)
+				synchronize_srcu(srcus[j]);
+			mutex_unlock(muts[i]);
+		}
+		return;
+	}
+
+	if (testtype == 2) {
+		pr_info("%s: test_srcu_lockdep = %05d: SRCU/rwsem %d-way %sdeadlock.\n",
+			__func__, test_srcu_lockdep, cyclelen, deadlock ? "" : "non-");
+		for (i = 0; i < cyclelen; i++) {
+			pr_info("%s: srcu_read_lock(%d), down_read(%d), up_read(%d), srcu_read_unlock(%d)\n",
+				__func__, i, i, i, i);
+			idx = srcu_read_lock(srcus[i]);
+			down_read(rwsems[i]);
+			up_read(rwsems[i]);
+			srcu_read_unlock(srcus[i], idx);
+
+			j = srcu_lockdep_next(__func__, "down_write", "synchronize_srcu",
+					      "up_write", i, cyclelen, deadlock);
+			down_write(rwsems[i]);
+			if (j >= 0)
+				synchronize_srcu(srcus[j]);
+			up_write(rwsems[i]);
+		}
+		return;
+	}
+
+err_out:
+	pr_info("%s: test_srcu_lockdep = %05d does nothing.\n", __func__, test_srcu_lockdep);
+	pr_info("%s: test_srcu_lockdep = DNNL.\n", __func__);
+	pr_info("%s: D: Deadlock if nonzero.\n", __func__);
+	pr_info("%s: NN: Test number, 0=SRCU, 1=SRCU/mutex, 2=SRCU/rwsem.\n", __func__);
+	pr_info("%s: L: Cycle length.\n", __func__);
+}
+
 static int __init
 rcu_torture_init(void)
 {
@@ -3504,6 +3653,8 @@ rcu_torture_init(void)
 	if (cur_ops->init)
 		cur_ops->init();
 
+	rcu_torture_init_srcu_lockdep();
+
 	if (nreaders >= 0) {
 		nrealreaders = nreaders;
 	} else {
-- 
2.39.2


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

* [PATCH rcu 6/7] rcutorture: Add RCU Tasks Trace and SRCU deadlock scenarios
  2023-03-17  3:13 [PATCH rcu 0/7] RCU-related lockdep changes for v6.4 Boqun Feng
                   ` (4 preceding siblings ...)
  2023-03-17  3:13 ` [PATCH rcu 5/7] rcutorture: Add SRCU deadlock scenarios Boqun Feng
@ 2023-03-17  3:13 ` Boqun Feng
  2023-03-17  3:13 ` [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh Boqun Feng
  6 siblings, 0 replies; 20+ messages in thread
From: Boqun Feng @ 2023-03-17  3:13 UTC (permalink / raw)
  To: rcu
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Boqun Feng, Lai Jiangshan, Paul E. McKenney, Josh Triplett,
	Steven Rostedt, Mathieu Desnoyers, Davidlohr Bueso,
	Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes, Shuah Khan,
	David Woodhouse, Paolo Bonzini, kvm, seanjc, linux-kernel,
	linux-kselftest

From: "Paul E. McKenney" <paulmck@kernel.org>

Add a test number 3 that creates deadlock cycles involving one RCU
Tasks Trace step and L-1 SRCU steps.  Please note that lockdep will not
detect these deadlocks until synchronize_rcu_tasks_trace() is marked
with lockdep's new "sync" annotation, which will probably not happen
until some time after these markings prove their worth on SRCU.

Please note that these tests are available only in kernels built with
CONFIG_TASKS_TRACE_RCU=y.

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 kernel/rcu/rcutorture.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 80ff9a743d31..9efb8258a272 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -3604,12 +3604,46 @@ static void rcu_torture_init_srcu_lockdep(void)
 		return;
 	}
 
+#ifdef CONFIG_TASKS_TRACE_RCU
+	if (testtype == 3) {
+		pr_info("%s: test_srcu_lockdep = %05d: SRCU and Tasks Trace RCU %d-way %sdeadlock.\n",
+			__func__, test_srcu_lockdep, cyclelen, deadlock ? "" : "non-");
+		if (deadlock && cyclelen == 1)
+			pr_info("%s: Expect hang.\n", __func__);
+		for (i = 0; i < cyclelen; i++) {
+			char *fl = i == 0 ? "rcu_read_lock_trace" : "srcu_read_lock";
+			char *fs = i == cyclelen - 1 ? "synchronize_rcu_tasks_trace"
+						     : "synchronize_srcu";
+			char *fu = i == 0 ? "rcu_read_unlock_trace" : "srcu_read_unlock";
+
+			j = srcu_lockdep_next(__func__, fl, fs, fu, i, cyclelen, deadlock);
+			if (i == 0)
+				rcu_read_lock_trace();
+			else
+				idx = srcu_read_lock(srcus[i]);
+			if (j >= 0) {
+				if (i == cyclelen - 1)
+					synchronize_rcu_tasks_trace();
+				else
+					synchronize_srcu(srcus[j]);
+			}
+			if (i == 0)
+				rcu_read_unlock_trace();
+			else
+				srcu_read_unlock(srcus[i], idx);
+		}
+		return;
+	}
+#endif // #ifdef CONFIG_TASKS_TRACE_RCU
+
 err_out:
 	pr_info("%s: test_srcu_lockdep = %05d does nothing.\n", __func__, test_srcu_lockdep);
 	pr_info("%s: test_srcu_lockdep = DNNL.\n", __func__);
 	pr_info("%s: D: Deadlock if nonzero.\n", __func__);
-	pr_info("%s: NN: Test number, 0=SRCU, 1=SRCU/mutex, 2=SRCU/rwsem.\n", __func__);
+	pr_info("%s: NN: Test number, 0=SRCU, 1=SRCU/mutex, 2=SRCU/rwsem, 3=SRCU/Tasks Trace RCU.\n", __func__);
 	pr_info("%s: L: Cycle length.\n", __func__);
+	if (!IS_ENABLED(CONFIG_TASKS_TRACE_RCU))
+		pr_info("%s: NN=3 disallowed because kernel is built with CONFIG_TASKS_TRACE_RCU=n\n", __func__);
 }
 
 static int __init
-- 
2.39.2


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

* [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh
  2023-03-17  3:13 [PATCH rcu 0/7] RCU-related lockdep changes for v6.4 Boqun Feng
                   ` (5 preceding siblings ...)
  2023-03-17  3:13 ` [PATCH rcu 6/7] rcutorture: Add RCU Tasks Trace and " Boqun Feng
@ 2023-03-17  3:13 ` Boqun Feng
  2023-03-20 18:19   ` Boqun Feng
  6 siblings, 1 reply; 20+ messages in thread
From: Boqun Feng @ 2023-03-17  3:13 UTC (permalink / raw)
  To: rcu
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Boqun Feng, Lai Jiangshan, Paul E. McKenney, Josh Triplett,
	Steven Rostedt, Mathieu Desnoyers, Davidlohr Bueso,
	Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes, Shuah Khan,
	David Woodhouse, Paolo Bonzini, kvm, seanjc, linux-kernel,
	linux-kselftest

From: "Paul E. McKenney" <paulmck@kernel.org>

This commit adds an srcu_lockdep.sh script that checks whether lockdep
correctly classifies SRCU-based, SRCU/mutex-based, and SRCU/rwsem-based
deadlocks.

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
[ boqun: Fix "RCUTORTURE" with "$RCUTORTURE" ]
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 .../selftests/rcutorture/bin/srcu_lockdep.sh  | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100755 tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh

diff --git a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
new file mode 100755
index 000000000000..961932754684
--- /dev/null
+++ b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
@@ -0,0 +1,73 @@
+#!/bin/bash
+#
+# Run SRCU-lockdep tests and report any that fail to meet expectations.
+
+usage () {
+	echo "Usage: $scriptname optional arguments:"
+	echo "       --datestamp string"
+	exit 1
+}
+
+ds=`date +%Y.%m.%d-%H.%M.%S`-srcu_lockdep
+scriptname="$0"
+
+T="`mktemp -d ${TMPDIR-/tmp}/srcu_lockdep.sh.XXXXXX`"
+trap 'rm -rf $T' 0
+
+RCUTORTURE="`pwd`/tools/testing/selftests/rcutorture"; export RCUTORTURE
+PATH=${RCUTORTURE}/bin:$PATH; export PATH
+. functions.sh
+
+while test $# -gt 0
+do
+	case "$1" in
+	--datestamp)
+		checkarg --datestamp "(relative pathname)" "$#" "$2" '^[a-zA-Z0-9._/-]*$' '^--'
+		ds=$2
+		shift
+		;;
+	*)
+		echo Unknown argument $1
+		usage
+		;;
+	esac
+	shift
+done
+
+err=
+nerrs=0
+for d in 0 1
+do
+	for t in 0 1 2
+	do
+		for c in 1 2 3
+		do
+			err=
+			val=$((d*1000+t*10+c))
+			tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 5s --configs "SRCU-P" --bootargs "rcutorture.test_srcu_lockdep=$val" --trust-make --datestamp "$ds/$val" > "$T/kvm.sh.out" 2>&1
+			ret=$?
+			mv "$T/kvm.sh.out" "$RCUTORTURE/res/$ds/$val"
+			if test "$d" -ne 0 && test "$ret" -eq 0
+			then
+				err=1
+				echo -n Unexpected success for > "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
+			fi
+			if test "$d" -eq 0 && test "$ret" -ne 0
+			then
+				err=1
+				echo -n Unexpected failure for > "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
+			fi
+			if test -n "$err"
+			then
+				grep "rcu_torture_init_srcu_lockdep: test_srcu_lockdep = " "$RCUTORTURE/res/$ds/$val/SRCU-P/console.log" | sed -e 's/^.*rcu_torture_init_srcu_lockdep://' >> "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
+				cat "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
+				nerrs=$((nerrs+1))
+			fi
+		done
+	done
+done
+if test "$nerrs" -ne 0
+then
+	exit 1
+fi
+exit 0
-- 
2.39.2


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

* Re: [PATCH rcu 3/7] locking: Reduce the number of locks in ww_mutex stress tests
  2023-03-17  3:13 ` [PATCH rcu 3/7] locking: Reduce the number of locks in ww_mutex stress tests Boqun Feng
@ 2023-03-17 18:38   ` Paul E. McKenney
  2023-03-17 21:26     ` Boqun Feng
  0 siblings, 1 reply; 20+ messages in thread
From: Paul E. McKenney @ 2023-03-17 18:38 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rcu, Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Lai Jiangshan, Josh Triplett, Steven Rostedt, Mathieu Desnoyers,
	Davidlohr Bueso, Frederic Weisbecker, Neeraj Upadhyay,
	Joel Fernandes, Shuah Khan, David Woodhouse, Paolo Bonzini, kvm,
	seanjc, linux-kernel, linux-kselftest, kernel test robot

On Thu, Mar 16, 2023 at 08:13:35PM -0700, Boqun Feng wrote:
> The stress test in test_ww_mutex_init() uses 4095 locks since
> lockdep::reference has 12 bits, and since we are going to reduce it to
> 11 bits to support lock_sync(), and 2047 is still a reasonable number of
> the max nesting level for locks, so adjust the test.
> 
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Link: https://lore.kernel.org/oe-lkp/202302011445.9d99dae2-oliver.sang@intel.com
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>

Tested-by: Paul E. McKenney <paulmck@kernel.org>

> ---
>  kernel/locking/test-ww_mutex.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/locking/test-ww_mutex.c b/kernel/locking/test-ww_mutex.c
> index 29dc253d03af..93cca6e69860 100644
> --- a/kernel/locking/test-ww_mutex.c
> +++ b/kernel/locking/test-ww_mutex.c
> @@ -659,7 +659,7 @@ static int __init test_ww_mutex_init(void)
>  	if (ret)
>  		return ret;
>  
> -	ret = stress(4095, hweight32(STRESS_ALL)*ncpus, STRESS_ALL);
> +	ret = stress(2047, hweight32(STRESS_ALL)*ncpus, STRESS_ALL);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.39.2
> 

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

* Re: [PATCH rcu 3/7] locking: Reduce the number of locks in ww_mutex stress tests
  2023-03-17 18:38   ` Paul E. McKenney
@ 2023-03-17 21:26     ` Boqun Feng
  0 siblings, 0 replies; 20+ messages in thread
From: Boqun Feng @ 2023-03-17 21:26 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: rcu, Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Lai Jiangshan, Josh Triplett, Steven Rostedt, Mathieu Desnoyers,
	Davidlohr Bueso, Frederic Weisbecker, Neeraj Upadhyay,
	Joel Fernandes, Shuah Khan, David Woodhouse, Paolo Bonzini, kvm,
	seanjc, linux-kernel, linux-kselftest, kernel test robot

On Fri, Mar 17, 2023 at 11:38:19AM -0700, Paul E. McKenney wrote:
> On Thu, Mar 16, 2023 at 08:13:35PM -0700, Boqun Feng wrote:
> > The stress test in test_ww_mutex_init() uses 4095 locks since
> > lockdep::reference has 12 bits, and since we are going to reduce it to
> > 11 bits to support lock_sync(), and 2047 is still a reasonable number of
> > the max nesting level for locks, so adjust the test.
> > 
> > Reported-by: kernel test robot <oliver.sang@intel.com>
> > Link: https://lore.kernel.org/oe-lkp/202302011445.9d99dae2-oliver.sang@intel.com
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> 
> Tested-by: Paul E. McKenney <paulmck@kernel.org>
> 

Applied, thanks!

Regards,
Boqun

> > ---
> >  kernel/locking/test-ww_mutex.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/locking/test-ww_mutex.c b/kernel/locking/test-ww_mutex.c
> > index 29dc253d03af..93cca6e69860 100644
> > --- a/kernel/locking/test-ww_mutex.c
> > +++ b/kernel/locking/test-ww_mutex.c
> > @@ -659,7 +659,7 @@ static int __init test_ww_mutex_init(void)
> >  	if (ret)
> >  		return ret;
> >  
> > -	ret = stress(4095, hweight32(STRESS_ALL)*ncpus, STRESS_ALL);
> > +	ret = stress(2047, hweight32(STRESS_ALL)*ncpus, STRESS_ALL);
> >  	if (ret)
> >  		return ret;
> >  
> > -- 
> > 2.39.2
> > 

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

* Re: [PATCH rcu 4/7] locking/lockdep: Improve the deadlock scenario print for sync and read lock
  2023-03-17  3:13 ` [PATCH rcu 4/7] locking/lockdep: Improve the deadlock scenario print for sync and read lock Boqun Feng
@ 2023-03-20 12:13   ` Peter Zijlstra
  2023-03-20 17:50     ` Boqun Feng
  0 siblings, 1 reply; 20+ messages in thread
From: Peter Zijlstra @ 2023-03-20 12:13 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rcu, Ingo Molnar, Will Deacon, Waiman Long, Lai Jiangshan,
	Paul E. McKenney, Josh Triplett, Steven Rostedt,
	Mathieu Desnoyers, Davidlohr Bueso, Frederic Weisbecker,
	Neeraj Upadhyay, Joel Fernandes, Shuah Khan, David Woodhouse,
	Paolo Bonzini, kvm, seanjc, linux-kernel, linux-kselftest

On Thu, Mar 16, 2023 at 08:13:36PM -0700, Boqun Feng wrote:
> Lock scenario print is always a weak spot of lockdep splats. Improvement
> can be made if we rework the dependency search and the error printing.
> 
> However without touching the graph search, we can improve a little for
> the circular deadlock case, since we have the to-be-added lock
> dependency, and know whether these two locks are read/write/sync.
> 
> In order to know whether a held_lock is sync or not, a bit was
> "stolen" from ->references, which reduce our limit for the same lock
> class nesting from 2^12 to 2^11, and it should still be good enough.
> 
> Besides, since we now have bit in held_lock for sync, we don't need the
> "hardirqoffs being 1" trick, and also we can avoid the __lock_release()
> if we jump out of __lock_acquire() before the held_lock stored.
> 
> With these changes, a deadlock case evolved with read lock and sync gets
> a better print-out from:
> 
> 	[...]  Possible unsafe locking scenario:
> 	[...]
> 	[...]        CPU0                    CPU1
> 	[...]        ----                    ----
> 	[...]   lock(srcuA);
> 	[...]                                lock(srcuB);
> 	[...]                                lock(srcuA);
> 	[...]   lock(srcuB);
> 
> to
> 
> 	[...]  Possible unsafe locking scenario:
> 	[...]
> 	[...]        CPU0                    CPU1
> 	[...]        ----                    ----
> 	[...]   rlock(srcuA);
> 	[...]                                lock(srcuB);
> 	[...]                                lock(srcuA);
> 	[...]   sync(srcuB);
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  include/linux/lockdep.h  |  3 ++-
>  kernel/locking/lockdep.c | 48 ++++++++++++++++++++++++++--------------
>  2 files changed, 34 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> index 14d9dbedc6c1..b32256e9e944 100644
> --- a/include/linux/lockdep.h
> +++ b/include/linux/lockdep.h
> @@ -134,7 +134,8 @@ struct held_lock {
>  	unsigned int read:2;        /* see lock_acquire() comment */
>  	unsigned int check:1;       /* see lock_acquire() comment */
>  	unsigned int hardirqs_off:1;
> -	unsigned int references:12;					/* 32 bits */
> +	unsigned int sync:1;
> +	unsigned int references:11;					/* 32 bits */
>  	unsigned int pin_count;
>  };
>  

Yeah, I suppose we can do that -- another option is to steal some bits
from pin_count, but whatever (references used to be 11 a long while ago,
no problem going back to that).

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

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

* Re: [PATCH rcu 1/7] locking/lockdep: Introduce lock_sync()
  2023-03-17  3:13 ` [PATCH rcu 1/7] locking/lockdep: Introduce lock_sync() Boqun Feng
@ 2023-03-20 17:06   ` Davidlohr Bueso
  2023-03-20 17:50     ` Boqun Feng
  0 siblings, 1 reply; 20+ messages in thread
From: Davidlohr Bueso @ 2023-03-20 17:06 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rcu, Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Lai Jiangshan, Paul E. McKenney, Josh Triplett, Steven Rostedt,
	Mathieu Desnoyers, Frederic Weisbecker, Neeraj Upadhyay,
	Joel Fernandes, Shuah Khan, David Woodhouse, Paolo Bonzini, kvm,
	seanjc, linux-kernel, linux-kselftest

On Thu, 16 Mar 2023, Boqun Feng wrote:

>+/*
>+ * lock_sync() - A special annotation for synchronize_{s,}rcu()-like API.
>+ *
>+ * No actual critical section is created by the APIs annotated with this: these
>+ * APIs are used to wait for one or multiple critical sections (on other CPUs
>+ * or threads), and it means that calling these APIs inside these critical
>+ * sections is potential deadlock.
>+ *
>+ * This annotation acts as an acqurie+release anontation pair with hardirqoff
				^acquire

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

* Re: [PATCH rcu 4/7] locking/lockdep: Improve the deadlock scenario print for sync and read lock
  2023-03-20 12:13   ` Peter Zijlstra
@ 2023-03-20 17:50     ` Boqun Feng
  0 siblings, 0 replies; 20+ messages in thread
From: Boqun Feng @ 2023-03-20 17:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: rcu, Ingo Molnar, Will Deacon, Waiman Long, Lai Jiangshan,
	Paul E. McKenney, Josh Triplett, Steven Rostedt,
	Mathieu Desnoyers, Davidlohr Bueso, Frederic Weisbecker,
	Neeraj Upadhyay, Joel Fernandes, Shuah Khan, David Woodhouse,
	Paolo Bonzini, kvm, seanjc, linux-kernel, linux-kselftest

On Mon, Mar 20, 2023 at 01:13:05PM +0100, Peter Zijlstra wrote:
> On Thu, Mar 16, 2023 at 08:13:36PM -0700, Boqun Feng wrote:
> > Lock scenario print is always a weak spot of lockdep splats. Improvement
> > can be made if we rework the dependency search and the error printing.
> > 
> > However without touching the graph search, we can improve a little for
> > the circular deadlock case, since we have the to-be-added lock
> > dependency, and know whether these two locks are read/write/sync.
> > 
> > In order to know whether a held_lock is sync or not, a bit was
> > "stolen" from ->references, which reduce our limit for the same lock
> > class nesting from 2^12 to 2^11, and it should still be good enough.
> > 
> > Besides, since we now have bit in held_lock for sync, we don't need the
> > "hardirqoffs being 1" trick, and also we can avoid the __lock_release()
> > if we jump out of __lock_acquire() before the held_lock stored.
> > 
> > With these changes, a deadlock case evolved with read lock and sync gets
> > a better print-out from:
> > 
> > 	[...]  Possible unsafe locking scenario:
> > 	[...]
> > 	[...]        CPU0                    CPU1
> > 	[...]        ----                    ----
> > 	[...]   lock(srcuA);
> > 	[...]                                lock(srcuB);
> > 	[...]                                lock(srcuA);
> > 	[...]   lock(srcuB);
> > 
> > to
> > 
> > 	[...]  Possible unsafe locking scenario:
> > 	[...]
> > 	[...]        CPU0                    CPU1
> > 	[...]        ----                    ----
> > 	[...]   rlock(srcuA);
> > 	[...]                                lock(srcuB);
> > 	[...]                                lock(srcuA);
> > 	[...]   sync(srcuB);
> > 
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> >  include/linux/lockdep.h  |  3 ++-
> >  kernel/locking/lockdep.c | 48 ++++++++++++++++++++++++++--------------
> >  2 files changed, 34 insertions(+), 17 deletions(-)
> > 
> > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> > index 14d9dbedc6c1..b32256e9e944 100644
> > --- a/include/linux/lockdep.h
> > +++ b/include/linux/lockdep.h
> > @@ -134,7 +134,8 @@ struct held_lock {
> >  	unsigned int read:2;        /* see lock_acquire() comment */
> >  	unsigned int check:1;       /* see lock_acquire() comment */
> >  	unsigned int hardirqs_off:1;
> > -	unsigned int references:12;					/* 32 bits */
> > +	unsigned int sync:1;
> > +	unsigned int references:11;					/* 32 bits */
> >  	unsigned int pin_count;
> >  };
> >  
> 
> Yeah, I suppose we can do that -- another option is to steal some bits
> from pin_count, but whatever (references used to be 11 a long while ago,
> no problem going back to that).

Thanks!

> 
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Applied locally.

Regards,
Boqun

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

* Re: [PATCH rcu 1/7] locking/lockdep: Introduce lock_sync()
  2023-03-20 17:06   ` Davidlohr Bueso
@ 2023-03-20 17:50     ` Boqun Feng
  0 siblings, 0 replies; 20+ messages in thread
From: Boqun Feng @ 2023-03-20 17:50 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: rcu, Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Lai Jiangshan, Paul E. McKenney, Josh Triplett, Steven Rostedt,
	Mathieu Desnoyers, Frederic Weisbecker, Neeraj Upadhyay,
	Joel Fernandes, Shuah Khan, David Woodhouse, Paolo Bonzini, kvm,
	seanjc, linux-kernel, linux-kselftest

On Mon, Mar 20, 2023 at 10:06:14AM -0700, Davidlohr Bueso wrote:
> On Thu, 16 Mar 2023, Boqun Feng wrote:
> 
> > +/*
> > + * lock_sync() - A special annotation for synchronize_{s,}rcu()-like API.
> > + *
> > + * No actual critical section is created by the APIs annotated with this: these
> > + * APIs are used to wait for one or multiple critical sections (on other CPUs
> > + * or threads), and it means that calling these APIs inside these critical
> > + * sections is potential deadlock.
> > + *
> > + * This annotation acts as an acqurie+release anontation pair with hardirqoff
> 				^acquire

Good eye! Applied locally.

Regards,
Boqun

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

* Re: [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh
  2023-03-17  3:13 ` [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh Boqun Feng
@ 2023-03-20 18:19   ` Boqun Feng
  2023-03-20 19:09     ` Paul E. McKenney
  0 siblings, 1 reply; 20+ messages in thread
From: Boqun Feng @ 2023-03-20 18:19 UTC (permalink / raw)
  To: rcu
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Lai Jiangshan, Paul E. McKenney, Josh Triplett, Steven Rostedt,
	Mathieu Desnoyers, Davidlohr Bueso, Frederic Weisbecker,
	Neeraj Upadhyay, Joel Fernandes, Shuah Khan, David Woodhouse,
	Paolo Bonzini, kvm, seanjc, linux-kernel, linux-kselftest

Hi Paul,

On Thu, Mar 16, 2023 at 08:13:39PM -0700, Boqun Feng wrote:
> From: "Paul E. McKenney" <paulmck@kernel.org>
> 
> This commit adds an srcu_lockdep.sh script that checks whether lockdep
> correctly classifies SRCU-based, SRCU/mutex-based, and SRCU/rwsem-based
> deadlocks.
> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> [ boqun: Fix "RCUTORTURE" with "$RCUTORTURE" ]
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  .../selftests/rcutorture/bin/srcu_lockdep.sh  | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100755 tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> 
> diff --git a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> new file mode 100755
> index 000000000000..961932754684
> --- /dev/null
> +++ b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh

Could you provide the SPDX header and copyright bits for this newly
added file? For small changes I can do it myself, however this is about
licenses and copyright, so I need it from you, thanks!

Regards,
Boqun

> @@ -0,0 +1,73 @@
> +#!/bin/bash
> +#
> +# Run SRCU-lockdep tests and report any that fail to meet expectations.
> +
> +usage () {
> +	echo "Usage: $scriptname optional arguments:"
> +	echo "       --datestamp string"
> +	exit 1
> +}
> +
> +ds=`date +%Y.%m.%d-%H.%M.%S`-srcu_lockdep
> +scriptname="$0"
> +
> +T="`mktemp -d ${TMPDIR-/tmp}/srcu_lockdep.sh.XXXXXX`"
> +trap 'rm -rf $T' 0
> +
> +RCUTORTURE="`pwd`/tools/testing/selftests/rcutorture"; export RCUTORTURE
> +PATH=${RCUTORTURE}/bin:$PATH; export PATH
> +. functions.sh
> +
> +while test $# -gt 0
> +do
> +	case "$1" in
> +	--datestamp)
> +		checkarg --datestamp "(relative pathname)" "$#" "$2" '^[a-zA-Z0-9._/-]*$' '^--'
> +		ds=$2
> +		shift
> +		;;
> +	*)
> +		echo Unknown argument $1
> +		usage
> +		;;
> +	esac
> +	shift
> +done
> +
> +err=
> +nerrs=0
> +for d in 0 1
> +do
> +	for t in 0 1 2
> +	do
> +		for c in 1 2 3
> +		do
> +			err=
> +			val=$((d*1000+t*10+c))
> +			tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 5s --configs "SRCU-P" --bootargs "rcutorture.test_srcu_lockdep=$val" --trust-make --datestamp "$ds/$val" > "$T/kvm.sh.out" 2>&1
> +			ret=$?
> +			mv "$T/kvm.sh.out" "$RCUTORTURE/res/$ds/$val"
> +			if test "$d" -ne 0 && test "$ret" -eq 0
> +			then
> +				err=1
> +				echo -n Unexpected success for > "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> +			fi
> +			if test "$d" -eq 0 && test "$ret" -ne 0
> +			then
> +				err=1
> +				echo -n Unexpected failure for > "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> +			fi
> +			if test -n "$err"
> +			then
> +				grep "rcu_torture_init_srcu_lockdep: test_srcu_lockdep = " "$RCUTORTURE/res/$ds/$val/SRCU-P/console.log" | sed -e 's/^.*rcu_torture_init_srcu_lockdep://' >> "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> +				cat "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> +				nerrs=$((nerrs+1))
> +			fi
> +		done
> +	done
> +done
> +if test "$nerrs" -ne 0
> +then
> +	exit 1
> +fi
> +exit 0
> -- 
> 2.39.2
> 

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

* Re: [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh
  2023-03-20 18:19   ` Boqun Feng
@ 2023-03-20 19:09     ` Paul E. McKenney
  2023-03-20 19:28       ` Boqun Feng
  0 siblings, 1 reply; 20+ messages in thread
From: Paul E. McKenney @ 2023-03-20 19:09 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rcu, Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Lai Jiangshan, Josh Triplett, Steven Rostedt, Mathieu Desnoyers,
	Davidlohr Bueso, Frederic Weisbecker, Neeraj Upadhyay,
	Joel Fernandes, Shuah Khan, David Woodhouse, Paolo Bonzini, kvm,
	seanjc, linux-kernel, linux-kselftest

On Mon, Mar 20, 2023 at 11:19:05AM -0700, Boqun Feng wrote:
> Hi Paul,
> 
> On Thu, Mar 16, 2023 at 08:13:39PM -0700, Boqun Feng wrote:
> > From: "Paul E. McKenney" <paulmck@kernel.org>
> > 
> > This commit adds an srcu_lockdep.sh script that checks whether lockdep
> > correctly classifies SRCU-based, SRCU/mutex-based, and SRCU/rwsem-based
> > deadlocks.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > [ boqun: Fix "RCUTORTURE" with "$RCUTORTURE" ]
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> >  .../selftests/rcutorture/bin/srcu_lockdep.sh  | 73 +++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >  create mode 100755 tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > 
> > diff --git a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > new file mode 100755
> > index 000000000000..961932754684
> > --- /dev/null
> > +++ b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> 
> Could you provide the SPDX header and copyright bits for this newly
> added file? For small changes I can do it myself, however this is about
> licenses and copyright, so I need it from you, thanks!

Good catch, thank you!

Would you like a delta patch to merge into your existing one, or would
you prefer a replacement patch?  Either way works for me.

							Thanx, Paul

> Regards,
> Boqun
> 
> > @@ -0,0 +1,73 @@
> > +#!/bin/bash
> > +#
> > +# Run SRCU-lockdep tests and report any that fail to meet expectations.
> > +
> > +usage () {
> > +	echo "Usage: $scriptname optional arguments:"
> > +	echo "       --datestamp string"
> > +	exit 1
> > +}
> > +
> > +ds=`date +%Y.%m.%d-%H.%M.%S`-srcu_lockdep
> > +scriptname="$0"
> > +
> > +T="`mktemp -d ${TMPDIR-/tmp}/srcu_lockdep.sh.XXXXXX`"
> > +trap 'rm -rf $T' 0
> > +
> > +RCUTORTURE="`pwd`/tools/testing/selftests/rcutorture"; export RCUTORTURE
> > +PATH=${RCUTORTURE}/bin:$PATH; export PATH
> > +. functions.sh
> > +
> > +while test $# -gt 0
> > +do
> > +	case "$1" in
> > +	--datestamp)
> > +		checkarg --datestamp "(relative pathname)" "$#" "$2" '^[a-zA-Z0-9._/-]*$' '^--'
> > +		ds=$2
> > +		shift
> > +		;;
> > +	*)
> > +		echo Unknown argument $1
> > +		usage
> > +		;;
> > +	esac
> > +	shift
> > +done
> > +
> > +err=
> > +nerrs=0
> > +for d in 0 1
> > +do
> > +	for t in 0 1 2
> > +	do
> > +		for c in 1 2 3
> > +		do
> > +			err=
> > +			val=$((d*1000+t*10+c))
> > +			tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 5s --configs "SRCU-P" --bootargs "rcutorture.test_srcu_lockdep=$val" --trust-make --datestamp "$ds/$val" > "$T/kvm.sh.out" 2>&1
> > +			ret=$?
> > +			mv "$T/kvm.sh.out" "$RCUTORTURE/res/$ds/$val"
> > +			if test "$d" -ne 0 && test "$ret" -eq 0
> > +			then
> > +				err=1
> > +				echo -n Unexpected success for > "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> > +			fi
> > +			if test "$d" -eq 0 && test "$ret" -ne 0
> > +			then
> > +				err=1
> > +				echo -n Unexpected failure for > "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> > +			fi
> > +			if test -n "$err"
> > +			then
> > +				grep "rcu_torture_init_srcu_lockdep: test_srcu_lockdep = " "$RCUTORTURE/res/$ds/$val/SRCU-P/console.log" | sed -e 's/^.*rcu_torture_init_srcu_lockdep://' >> "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> > +				cat "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> > +				nerrs=$((nerrs+1))
> > +			fi
> > +		done
> > +	done
> > +done
> > +if test "$nerrs" -ne 0
> > +then
> > +	exit 1
> > +fi
> > +exit 0
> > -- 
> > 2.39.2
> > 

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

* Re: [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh
  2023-03-20 19:09     ` Paul E. McKenney
@ 2023-03-20 19:28       ` Boqun Feng
  2023-03-20 20:22         ` Paul E. McKenney
  0 siblings, 1 reply; 20+ messages in thread
From: Boqun Feng @ 2023-03-20 19:28 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: rcu, Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Lai Jiangshan, Josh Triplett, Steven Rostedt, Mathieu Desnoyers,
	Davidlohr Bueso, Frederic Weisbecker, Neeraj Upadhyay,
	Joel Fernandes, Shuah Khan, David Woodhouse, Paolo Bonzini, kvm,
	seanjc, linux-kernel, linux-kselftest

On Mon, Mar 20, 2023 at 12:09:00PM -0700, Paul E. McKenney wrote:
> On Mon, Mar 20, 2023 at 11:19:05AM -0700, Boqun Feng wrote:
> > Hi Paul,
> > 
> > On Thu, Mar 16, 2023 at 08:13:39PM -0700, Boqun Feng wrote:
> > > From: "Paul E. McKenney" <paulmck@kernel.org>
> > > 
> > > This commit adds an srcu_lockdep.sh script that checks whether lockdep
> > > correctly classifies SRCU-based, SRCU/mutex-based, and SRCU/rwsem-based
> > > deadlocks.
> > > 
> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > [ boqun: Fix "RCUTORTURE" with "$RCUTORTURE" ]
> > > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > > ---
> > >  .../selftests/rcutorture/bin/srcu_lockdep.sh  | 73 +++++++++++++++++++
> > >  1 file changed, 73 insertions(+)
> > >  create mode 100755 tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > > 
> > > diff --git a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > > new file mode 100755
> > > index 000000000000..961932754684
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > 
> > Could you provide the SPDX header and copyright bits for this newly
> > added file? For small changes I can do it myself, however this is about
> > licenses and copyright, so I need it from you, thanks!
> 
> Good catch, thank you!
> 
> Would you like a delta patch to merge into your existing one, or would
> you prefer a replacement patch?  Either way works for me.
> 

A delta patch if that's not much trouble. I will fold it into this one.

Regards,
Boqun

> 							Thanx, Paul
> 
> > Regards,
> > Boqun
> > 
> > > @@ -0,0 +1,73 @@
> > > +#!/bin/bash
> > > +#
> > > +# Run SRCU-lockdep tests and report any that fail to meet expectations.
> > > +
> > > +usage () {
> > > +	echo "Usage: $scriptname optional arguments:"
> > > +	echo "       --datestamp string"
> > > +	exit 1
> > > +}
> > > +
> > > +ds=`date +%Y.%m.%d-%H.%M.%S`-srcu_lockdep
> > > +scriptname="$0"
> > > +
> > > +T="`mktemp -d ${TMPDIR-/tmp}/srcu_lockdep.sh.XXXXXX`"
> > > +trap 'rm -rf $T' 0
> > > +
> > > +RCUTORTURE="`pwd`/tools/testing/selftests/rcutorture"; export RCUTORTURE
> > > +PATH=${RCUTORTURE}/bin:$PATH; export PATH
> > > +. functions.sh
> > > +
> > > +while test $# -gt 0
> > > +do
> > > +	case "$1" in
> > > +	--datestamp)
> > > +		checkarg --datestamp "(relative pathname)" "$#" "$2" '^[a-zA-Z0-9._/-]*$' '^--'
> > > +		ds=$2
> > > +		shift
> > > +		;;
> > > +	*)
> > > +		echo Unknown argument $1
> > > +		usage
> > > +		;;
> > > +	esac
> > > +	shift
> > > +done
> > > +
> > > +err=
> > > +nerrs=0
> > > +for d in 0 1
> > > +do
> > > +	for t in 0 1 2
> > > +	do
> > > +		for c in 1 2 3
> > > +		do
> > > +			err=
> > > +			val=$((d*1000+t*10+c))
> > > +			tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 5s --configs "SRCU-P" --bootargs "rcutorture.test_srcu_lockdep=$val" --trust-make --datestamp "$ds/$val" > "$T/kvm.sh.out" 2>&1
> > > +			ret=$?
> > > +			mv "$T/kvm.sh.out" "$RCUTORTURE/res/$ds/$val"
> > > +			if test "$d" -ne 0 && test "$ret" -eq 0
> > > +			then
> > > +				err=1
> > > +				echo -n Unexpected success for > "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> > > +			fi
> > > +			if test "$d" -eq 0 && test "$ret" -ne 0
> > > +			then
> > > +				err=1
> > > +				echo -n Unexpected failure for > "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> > > +			fi
> > > +			if test -n "$err"
> > > +			then
> > > +				grep "rcu_torture_init_srcu_lockdep: test_srcu_lockdep = " "$RCUTORTURE/res/$ds/$val/SRCU-P/console.log" | sed -e 's/^.*rcu_torture_init_srcu_lockdep://' >> "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> > > +				cat "$RCUTORTURE/res/$ds/$val/kvm.sh.err"
> > > +				nerrs=$((nerrs+1))
> > > +			fi
> > > +		done
> > > +	done
> > > +done
> > > +if test "$nerrs" -ne 0
> > > +then
> > > +	exit 1
> > > +fi
> > > +exit 0
> > > -- 
> > > 2.39.2
> > > 

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

* Re: [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh
  2023-03-20 19:28       ` Boqun Feng
@ 2023-03-20 20:22         ` Paul E. McKenney
  2023-03-20 20:26           ` Boqun Feng
  0 siblings, 1 reply; 20+ messages in thread
From: Paul E. McKenney @ 2023-03-20 20:22 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rcu, Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Lai Jiangshan, Josh Triplett, Steven Rostedt, Mathieu Desnoyers,
	Davidlohr Bueso, Frederic Weisbecker, Neeraj Upadhyay,
	Joel Fernandes, Shuah Khan, David Woodhouse, Paolo Bonzini, kvm,
	seanjc, linux-kernel, linux-kselftest

On Mon, Mar 20, 2023 at 12:28:05PM -0700, Boqun Feng wrote:
> On Mon, Mar 20, 2023 at 12:09:00PM -0700, Paul E. McKenney wrote:
> > On Mon, Mar 20, 2023 at 11:19:05AM -0700, Boqun Feng wrote:
> > > Hi Paul,
> > > 
> > > On Thu, Mar 16, 2023 at 08:13:39PM -0700, Boqun Feng wrote:
> > > > From: "Paul E. McKenney" <paulmck@kernel.org>
> > > > 
> > > > This commit adds an srcu_lockdep.sh script that checks whether lockdep
> > > > correctly classifies SRCU-based, SRCU/mutex-based, and SRCU/rwsem-based
> > > > deadlocks.
> > > > 
> > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > > [ boqun: Fix "RCUTORTURE" with "$RCUTORTURE" ]
> > > > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > > > ---
> > > >  .../selftests/rcutorture/bin/srcu_lockdep.sh  | 73 +++++++++++++++++++
> > > >  1 file changed, 73 insertions(+)
> > > >  create mode 100755 tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > > > 
> > > > diff --git a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > > > new file mode 100755
> > > > index 000000000000..961932754684
> > > > --- /dev/null
> > > > +++ b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > > 
> > > Could you provide the SPDX header and copyright bits for this newly
> > > added file? For small changes I can do it myself, however this is about
> > > licenses and copyright, so I need it from you, thanks!
> > 
> > Good catch, thank you!
> > 
> > Would you like a delta patch to merge into your existing one, or would
> > you prefer a replacement patch?  Either way works for me.
> > 
> 
> A delta patch if that's not much trouble. I will fold it into this one.

Here you go!

							Thanx, Paul

------------------------------------------------------------------------

rcutorture: Add proper comment header to srcu_lockdep.sh

This patch adds a proper comment header to srcu_lockdep.sh,
and is intended to be folded into 9dc68f40c665 ("rcutorture: Add
srcu_lockdep.sh").

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

diff --git a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
index 961932754684..2e63ef009d59 100755
--- a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
+++ b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
@@ -1,6 +1,11 @@
 #!/bin/bash
+# SPDX-License-Identifier: GPL-2.0+
 #
 # Run SRCU-lockdep tests and report any that fail to meet expectations.
+#
+# Copyright (C) 2021 Meta Platforms, Inc.
+#
+# Authors: Paul E. McKenney <paulmck@kernel.org>
 
 usage () {
 	echo "Usage: $scriptname optional arguments:"

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

* Re: [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh
  2023-03-20 20:22         ` Paul E. McKenney
@ 2023-03-20 20:26           ` Boqun Feng
  2023-03-20 20:36             ` Paul E. McKenney
  0 siblings, 1 reply; 20+ messages in thread
From: Boqun Feng @ 2023-03-20 20:26 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: rcu, Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Lai Jiangshan, Josh Triplett, Steven Rostedt, Mathieu Desnoyers,
	Davidlohr Bueso, Frederic Weisbecker, Neeraj Upadhyay,
	Joel Fernandes, Shuah Khan, David Woodhouse, Paolo Bonzini, kvm,
	seanjc, linux-kernel, linux-kselftest

On Mon, Mar 20, 2023 at 01:22:24PM -0700, Paul E. McKenney wrote:
> On Mon, Mar 20, 2023 at 12:28:05PM -0700, Boqun Feng wrote:
> > On Mon, Mar 20, 2023 at 12:09:00PM -0700, Paul E. McKenney wrote:
> > > On Mon, Mar 20, 2023 at 11:19:05AM -0700, Boqun Feng wrote:
> > > > Hi Paul,
> > > > 
> > > > On Thu, Mar 16, 2023 at 08:13:39PM -0700, Boqun Feng wrote:
> > > > > From: "Paul E. McKenney" <paulmck@kernel.org>
> > > > > 
> > > > > This commit adds an srcu_lockdep.sh script that checks whether lockdep
> > > > > correctly classifies SRCU-based, SRCU/mutex-based, and SRCU/rwsem-based
> > > > > deadlocks.
> > > > > 
> > > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > > > [ boqun: Fix "RCUTORTURE" with "$RCUTORTURE" ]
> > > > > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > > > > ---
> > > > >  .../selftests/rcutorture/bin/srcu_lockdep.sh  | 73 +++++++++++++++++++
> > > > >  1 file changed, 73 insertions(+)
> > > > >  create mode 100755 tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > > > > 
> > > > > diff --git a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > > > > new file mode 100755
> > > > > index 000000000000..961932754684
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > > > 
> > > > Could you provide the SPDX header and copyright bits for this newly
> > > > added file? For small changes I can do it myself, however this is about
> > > > licenses and copyright, so I need it from you, thanks!
> > > 
> > > Good catch, thank you!
> > > 
> > > Would you like a delta patch to merge into your existing one, or would
> > > you prefer a replacement patch?  Either way works for me.
> > > 
> > 
> > A delta patch if that's not much trouble. I will fold it into this one.
> 
> Here you go!
> 

Thanks! Fold it in patch #7.

Regards,
Boqun

> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> rcutorture: Add proper comment header to srcu_lockdep.sh
> 
> This patch adds a proper comment header to srcu_lockdep.sh,
> and is intended to be folded into 9dc68f40c665 ("rcutorture: Add
> srcu_lockdep.sh").
> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> 
> diff --git a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> index 961932754684..2e63ef009d59 100755
> --- a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> +++ b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> @@ -1,6 +1,11 @@
>  #!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0+
>  #
>  # Run SRCU-lockdep tests and report any that fail to meet expectations.
> +#
> +# Copyright (C) 2021 Meta Platforms, Inc.
> +#
> +# Authors: Paul E. McKenney <paulmck@kernel.org>
>  
>  usage () {
>  	echo "Usage: $scriptname optional arguments:"

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

* Re: [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh
  2023-03-20 20:26           ` Boqun Feng
@ 2023-03-20 20:36             ` Paul E. McKenney
  0 siblings, 0 replies; 20+ messages in thread
From: Paul E. McKenney @ 2023-03-20 20:36 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rcu, Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Lai Jiangshan, Josh Triplett, Steven Rostedt, Mathieu Desnoyers,
	Davidlohr Bueso, Frederic Weisbecker, Neeraj Upadhyay,
	Joel Fernandes, Shuah Khan, David Woodhouse, Paolo Bonzini, kvm,
	seanjc, linux-kernel, linux-kselftest

On Mon, Mar 20, 2023 at 01:26:39PM -0700, Boqun Feng wrote:
> On Mon, Mar 20, 2023 at 01:22:24PM -0700, Paul E. McKenney wrote:
> > On Mon, Mar 20, 2023 at 12:28:05PM -0700, Boqun Feng wrote:
> > > On Mon, Mar 20, 2023 at 12:09:00PM -0700, Paul E. McKenney wrote:
> > > > On Mon, Mar 20, 2023 at 11:19:05AM -0700, Boqun Feng wrote:
> > > > > Hi Paul,
> > > > > 
> > > > > On Thu, Mar 16, 2023 at 08:13:39PM -0700, Boqun Feng wrote:
> > > > > > From: "Paul E. McKenney" <paulmck@kernel.org>
> > > > > > 
> > > > > > This commit adds an srcu_lockdep.sh script that checks whether lockdep
> > > > > > correctly classifies SRCU-based, SRCU/mutex-based, and SRCU/rwsem-based
> > > > > > deadlocks.
> > > > > > 
> > > > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > > > > [ boqun: Fix "RCUTORTURE" with "$RCUTORTURE" ]
> > > > > > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > > > > > ---
> > > > > >  .../selftests/rcutorture/bin/srcu_lockdep.sh  | 73 +++++++++++++++++++
> > > > > >  1 file changed, 73 insertions(+)
> > > > > >  create mode 100755 tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > > > > > 
> > > > > > diff --git a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > > > > > new file mode 100755
> > > > > > index 000000000000..961932754684
> > > > > > --- /dev/null
> > > > > > +++ b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > > > > 
> > > > > Could you provide the SPDX header and copyright bits for this newly
> > > > > added file? For small changes I can do it myself, however this is about
> > > > > licenses and copyright, so I need it from you, thanks!
> > > > 
> > > > Good catch, thank you!
> > > > 
> > > > Would you like a delta patch to merge into your existing one, or would
> > > > you prefer a replacement patch?  Either way works for me.
> > > > 
> > > 
> > > A delta patch if that's not much trouble. I will fold it into this one.
> > 
> > Here you go!
> 
> Thanks! Fold it in patch #7.

Thank you!

And this might be a problem in my current process when handing off
to others.  I normally don't run checkpatch until just before a send
out the patches, which means that none of the commits I handed off to
you guys had been checkpatched.  There was I time I tried a checkpatch
hook in git, but the false-positive rate made that a non-starter.

							Thanx, Paul

> Regards,
> Boqun
> 
> > 							Thanx, Paul
> > 
> > ------------------------------------------------------------------------
> > 
> > rcutorture: Add proper comment header to srcu_lockdep.sh
> > 
> > This patch adds a proper comment header to srcu_lockdep.sh,
> > and is intended to be folded into 9dc68f40c665 ("rcutorture: Add
> > srcu_lockdep.sh").
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > 
> > diff --git a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > index 961932754684..2e63ef009d59 100755
> > --- a/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > +++ b/tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh
> > @@ -1,6 +1,11 @@
> >  #!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0+
> >  #
> >  # Run SRCU-lockdep tests and report any that fail to meet expectations.
> > +#
> > +# Copyright (C) 2021 Meta Platforms, Inc.
> > +#
> > +# Authors: Paul E. McKenney <paulmck@kernel.org>
> >  
> >  usage () {
> >  	echo "Usage: $scriptname optional arguments:"

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

end of thread, other threads:[~2023-03-20 20:38 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-17  3:13 [PATCH rcu 0/7] RCU-related lockdep changes for v6.4 Boqun Feng
2023-03-17  3:13 ` [PATCH rcu 1/7] locking/lockdep: Introduce lock_sync() Boqun Feng
2023-03-20 17:06   ` Davidlohr Bueso
2023-03-20 17:50     ` Boqun Feng
2023-03-17  3:13 ` [PATCH rcu 2/7] rcu: Annotate SRCU's update-side lockdep dependencies Boqun Feng
2023-03-17  3:13 ` [PATCH rcu 3/7] locking: Reduce the number of locks in ww_mutex stress tests Boqun Feng
2023-03-17 18:38   ` Paul E. McKenney
2023-03-17 21:26     ` Boqun Feng
2023-03-17  3:13 ` [PATCH rcu 4/7] locking/lockdep: Improve the deadlock scenario print for sync and read lock Boqun Feng
2023-03-20 12:13   ` Peter Zijlstra
2023-03-20 17:50     ` Boqun Feng
2023-03-17  3:13 ` [PATCH rcu 5/7] rcutorture: Add SRCU deadlock scenarios Boqun Feng
2023-03-17  3:13 ` [PATCH rcu 6/7] rcutorture: Add RCU Tasks Trace and " Boqun Feng
2023-03-17  3:13 ` [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh Boqun Feng
2023-03-20 18:19   ` Boqun Feng
2023-03-20 19:09     ` Paul E. McKenney
2023-03-20 19:28       ` Boqun Feng
2023-03-20 20:22         ` Paul E. McKenney
2023-03-20 20:26           ` Boqun Feng
2023-03-20 20:36             ` Paul E. McKenney

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).