All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter
@ 2023-02-02 22:34 John Stultz
  2023-02-02 22:34 ` [RFC][PATCH 2/4] locktorture: Add nested locking to mutex torture tests John Stultz
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: John Stultz @ 2023-02-02 22:34 UTC (permalink / raw)
  To: LKML
  Cc: John Stultz, Davidlohr Bueso, Paul E. McKenney, Josh Triplett,
	Joel Fernandes, Juri Lelli, Valentin Schneider, Dietmar Eggemann

In order ot extend locktorture to support lock nesting, add
nested_lock() and nested_unlock() hooks to the torture ops.

These take a 32bit lockset mask which is generated at random,
so some number of locks will be taken before the main lock is
taken and released afterwards.

Additionally, add nlocks module parameter to allow specifying
the number of nested locks to be used.

This has been helpful to uncover issues in the proxy-exec
series development.

This was inspired by locktorture extensions originally implemented
by Connor O'Brien, for stress testing the proxy-execution series:
  https://lore.kernel.org/lkml/20221003214501.2050087-12-connoro@google.com/

Comments or feedback would be greatly appreciated!

Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Signed-off-by: John Stultz <jstultz@google.com>
---
 kernel/locking/locktorture.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 9c2fb613a55d..f4fbd3194654 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -48,6 +48,9 @@ torture_param(int, stat_interval, 60,
 torture_param(int, stutter, 5, "Number of jiffies to run/halt test, 0=disable");
 torture_param(int, verbose, 1,
 	     "Enable verbose debugging printk()s");
+torture_param(int, nlocks, 0, "Number of nested locks");
+/* Going much higher trips "BUG: MAX_LOCKDEP_CHAIN_HLOCKS too low!" errors */
+#define MAX_LOCKS 8
 
 static char *torture_type = "spin_lock";
 module_param(torture_type, charp, 0444);
@@ -76,10 +79,12 @@ static void lock_torture_cleanup(void);
 struct lock_torture_ops {
 	void (*init)(void);
 	void (*exit)(void);
+	int (*nested_lock)(int tid, u32 lockset);
 	int (*writelock)(int tid);
 	void (*write_delay)(struct torture_random_state *trsp);
 	void (*task_boost)(struct torture_random_state *trsp);
 	void (*writeunlock)(int tid);
+	void (*nested_unlock)(int tid, u32 lockset);
 	int (*readlock)(int tid);
 	void (*read_delay)(struct torture_random_state *trsp);
 	void (*readunlock)(int tid);
@@ -669,6 +674,7 @@ static int lock_torture_writer(void *arg)
 	struct lock_stress_stats *lwsp = arg;
 	int tid = lwsp - cxt.lwsa;
 	DEFINE_TORTURE_RANDOM(rand);
+	u32 lockset_mask;
 
 	VERBOSE_TOROUT_STRING("lock_torture_writer task started");
 	set_user_nice(current, MAX_NICE);
@@ -677,7 +683,10 @@ static int lock_torture_writer(void *arg)
 		if ((torture_random(&rand) & 0xfffff) == 0)
 			schedule_timeout_uninterruptible(1);
 
+		lockset_mask = torture_random(&rand);
 		cxt.cur_ops->task_boost(&rand);
+		if (cxt.cur_ops->nested_lock)
+			cxt.cur_ops->nested_lock(tid, lockset_mask);
 		cxt.cur_ops->writelock(tid);
 		if (WARN_ON_ONCE(lock_is_write_held))
 			lwsp->n_lock_fail++;
@@ -690,6 +699,8 @@ static int lock_torture_writer(void *arg)
 		lock_is_write_held = false;
 		WRITE_ONCE(last_lock_release, jiffies);
 		cxt.cur_ops->writeunlock(tid);
+		if (cxt.cur_ops->nested_unlock)
+			cxt.cur_ops->nested_unlock(tid, lockset_mask);
 
 		stutter_wait("lock_torture_writer");
 	} while (!torture_must_stop());
@@ -830,11 +841,11 @@ lock_torture_print_module_parms(struct lock_torture_ops *cur_ops,
 				const char *tag)
 {
 	pr_alert("%s" TORTURE_FLAG
-		 "--- %s%s: nwriters_stress=%d nreaders_stress=%d stat_interval=%d verbose=%d shuffle_interval=%d stutter=%d shutdown_secs=%d onoff_interval=%d onoff_holdoff=%d\n",
+		 "--- %s%s: nwriters_stress=%d nreaders_stress=%d nlocks=%d stat_interval=%d verbose=%d shuffle_interval=%d stutter=%d shutdown_secs=%d onoff_interval=%d onoff_holdoff=%d\n",
 		 torture_type, tag, cxt.debug_lock ? " [debug]": "",
-		 cxt.nrealwriters_stress, cxt.nrealreaders_stress, stat_interval,
-		 verbose, shuffle_interval, stutter, shutdown_secs,
-		 onoff_interval, onoff_holdoff);
+		 cxt.nrealwriters_stress, cxt.nrealreaders_stress, nlocks,
+		 stat_interval, verbose, shuffle_interval, stutter,
+		 shutdown_secs, onoff_interval, onoff_holdoff);
 }
 
 static void lock_torture_cleanup(void)
@@ -1053,6 +1064,10 @@ static int __init lock_torture_init(void)
 		}
 	}
 
+	/* cap nlocks to MAX_LOCKS */
+	if (nlocks > MAX_LOCKS)
+		nlocks = MAX_LOCKS;
+
 	if (cxt.cur_ops->readlock) {
 		reader_tasks = kcalloc(cxt.nrealreaders_stress,
 				       sizeof(reader_tasks[0]),
-- 
2.39.1.519.gcb327c4b5f-goog


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

* [RFC][PATCH 2/4] locktorture: Add nested locking to mutex torture tests
  2023-02-02 22:34 [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter John Stultz
@ 2023-02-02 22:34 ` John Stultz
  2023-02-02 22:34 ` [RFC][PATCH 3/4] locktorture: Add nested locking to rtmutex " John Stultz
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: John Stultz @ 2023-02-02 22:34 UTC (permalink / raw)
  To: LKML
  Cc: John Stultz, Davidlohr Bueso, Paul E. McKenney, Josh Triplett,
	Joel Fernandes, Juri Lelli, Valentin Schneider, Dietmar Eggemann

This patch adds randomized nested locking to the mutex torture
tests.

This was inspired by locktorture extensions originally implemented
by Connor O'Brien, for stress testing the proxy-execution series:
  https://lore.kernel.org/lkml/20221003214501.2050087-12-connoro@google.com/

Comments or feedback would be greatly appreciated!

Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Signed-off-by: John Stultz <jstultz@google.com>
---
 kernel/locking/locktorture.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index f4fbd3194654..27d92ce36836 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -332,6 +332,28 @@ static struct lock_torture_ops rw_lock_irq_ops = {
 };
 
 static DEFINE_MUTEX(torture_mutex);
+static struct mutex torture_nested_mutexes[MAX_LOCKS];
+static struct lock_class_key nested_mutex_keys[MAX_LOCKS];
+
+static void torture_mutex_init(void)
+{
+	int i;
+
+	for (i = 0; i < MAX_LOCKS; i++)
+		__mutex_init(&torture_nested_mutexes[i], __func__,
+			     &nested_mutex_keys[i]);
+}
+
+static int torture_mutex_nested_lock(int tid __maybe_unused,
+				     u32 lockset)
+{
+	int i;
+
+	for (i = 0; i < nlocks; i++)
+		if (lockset & (1 << i))
+			mutex_lock(&torture_nested_mutexes[i]);
+	return 0;
+}
 
 static int torture_mutex_lock(int tid __maybe_unused)
 __acquires(torture_mutex)
@@ -360,11 +382,24 @@ __releases(torture_mutex)
 	mutex_unlock(&torture_mutex);
 }
 
+static void torture_mutex_nested_unlock(int tid __maybe_unused,
+					u32 lockset)
+{
+	int i;
+
+	for (i = nlocks - 1; i >= 0; i--)
+		if (lockset & (1 << i))
+			mutex_unlock(&torture_nested_mutexes[i]);
+}
+
 static struct lock_torture_ops mutex_lock_ops = {
+	.init		= torture_mutex_init,
+	.nested_lock	= torture_mutex_nested_lock,
 	.writelock	= torture_mutex_lock,
 	.write_delay	= torture_mutex_delay,
 	.task_boost     = torture_boost_dummy,
 	.writeunlock	= torture_mutex_unlock,
+	.nested_unlock	= torture_mutex_nested_unlock,
 	.readlock       = NULL,
 	.read_delay     = NULL,
 	.readunlock     = NULL,
-- 
2.39.1.519.gcb327c4b5f-goog


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

* [RFC][PATCH 3/4] locktorture: Add nested locking to rtmutex torture tests
  2023-02-02 22:34 [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter John Stultz
  2023-02-02 22:34 ` [RFC][PATCH 2/4] locktorture: Add nested locking to mutex torture tests John Stultz
@ 2023-02-02 22:34 ` John Stultz
  2023-02-02 22:34 ` [RFC][PATCH 4/4] locktorture: With nested locks, occasionally skip main lock John Stultz
  2023-02-03  2:05 ` [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter Paul E. McKenney
  3 siblings, 0 replies; 7+ messages in thread
From: John Stultz @ 2023-02-02 22:34 UTC (permalink / raw)
  To: LKML
  Cc: John Stultz, Davidlohr Bueso, Paul E. McKenney, Josh Triplett,
	Joel Fernandes, Juri Lelli, Valentin Schneider, Dietmar Eggemann

This patch adds randomized nested locking to the mutex torture
tests.

This was inspired by locktorture extensions originally implemented
by Connor O'Brien, for stress testing the proxy-execution series:
  https://lore.kernel.org/lkml/20221003214501.2050087-12-connoro@google.com/

Comments or feedback would be greatly appreciated!

Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Signed-off-by: John Stultz <jstultz@google.com>
---
 kernel/locking/locktorture.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 27d92ce36836..5fb17a5057b5 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -506,6 +506,28 @@ static struct lock_torture_ops ww_mutex_lock_ops = {
 
 #ifdef CONFIG_RT_MUTEXES
 static DEFINE_RT_MUTEX(torture_rtmutex);
+static struct rt_mutex torture_nested_rtmutexes[MAX_LOCKS];
+static struct lock_class_key nested_rtmutex_keys[MAX_LOCKS];
+
+static void torture_rtmutex_init(void)
+{
+	int i;
+
+	for (i = 0; i < MAX_LOCKS; i++)
+		__rt_mutex_init(&torture_nested_rtmutexes[i], __func__,
+				&nested_rtmutex_keys[i]);
+}
+
+static int torture_rtmutex_nested_lock(int tid __maybe_unused,
+				       u32 lockset)
+{
+	int i;
+
+	for (i = 0; i < nlocks; i++)
+		if (lockset & (1 << i))
+			rt_mutex_lock(&torture_nested_rtmutexes[i]);
+	return 0;
+}
 
 static int torture_rtmutex_lock(int tid __maybe_unused)
 __acquires(torture_rtmutex)
@@ -570,11 +592,24 @@ __releases(torture_rtmutex)
 	rt_mutex_unlock(&torture_rtmutex);
 }
 
+static void torture_rtmutex_nested_unlock(int tid __maybe_unused,
+					  u32 lockset)
+{
+	int i;
+
+	for (i = nlocks - 1; i >= 0; i--)
+		if (lockset & (1 << i))
+			rt_mutex_unlock(&torture_nested_rtmutexes[i]);
+}
+
 static struct lock_torture_ops rtmutex_lock_ops = {
+	.init		= torture_rtmutex_init,
+	.nested_lock	= torture_rtmutex_nested_lock,
 	.writelock	= torture_rtmutex_lock,
 	.write_delay	= torture_rtmutex_delay,
 	.task_boost     = torture_rtmutex_boost,
 	.writeunlock	= torture_rtmutex_unlock,
+	.nested_unlock	= torture_rtmutex_nested_unlock,
 	.readlock       = NULL,
 	.read_delay     = NULL,
 	.readunlock     = NULL,
-- 
2.39.1.519.gcb327c4b5f-goog


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

* [RFC][PATCH 4/4] locktorture: With nested locks, occasionally skip main lock
  2023-02-02 22:34 [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter John Stultz
  2023-02-02 22:34 ` [RFC][PATCH 2/4] locktorture: Add nested locking to mutex torture tests John Stultz
  2023-02-02 22:34 ` [RFC][PATCH 3/4] locktorture: Add nested locking to rtmutex " John Stultz
@ 2023-02-02 22:34 ` John Stultz
  2023-02-03  2:05 ` [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter Paul E. McKenney
  3 siblings, 0 replies; 7+ messages in thread
From: John Stultz @ 2023-02-02 22:34 UTC (permalink / raw)
  To: LKML
  Cc: John Stultz, Davidlohr Bueso, Paul E. McKenney, Josh Triplett,
	Joel Fernandes, Juri Lelli, Valentin Schneider, Dietmar Eggemann

If we're using nested locking to stress things, occasionally
skip taking the main lock, so that we can get some different
contention patterns between the writers (to hopefully get two
disjoint blocked trees)

This patch was inspired by earlier work by Connor O'Brien.

Comments or feedback would be greatly appreciated!

Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Signed-off-by: John Stultz <jstultz@google.com>
---
 kernel/locking/locktorture.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 5fb17a5057b5..6f56dcb8a496 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -745,6 +745,7 @@ static int lock_torture_writer(void *arg)
 	int tid = lwsp - cxt.lwsa;
 	DEFINE_TORTURE_RANDOM(rand);
 	u32 lockset_mask;
+	bool skip_main_lock;
 
 	VERBOSE_TOROUT_STRING("lock_torture_writer task started");
 	set_user_nice(current, MAX_NICE);
@@ -754,21 +755,28 @@ static int lock_torture_writer(void *arg)
 			schedule_timeout_uninterruptible(1);
 
 		lockset_mask = torture_random(&rand);
+		skip_main_lock = nlocks && !(torture_random(&rand) % 100);
+
 		cxt.cur_ops->task_boost(&rand);
 		if (cxt.cur_ops->nested_lock)
 			cxt.cur_ops->nested_lock(tid, lockset_mask);
-		cxt.cur_ops->writelock(tid);
-		if (WARN_ON_ONCE(lock_is_write_held))
-			lwsp->n_lock_fail++;
-		lock_is_write_held = true;
-		if (WARN_ON_ONCE(atomic_read(&lock_is_read_held)))
-			lwsp->n_lock_fail++; /* rare, but... */
 
-		lwsp->n_lock_acquired++;
+		if (!skip_main_lock) {
+			cxt.cur_ops->writelock(tid);
+			if (WARN_ON_ONCE(lock_is_write_held))
+				lwsp->n_lock_fail++;
+			lock_is_write_held = true;
+			if (WARN_ON_ONCE(atomic_read(&lock_is_read_held)))
+				lwsp->n_lock_fail++; /* rare, but... */
+
+			lwsp->n_lock_acquired++;
+		}
 		cxt.cur_ops->write_delay(&rand);
-		lock_is_write_held = false;
-		WRITE_ONCE(last_lock_release, jiffies);
-		cxt.cur_ops->writeunlock(tid);
+		if (!skip_main_lock) {
+			lock_is_write_held = false;
+			WRITE_ONCE(last_lock_release, jiffies);
+			cxt.cur_ops->writeunlock(tid);
+		}
 		if (cxt.cur_ops->nested_unlock)
 			cxt.cur_ops->nested_unlock(tid, lockset_mask);
 
-- 
2.39.1.519.gcb327c4b5f-goog


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

* Re: [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter
  2023-02-02 22:34 [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter John Stultz
                   ` (2 preceding siblings ...)
  2023-02-02 22:34 ` [RFC][PATCH 4/4] locktorture: With nested locks, occasionally skip main lock John Stultz
@ 2023-02-03  2:05 ` Paul E. McKenney
  2023-02-03  3:56   ` John Stultz
  3 siblings, 1 reply; 7+ messages in thread
From: Paul E. McKenney @ 2023-02-03  2:05 UTC (permalink / raw)
  To: John Stultz
  Cc: LKML, Davidlohr Bueso, Josh Triplett, Joel Fernandes, Juri Lelli,
	Valentin Schneider, Dietmar Eggemann

On Thu, Feb 02, 2023 at 10:34:06PM +0000, John Stultz wrote:
> In order ot extend locktorture to support lock nesting, add
> nested_lock() and nested_unlock() hooks to the torture ops.
> 
> These take a 32bit lockset mask which is generated at random,
> so some number of locks will be taken before the main lock is
> taken and released afterwards.
> 
> Additionally, add nlocks module parameter to allow specifying
> the number of nested locks to be used.
> 
> This has been helpful to uncover issues in the proxy-exec
> series development.
> 
> This was inspired by locktorture extensions originally implemented
> by Connor O'Brien, for stress testing the proxy-execution series:
>   https://lore.kernel.org/lkml/20221003214501.2050087-12-connoro@google.com/
> 
> Comments or feedback would be greatly appreciated!

I have pulled this in for testing and further review, thank you!

Should either of these files have a locktorture.nlocks=<whatever>
added?

tools/testing/selftests/rcutorture/configs/lock/LOCK02.boot
tools/testing/selftests/rcutorture/configs/lock/LOCK05.boot

The first is for mutex_lock and the second for rtmutex_lock.

This did pass a quick "torture.sh --do-none --do-locktorture", which is
good, but this uses the existing .boot files.

							Thanx, Paul

> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: Joel Fernandes <joel@joelfernandes.org>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Signed-off-by: John Stultz <jstultz@google.com>
> ---
>  kernel/locking/locktorture.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> index 9c2fb613a55d..f4fbd3194654 100644
> --- a/kernel/locking/locktorture.c
> +++ b/kernel/locking/locktorture.c
> @@ -48,6 +48,9 @@ torture_param(int, stat_interval, 60,
>  torture_param(int, stutter, 5, "Number of jiffies to run/halt test, 0=disable");
>  torture_param(int, verbose, 1,
>  	     "Enable verbose debugging printk()s");
> +torture_param(int, nlocks, 0, "Number of nested locks");
> +/* Going much higher trips "BUG: MAX_LOCKDEP_CHAIN_HLOCKS too low!" errors */
> +#define MAX_LOCKS 8
>  
>  static char *torture_type = "spin_lock";
>  module_param(torture_type, charp, 0444);
> @@ -76,10 +79,12 @@ static void lock_torture_cleanup(void);
>  struct lock_torture_ops {
>  	void (*init)(void);
>  	void (*exit)(void);
> +	int (*nested_lock)(int tid, u32 lockset);
>  	int (*writelock)(int tid);
>  	void (*write_delay)(struct torture_random_state *trsp);
>  	void (*task_boost)(struct torture_random_state *trsp);
>  	void (*writeunlock)(int tid);
> +	void (*nested_unlock)(int tid, u32 lockset);
>  	int (*readlock)(int tid);
>  	void (*read_delay)(struct torture_random_state *trsp);
>  	void (*readunlock)(int tid);
> @@ -669,6 +674,7 @@ static int lock_torture_writer(void *arg)
>  	struct lock_stress_stats *lwsp = arg;
>  	int tid = lwsp - cxt.lwsa;
>  	DEFINE_TORTURE_RANDOM(rand);
> +	u32 lockset_mask;
>  
>  	VERBOSE_TOROUT_STRING("lock_torture_writer task started");
>  	set_user_nice(current, MAX_NICE);
> @@ -677,7 +683,10 @@ static int lock_torture_writer(void *arg)
>  		if ((torture_random(&rand) & 0xfffff) == 0)
>  			schedule_timeout_uninterruptible(1);
>  
> +		lockset_mask = torture_random(&rand);
>  		cxt.cur_ops->task_boost(&rand);
> +		if (cxt.cur_ops->nested_lock)
> +			cxt.cur_ops->nested_lock(tid, lockset_mask);
>  		cxt.cur_ops->writelock(tid);
>  		if (WARN_ON_ONCE(lock_is_write_held))
>  			lwsp->n_lock_fail++;
> @@ -690,6 +699,8 @@ static int lock_torture_writer(void *arg)
>  		lock_is_write_held = false;
>  		WRITE_ONCE(last_lock_release, jiffies);
>  		cxt.cur_ops->writeunlock(tid);
> +		if (cxt.cur_ops->nested_unlock)
> +			cxt.cur_ops->nested_unlock(tid, lockset_mask);
>  
>  		stutter_wait("lock_torture_writer");
>  	} while (!torture_must_stop());
> @@ -830,11 +841,11 @@ lock_torture_print_module_parms(struct lock_torture_ops *cur_ops,
>  				const char *tag)
>  {
>  	pr_alert("%s" TORTURE_FLAG
> -		 "--- %s%s: nwriters_stress=%d nreaders_stress=%d stat_interval=%d verbose=%d shuffle_interval=%d stutter=%d shutdown_secs=%d onoff_interval=%d onoff_holdoff=%d\n",
> +		 "--- %s%s: nwriters_stress=%d nreaders_stress=%d nlocks=%d stat_interval=%d verbose=%d shuffle_interval=%d stutter=%d shutdown_secs=%d onoff_interval=%d onoff_holdoff=%d\n",
>  		 torture_type, tag, cxt.debug_lock ? " [debug]": "",
> -		 cxt.nrealwriters_stress, cxt.nrealreaders_stress, stat_interval,
> -		 verbose, shuffle_interval, stutter, shutdown_secs,
> -		 onoff_interval, onoff_holdoff);
> +		 cxt.nrealwriters_stress, cxt.nrealreaders_stress, nlocks,
> +		 stat_interval, verbose, shuffle_interval, stutter,
> +		 shutdown_secs, onoff_interval, onoff_holdoff);
>  }
>  
>  static void lock_torture_cleanup(void)
> @@ -1053,6 +1064,10 @@ static int __init lock_torture_init(void)
>  		}
>  	}
>  
> +	/* cap nlocks to MAX_LOCKS */
> +	if (nlocks > MAX_LOCKS)
> +		nlocks = MAX_LOCKS;
> +
>  	if (cxt.cur_ops->readlock) {
>  		reader_tasks = kcalloc(cxt.nrealreaders_stress,
>  				       sizeof(reader_tasks[0]),
> -- 
> 2.39.1.519.gcb327c4b5f-goog
> 

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

* Re: [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter
  2023-02-03  2:05 ` [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter Paul E. McKenney
@ 2023-02-03  3:56   ` John Stultz
  2023-02-03  4:29     ` Paul E. McKenney
  0 siblings, 1 reply; 7+ messages in thread
From: John Stultz @ 2023-02-03  3:56 UTC (permalink / raw)
  To: paulmck
  Cc: LKML, Davidlohr Bueso, Josh Triplett, Joel Fernandes, Juri Lelli,
	Valentin Schneider, Dietmar Eggemann

On Thu, Feb 2, 2023 at 6:05 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Thu, Feb 02, 2023 at 10:34:06PM +0000, John Stultz wrote:
> > In order ot extend locktorture to support lock nesting, add
> > nested_lock() and nested_unlock() hooks to the torture ops.
> >
> > These take a 32bit lockset mask which is generated at random,
> > so some number of locks will be taken before the main lock is
> > taken and released afterwards.
> >
> > Additionally, add nlocks module parameter to allow specifying
> > the number of nested locks to be used.
> >
> > This has been helpful to uncover issues in the proxy-exec
> > series development.
> >
> > This was inspired by locktorture extensions originally implemented
> > by Connor O'Brien, for stress testing the proxy-execution series:
> >   https://lore.kernel.org/lkml/20221003214501.2050087-12-connoro@google.com/
> >
> > Comments or feedback would be greatly appreciated!
>
> I have pulled this in for testing and further review, thank you!
>
> Should either of these files have a locktorture.nlocks=<whatever>
> added?
>
> tools/testing/selftests/rcutorture/configs/lock/LOCK02.boot
> tools/testing/selftests/rcutorture/configs/lock/LOCK05.boot
>
> The first is for mutex_lock and the second for rtmutex_lock.

Potentially? I wasn't aware of these files. Is there some
documentation on the intent behind them?

While the nested locking is useful to cause different lock chains to
test boosting or proxy-execution, I worry they may cause extra noise
that could distract from just thrashing the *mutex lock primitive if
that's the existing intent.

So we might want additional config files for the nested case.

> This did pass a quick "torture.sh --do-none --do-locktorture", which is
> good, but this uses the existing .boot files.

Yeah, probably no effective change in that case.

thanks
-john

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

* Re: [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter
  2023-02-03  3:56   ` John Stultz
@ 2023-02-03  4:29     ` Paul E. McKenney
  0 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2023-02-03  4:29 UTC (permalink / raw)
  To: John Stultz
  Cc: LKML, Davidlohr Bueso, Josh Triplett, Joel Fernandes, Juri Lelli,
	Valentin Schneider, Dietmar Eggemann

On Thu, Feb 02, 2023 at 07:56:05PM -0800, John Stultz wrote:
> On Thu, Feb 2, 2023 at 6:05 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Thu, Feb 02, 2023 at 10:34:06PM +0000, John Stultz wrote:
> > > In order ot extend locktorture to support lock nesting, add
> > > nested_lock() and nested_unlock() hooks to the torture ops.
> > >
> > > These take a 32bit lockset mask which is generated at random,
> > > so some number of locks will be taken before the main lock is
> > > taken and released afterwards.
> > >
> > > Additionally, add nlocks module parameter to allow specifying
> > > the number of nested locks to be used.
> > >
> > > This has been helpful to uncover issues in the proxy-exec
> > > series development.
> > >
> > > This was inspired by locktorture extensions originally implemented
> > > by Connor O'Brien, for stress testing the proxy-execution series:
> > >   https://lore.kernel.org/lkml/20221003214501.2050087-12-connoro@google.com/
> > >
> > > Comments or feedback would be greatly appreciated!
> >
> > I have pulled this in for testing and further review, thank you!
> >
> > Should either of these files have a locktorture.nlocks=<whatever>
> > added?
> >
> > tools/testing/selftests/rcutorture/configs/lock/LOCK02.boot
> > tools/testing/selftests/rcutorture/configs/lock/LOCK05.boot
> >
> > The first is for mutex_lock and the second for rtmutex_lock.
> 
> Potentially? I wasn't aware of these files. Is there some
> documentation on the intent behind them?

There is a LOCK02 file that contains Kconfig options and the LOCK02.boot
file contains kernel boot parameters.  There is a CFLIST file that
contains the list of such files that the command below will test by
default.

The best documentation is probably here:

https://paulmck.livejournal.com/57769.html
https://paulmck.livejournal.com/58077.html

> While the nested locking is useful to cause different lock chains to
> test boosting or proxy-execution, I worry they may cause extra noise
> that could distract from just thrashing the *mutex lock primitive if
> that's the existing intent.

The intent is to find bugs by whatever means necessary, within reason.

> So we might want additional config files for the nested case.

That would work.

> > This did pass a quick "torture.sh --do-none --do-locktorture", which is
> > good, but this uses the existing .boot files.
> 
> Yeah, probably no effective change in that case.

At least nothing else got broken.  ;-)

							Thanx, Paul

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

end of thread, other threads:[~2023-02-03  4:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-02 22:34 [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter John Stultz
2023-02-02 22:34 ` [RFC][PATCH 2/4] locktorture: Add nested locking to mutex torture tests John Stultz
2023-02-02 22:34 ` [RFC][PATCH 3/4] locktorture: Add nested locking to rtmutex " John Stultz
2023-02-02 22:34 ` [RFC][PATCH 4/4] locktorture: With nested locks, occasionally skip main lock John Stultz
2023-02-03  2:05 ` [RFC][PATCH 1/4] locktorture: Add nested_[un]lock() hooks and nlocks parameter Paul E. McKenney
2023-02-03  3:56   ` John Stultz
2023-02-03  4:29     ` Paul E. McKenney

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