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/[email protected]/
Comments or feedback would be greatly appreciated!
Cc: Davidlohr Bueso <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Josh Triplett <[email protected]>
Cc: Joel Fernandes <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Dietmar Eggemann <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
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
This patch adds randomized nested locking to the mutex torture
tests, as well as new LOCK08 config files for testing mutexes
with nested locking
This was inspired by locktorture extensions originally implemented
by Connor O'Brien, for stress testing the proxy-execution series:
https://lore.kernel.org/lkml/[email protected]/
Comments or feedback would be greatly appreciated!
Cc: Davidlohr Bueso <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Josh Triplett <[email protected]>
Cc: Joel Fernandes <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Dietmar Eggemann <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
v2:
* Add LOCK08 config file suggested by Paul McKenney
---
kernel/locking/locktorture.c | 35 +++++++++++++++++++
.../selftests/rcutorture/configs/lock/CFLIST | 1 +
.../selftests/rcutorture/configs/lock/LOCK08 | 6 ++++
.../rcutorture/configs/lock/LOCK08.boot | 1 +
4 files changed, 43 insertions(+)
create mode 100644 tools/testing/selftests/rcutorture/configs/lock/LOCK08
create mode 100644 tools/testing/selftests/rcutorture/configs/lock/LOCK08.boot
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,
diff --git a/tools/testing/selftests/rcutorture/configs/lock/CFLIST b/tools/testing/selftests/rcutorture/configs/lock/CFLIST
index 41bae5824339..a48bba0d35a6 100644
--- a/tools/testing/selftests/rcutorture/configs/lock/CFLIST
+++ b/tools/testing/selftests/rcutorture/configs/lock/CFLIST
@@ -5,3 +5,4 @@ LOCK04
LOCK05
LOCK06
LOCK07
+LOCK08
diff --git a/tools/testing/selftests/rcutorture/configs/lock/LOCK08 b/tools/testing/selftests/rcutorture/configs/lock/LOCK08
new file mode 100644
index 000000000000..1d1da1477fc3
--- /dev/null
+++ b/tools/testing/selftests/rcutorture/configs/lock/LOCK08
@@ -0,0 +1,6 @@
+CONFIG_SMP=y
+CONFIG_NR_CPUS=4
+CONFIG_HOTPLUG_CPU=y
+CONFIG_PREEMPT_NONE=n
+CONFIG_PREEMPT_VOLUNTARY=n
+CONFIG_PREEMPT=y
diff --git a/tools/testing/selftests/rcutorture/configs/lock/LOCK08.boot b/tools/testing/selftests/rcutorture/configs/lock/LOCK08.boot
new file mode 100644
index 000000000000..e59a369b470e
--- /dev/null
+++ b/tools/testing/selftests/rcutorture/configs/lock/LOCK08.boot
@@ -0,0 +1 @@
+locktorture.torture_type=mutex_lock locktorture.nlocks=8
--
2.39.1.519.gcb327c4b5f-goog
This patch adds randomized nested locking to the rtmutex torture
tests. Additionally it adds LOCK09 config files for testing
rtmutexes with nested locking.
This was inspired by locktorture extensions originally implemented
by Connor O'Brien, for stress testing the proxy-execution series:
https://lore.kernel.org/lkml/[email protected]/
Comments or feedback would be greatly appreciated!
Cc: Davidlohr Bueso <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Josh Triplett <[email protected]>
Cc: Joel Fernandes <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Dietmar Eggemann <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
v2:
* Add LOCK08 config file suggested by Paul McKenney
---
kernel/locking/locktorture.c | 35 +++++++++++++++++++
.../selftests/rcutorture/configs/lock/CFLIST | 1 +
.../selftests/rcutorture/configs/lock/LOCK09 | 6 ++++
.../rcutorture/configs/lock/LOCK09.boot | 1 +
4 files changed, 43 insertions(+)
create mode 100644 tools/testing/selftests/rcutorture/configs/lock/LOCK09
create mode 100644 tools/testing/selftests/rcutorture/configs/lock/LOCK09.boot
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,
diff --git a/tools/testing/selftests/rcutorture/configs/lock/CFLIST b/tools/testing/selftests/rcutorture/configs/lock/CFLIST
index a48bba0d35a6..28e23d05d5a5 100644
--- a/tools/testing/selftests/rcutorture/configs/lock/CFLIST
+++ b/tools/testing/selftests/rcutorture/configs/lock/CFLIST
@@ -6,3 +6,4 @@ LOCK05
LOCK06
LOCK07
LOCK08
+LOCK09
diff --git a/tools/testing/selftests/rcutorture/configs/lock/LOCK09 b/tools/testing/selftests/rcutorture/configs/lock/LOCK09
new file mode 100644
index 000000000000..1d1da1477fc3
--- /dev/null
+++ b/tools/testing/selftests/rcutorture/configs/lock/LOCK09
@@ -0,0 +1,6 @@
+CONFIG_SMP=y
+CONFIG_NR_CPUS=4
+CONFIG_HOTPLUG_CPU=y
+CONFIG_PREEMPT_NONE=n
+CONFIG_PREEMPT_VOLUNTARY=n
+CONFIG_PREEMPT=y
diff --git a/tools/testing/selftests/rcutorture/configs/lock/LOCK09.boot b/tools/testing/selftests/rcutorture/configs/lock/LOCK09.boot
new file mode 100644
index 000000000000..6c413e3e1bdf
--- /dev/null
+++ b/tools/testing/selftests/rcutorture/configs/lock/LOCK09.boot
@@ -0,0 +1 @@
+locktorture.torture_type=rtmutex_lock locktorture.nlocks=8
--
2.39.1.519.gcb327c4b5f-goog
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 <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Josh Triplett <[email protected]>
Cc: Joel Fernandes <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Dietmar Eggemann <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
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
On Fri, Feb 03, 2023 at 08:01:35PM +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/[email protected]/
>
> Comments or feedback would be greatly appreciated!
I have applied this series in place of the previous one, and thank you
for the added scripting updates!
Thanx, Paul
> Cc: Davidlohr Bueso <[email protected]>
> Cc: "Paul E. McKenney" <[email protected]>
> Cc: Josh Triplett <[email protected]>
> Cc: Joel Fernandes <[email protected]>
> Cc: Juri Lelli <[email protected]>
> Cc: Valentin Schneider <[email protected]>
> Cc: Dietmar Eggemann <[email protected]>
> Signed-off-by: John Stultz <[email protected]>
> ---
> 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
>
On Fri, 03 Feb 2023, John Stultz wrote:
>In order ot extend locktorture to support lock nesting, add
^ to
>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/[email protected]/
>
>Comments or feedback would be greatly appreciated!
Both of the above statements should be in a cover letter patch.
>
>Cc: Davidlohr Bueso <[email protected]>
>Cc: "Paul E. McKenney" <[email protected]>
>Cc: Josh Triplett <[email protected]>
>Cc: Joel Fernandes <[email protected]>
>Cc: Juri Lelli <[email protected]>
>Cc: Valentin Schneider <[email protected]>
>Cc: Dietmar Eggemann <[email protected]>
>Signed-off-by: John Stultz <[email protected]>
Reviewed-by: Davidlohr Bueso <[email protected]>
With some small nits below.
>---
> 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");
Maybe rename the parameter to 'nested_locks'? It would also be good to
inform the 8 limit. "Number of nested locks (max = 8)".
On Fri, 03 Feb 2023, John Stultz wrote:
>This patch adds randomized nested locking to the rtmutex torture
>tests. Additionally it adds LOCK09 config files for testing
>rtmutexes with nested locking.
>
>This was inspired by locktorture extensions originally implemented
>by Connor O'Brien, for stress testing the proxy-execution series:
> https://lore.kernel.org/lkml/[email protected]/
>
>Comments or feedback would be greatly appreciated!
Same comments for all the patches in the series.
>
>Cc: Davidlohr Bueso <[email protected]>
>Cc: "Paul E. McKenney" <[email protected]>
>Cc: Josh Triplett <[email protected]>
>Cc: Joel Fernandes <[email protected]>
>Cc: Juri Lelli <[email protected]>
>Cc: Valentin Schneider <[email protected]>
>Cc: Dietmar Eggemann <[email protected]>
>Signed-off-by: John Stultz <[email protected]>
Reviewed-by: Davidlohr Bueso <[email protected]>
On Fri, 03 Feb 2023, John Stultz wrote:
>This patch adds randomized nested locking to the rtmutex torture
>tests. Additionally it adds LOCK09 config files for testing
>rtmutexes with nested locking.
>
>This was inspired by locktorture extensions originally implemented
>by Connor O'Brien, for stress testing the proxy-execution series:
> https://lore.kernel.org/lkml/[email protected]/
>
>Comments or feedback would be greatly appreciated!
>
>Cc: Davidlohr Bueso <[email protected]>
>Cc: "Paul E. McKenney" <[email protected]>
>Cc: Josh Triplett <[email protected]>
>Cc: Joel Fernandes <[email protected]>
>Cc: Juri Lelli <[email protected]>
>Cc: Valentin Schneider <[email protected]>
>Cc: Dietmar Eggemann <[email protected]>
>Signed-off-by: John Stultz <[email protected]>
Reviewed-by: Davidlohr Bueso <[email protected]>
On Fri, 03 Feb 2023, John Stultz wrote:
>@@ -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);
For the sake of future readers of locktorture.c, this deserves a comment
as to why we wanna skip the main lock (copy/paste from the changelog).
Thanks,
Davidlohr
On Fri, 03 Feb 2023, John Stultz wrote:
>This patch adds randomized nested locking to the mutex torture
>tests, as well as new LOCK08 config files for testing mutexes
>with nested locking
>
>This was inspired by locktorture extensions originally implemented
>by Connor O'Brien, for stress testing the proxy-execution series:
> https://lore.kernel.org/lkml/[email protected]/
>
>Comments or feedback would be greatly appreciated!
>
>Cc: Davidlohr Bueso <[email protected]>
>Cc: "Paul E. McKenney" <[email protected]>
>Cc: Josh Triplett <[email protected]>
>Cc: Joel Fernandes <[email protected]>
>Cc: Juri Lelli <[email protected]>
>Cc: Valentin Schneider <[email protected]>
>Cc: Dietmar Eggemann <[email protected]>
>Signed-off-by: John Stultz <[email protected]>
Just realized I sent two review tags for rtmutex, one was supposed
to be this patch.
Reviewed-by: Davidlohr Bueso <[email protected]>
On Sun, Feb 05, 2023 at 11:07:53AM -0800, Davidlohr Bueso wrote:
> On Fri, 03 Feb 2023, John Stultz wrote:
>
> > This patch adds randomized nested locking to the mutex torture
> > tests, as well as new LOCK08 config files for testing mutexes
> > with nested locking
> >
> > This was inspired by locktorture extensions originally implemented
> > by Connor O'Brien, for stress testing the proxy-execution series:
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > Comments or feedback would be greatly appreciated!
> >
> > Cc: Davidlohr Bueso <[email protected]>
> > Cc: "Paul E. McKenney" <[email protected]>
> > Cc: Josh Triplett <[email protected]>
> > Cc: Joel Fernandes <[email protected]>
> > Cc: Juri Lelli <[email protected]>
> > Cc: Valentin Schneider <[email protected]>
> > Cc: Dietmar Eggemann <[email protected]>
> > Signed-off-by: John Stultz <[email protected]>
>
> Just realized I sent two review tags for rtmutex, one was supposed
> to be this patch.
>
> Reviewed-by: Davidlohr Bueso <[email protected]>
Thank you!
I will apply this to these commits on my next rebase:
f5a632cbbbea ("locktorture: Add nested locking to mutex torture tests")
83743c9839f5 ("locktorture: Add nested locking to rtmutex torture tests")
John, if you send a new version of these, please add Davidlohr's review
tags.
Thanx, Paul
On Mon, Feb 6, 2023 at 9:20 AM Paul E. McKenney <[email protected]> wrote:
>
> On Sun, Feb 05, 2023 at 11:07:53AM -0800, Davidlohr Bueso wrote:
> > On Fri, 03 Feb 2023, John Stultz wrote:
> >
> > > This patch adds randomized nested locking to the mutex torture
> > > tests, as well as new LOCK08 config files for testing mutexes
> > > with nested locking
> > >
> > > This was inspired by locktorture extensions originally implemented
> > > by Connor O'Brien, for stress testing the proxy-execution series:
> > > https://lore.kernel.org/lkml/[email protected]/
> > >
> > > Comments or feedback would be greatly appreciated!
> > >
> > > Cc: Davidlohr Bueso <[email protected]>
> > > Cc: "Paul E. McKenney" <[email protected]>
> > > Cc: Josh Triplett <[email protected]>
> > > Cc: Joel Fernandes <[email protected]>
> > > Cc: Juri Lelli <[email protected]>
> > > Cc: Valentin Schneider <[email protected]>
> > > Cc: Dietmar Eggemann <[email protected]>
> > > Signed-off-by: John Stultz <[email protected]>
> >
> > Just realized I sent two review tags for rtmutex, one was supposed
> > to be this patch.
> >
> > Reviewed-by: Davidlohr Bueso <[email protected]>
>
> Thank you!
>
> I will apply this to these commits on my next rebase:
>
> f5a632cbbbea ("locktorture: Add nested locking to mutex torture tests")
> 83743c9839f5 ("locktorture: Add nested locking to rtmutex torture tests")
>
> John, if you send a new version of these, please add Davidlohr's review
> tags.
Yeah. I'll respin to address most of the feedback.
thanks
-john
On Sun, Feb 5, 2023 at 11:01 AM Davidlohr Bueso <[email protected]> wrote:
> On Fri, 03 Feb 2023, John Stultz wrote:
>
> >In order ot extend locktorture to support lock nesting, add
> ^ to
Fixed. Thanks for catching this!
> >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/[email protected]/
> >
> >Comments or feedback would be greatly appreciated!
>
> Both of the above statements should be in a cover letter patch.
I'll drop the second line, but I'd like to preserve Connor's
contribution in the git log.
If it's ok, I'll keep it to just one of the patches to not be repetitive.
> >Cc: Davidlohr Bueso <[email protected]>
> >Cc: "Paul E. McKenney" <[email protected]>
> >Cc: Josh Triplett <[email protected]>
> >Cc: Joel Fernandes <[email protected]>
> >Cc: Juri Lelli <[email protected]>
> >Cc: Valentin Schneider <[email protected]>
> >Cc: Dietmar Eggemann <[email protected]>
> >Signed-off-by: John Stultz <[email protected]>
>
> Reviewed-by: Davidlohr Bueso <[email protected]>
>
> With some small nits below.
>
> >---
> > 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");
>
> Maybe rename the parameter to 'nested_locks'? It would also be good to
> inform the 8 limit. "Number of nested locks (max = 8)".
Done!
Thanks so much for your time reviewing and the feedback!
I'll resend the reworked patches shortly.
-john
On Sun, Feb 5, 2023 at 11:33 AM Davidlohr Bueso <[email protected]> wrote:
> On Fri, 03 Feb 2023, John Stultz wrote:
>
> >@@ -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);
>
> For the sake of future readers of locktorture.c, this deserves a comment
> as to why we wanna skip the main lock (copy/paste from the changelog).
Good point! I'll send this update out soon.
Thanks again for the review and feedback!
-john