[PATCH rcu 0/6] Torture-test updates for v6.5

[PATCH rcu 0/6] Torture-test updates for v6.5

[Paul E. McKenney]

Hello!

This series contains torture-test and torture-test-scripting updates
for v6.5:

1.	Add long_hold to adjust lock-hold delays.

2.	Correct name of use_softirq module parameter.

3.	rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup(),
	courtesy of Qiuxu Zhuo.

4.	rcu/rcuscale: Stop kfree_scale_thread thread(s) after unloading
	rcuscale, courtesy of Qiuxu Zhuo.

5.	doc/rcutorture: Add description of rcutorture.stall_cpu_block,
	courtesy of Zqiang.

6.	Remove duplicated argument -enable-kvm for ppc64, courtesy of
	Zhouyi Zhou.

						Thanx, Paul

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

 b/Documentation/admin-guide/kernel-parameters.txt                  |   12 
 b/kernel/locking/locktorture.c                                     |   51 +-
 b/kernel/rcu/rcuscale.c                                            |  194 +++++-----
 b/tools/testing/selftests/rcutorture/bin/functions.sh              |    2 
 b/tools/testing/selftests/rcutorture/configs/rcu/BUSTED-BOOST.boot |    2 
 b/tools/testing/selftests/rcutorture/configs/rcu/TREE03.boot       |    2 
 kernel/rcu/rcuscale.c                                              |    5 
 7 files changed, 137 insertions(+), 131 deletions(-)

[PATCH rcu 1/6] locktorture: Add long_hold to adjust lock-hold delays

[Paul E. McKenney]

This commit adds a long_hold module parameter to allow testing diagnostics
for excessive lock-hold times.  Also adjust torture_param() invocations
for longer line length while in the area.

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/locking/locktorture.c | 51 ++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 29 deletions(-)

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 153ddc4c47ef..949d3deae506 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -33,24 +33,19 @@
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Paul E. McKenney <paulmck@linux.ibm.com>");
 
-torture_param(int, nwriters_stress, -1,
-	     "Number of write-locking stress-test threads");
-torture_param(int, nreaders_stress, -1,
-	     "Number of read-locking stress-test threads");
+torture_param(int, nwriters_stress, -1, "Number of write-locking stress-test threads");
+torture_param(int, nreaders_stress, -1, "Number of read-locking stress-test threads");
+torture_param(int, long_hold, 100, "Do occasional long hold of lock (ms), 0=disable");
 torture_param(int, onoff_holdoff, 0, "Time after boot before CPU hotplugs (s)");
-torture_param(int, onoff_interval, 0,
-	     "Time between CPU hotplugs (s), 0=disable");
-torture_param(int, shuffle_interval, 3,
-	     "Number of jiffies between shuffles, 0=disable");
+torture_param(int, onoff_interval, 0, "Time between CPU hotplugs (s), 0=disable");
+torture_param(int, shuffle_interval, 3, "Number of jiffies between shuffles, 0=disable");
 torture_param(int, shutdown_secs, 0, "Shutdown time (j), <= zero to disable.");
-torture_param(int, stat_interval, 60,
-	     "Number of seconds between stats printk()s");
+torture_param(int, stat_interval, 60, "Number of seconds between stats printk()s");
 torture_param(int, stutter, 5, "Number of jiffies to run/halt test, 0=disable");
 torture_param(int, rt_boost, 2,
-		"Do periodic rt-boost. 0=Disable, 1=Only for rt_mutex, 2=For all lock types.");
+		   "Do periodic rt-boost. 0=Disable, 1=Only for rt_mutex, 2=For all lock types.");
 torture_param(int, rt_boost_factor, 50, "A factor determining how often rt-boost happens.");
-torture_param(int, verbose, 1,
-	     "Enable verbose debugging printk()s");
+torture_param(int, verbose, 1, "Enable verbose debugging printk()s");
 torture_param(int, nested_locks, 0, "Number of nested locks (max = 8)");
 /* Going much higher trips "BUG: MAX_LOCKDEP_CHAIN_HLOCKS too low!" errors */
 #define MAX_NESTED_LOCKS 8
@@ -120,7 +115,7 @@ static int torture_lock_busted_write_lock(int tid __maybe_unused)
 
 static void torture_lock_busted_write_delay(struct torture_random_state *trsp)
 {
-	const unsigned long longdelay_ms = 100;
+	const unsigned long longdelay_ms = long_hold ? long_hold : ULONG_MAX;
 
 	/* We want a long delay occasionally to force massive contention.  */
 	if (!(torture_random(trsp) %
@@ -198,16 +193,18 @@ __acquires(torture_spinlock)
 static void torture_spin_lock_write_delay(struct torture_random_state *trsp)
 {
 	const unsigned long shortdelay_us = 2;
-	const unsigned long longdelay_ms = 100;
+	const unsigned long longdelay_ms = long_hold ? long_hold : ULONG_MAX;
+	unsigned long j;
 
 	/* We want a short delay mostly to emulate likely code, and
 	 * we want a long delay occasionally to force massive contention.
 	 */
-	if (!(torture_random(trsp) %
-	      (cxt.nrealwriters_stress * 2000 * longdelay_ms)))
+	if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 2000 * longdelay_ms))) {
+		j = jiffies;
 		mdelay(longdelay_ms);
-	if (!(torture_random(trsp) %
-	      (cxt.nrealwriters_stress * 2 * shortdelay_us)))
+		pr_alert("%s: delay = %lu jiffies.\n", __func__, jiffies - j);
+	}
+	if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 200 * shortdelay_us)))
 		udelay(shortdelay_us);
 	if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 20000)))
 		torture_preempt_schedule();  /* Allow test to be preempted. */
@@ -322,7 +319,7 @@ __acquires(torture_rwlock)
 static void torture_rwlock_write_delay(struct torture_random_state *trsp)
 {
 	const unsigned long shortdelay_us = 2;
-	const unsigned long longdelay_ms = 100;
+	const unsigned long longdelay_ms = long_hold ? long_hold : ULONG_MAX;
 
 	/* We want a short delay mostly to emulate likely code, and
 	 * we want a long delay occasionally to force massive contention.
@@ -455,14 +452,12 @@ __acquires(torture_mutex)
 
 static void torture_mutex_delay(struct torture_random_state *trsp)
 {
-	const unsigned long longdelay_ms = 100;
+	const unsigned long longdelay_ms = long_hold ? long_hold : ULONG_MAX;
 
 	/* We want a long delay occasionally to force massive contention.  */
 	if (!(torture_random(trsp) %
 	      (cxt.nrealwriters_stress * 2000 * longdelay_ms)))
 		mdelay(longdelay_ms * 5);
-	else
-		mdelay(longdelay_ms / 5);
 	if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 20000)))
 		torture_preempt_schedule();  /* Allow test to be preempted. */
 }
@@ -630,7 +625,7 @@ __acquires(torture_rtmutex)
 static void torture_rtmutex_delay(struct torture_random_state *trsp)
 {
 	const unsigned long shortdelay_us = 2;
-	const unsigned long longdelay_ms = 100;
+	const unsigned long longdelay_ms = long_hold ? long_hold : ULONG_MAX;
 
 	/*
 	 * We want a short delay mostly to emulate likely code, and
@@ -640,7 +635,7 @@ static void torture_rtmutex_delay(struct torture_random_state *trsp)
 	      (cxt.nrealwriters_stress * 2000 * longdelay_ms)))
 		mdelay(longdelay_ms);
 	if (!(torture_random(trsp) %
-	      (cxt.nrealwriters_stress * 2 * shortdelay_us)))
+	      (cxt.nrealwriters_stress * 200 * shortdelay_us)))
 		udelay(shortdelay_us);
 	if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 20000)))
 		torture_preempt_schedule();  /* Allow test to be preempted. */
@@ -695,14 +690,12 @@ __acquires(torture_rwsem)
 
 static void torture_rwsem_write_delay(struct torture_random_state *trsp)
 {
-	const unsigned long longdelay_ms = 100;
+	const unsigned long longdelay_ms = long_hold ? long_hold : ULONG_MAX;
 
 	/* We want a long delay occasionally to force massive contention.  */
 	if (!(torture_random(trsp) %
 	      (cxt.nrealwriters_stress * 2000 * longdelay_ms)))
 		mdelay(longdelay_ms * 10);
-	else
-		mdelay(longdelay_ms / 10);
 	if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 20000)))
 		torture_preempt_schedule();  /* Allow test to be preempted. */
 }
@@ -848,8 +841,8 @@ static int lock_torture_writer(void *arg)
 
 			lwsp->n_lock_acquired++;
 		}
-		cxt.cur_ops->write_delay(&rand);
 		if (!skip_main_lock) {
+			cxt.cur_ops->write_delay(&rand);
 			lock_is_write_held = false;
 			WRITE_ONCE(last_lock_release, jiffies);
 			cxt.cur_ops->writeunlock(tid);
-- 
2.40.1

[PATCH rcu 2/6] rcutorture: Correct name of use_softirq module parameter

[Paul E. McKenney]

The BUSTED-BOOST and TREE03 scenarios specify a mythical tree.use_softirq
module parameter, which means a failure to get full test coverage.  This
commit therefore corrects the name to rcutree.use_softirq.

Fixes: e2b949d54392 ("rcutorture: Make TREE03 use real-time tree.use_softirq setting")
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 .../testing/selftests/rcutorture/configs/rcu/BUSTED-BOOST.boot  | 2 +-
 tools/testing/selftests/rcutorture/configs/rcu/TREE03.boot      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/rcutorture/configs/rcu/BUSTED-BOOST.boot b/tools/testing/selftests/rcutorture/configs/rcu/BUSTED-BOOST.boot
index f57720c52c0f..84f6bb98ce99 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/BUSTED-BOOST.boot
+++ b/tools/testing/selftests/rcutorture/configs/rcu/BUSTED-BOOST.boot
@@ -5,4 +5,4 @@ rcutree.gp_init_delay=3
 rcutree.gp_cleanup_delay=3
 rcutree.kthread_prio=2
 threadirqs
-tree.use_softirq=0
+rcutree.use_softirq=0
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TREE03.boot b/tools/testing/selftests/rcutorture/configs/rcu/TREE03.boot
index 64f864f1f361..8e50bfd4b710 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/TREE03.boot
+++ b/tools/testing/selftests/rcutorture/configs/rcu/TREE03.boot
@@ -4,4 +4,4 @@ rcutree.gp_init_delay=3
 rcutree.gp_cleanup_delay=3
 rcutree.kthread_prio=2
 threadirqs
-tree.use_softirq=0
+rcutree.use_softirq=0
-- 
2.40.1

[PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup()

[Paul E. McKenney]

From: Qiuxu Zhuo <qiuxu.zhuo@intel.com>

This code-movement-only commit moves the rcu_scale_cleanup() and
rcu_scale_shutdown() functions to follow kfree_scale_cleanup().
This is code movement is in preparation for a bug-fix patch that invokes
kfree_scale_cleanup() from rcu_scale_cleanup().

Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/rcu/rcuscale.c | 194 +++++++++++++++++++++---------------------
 1 file changed, 97 insertions(+), 97 deletions(-)

diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
index e82ec9f9a5d8..7e8965b0827a 100644
--- a/kernel/rcu/rcuscale.c
+++ b/kernel/rcu/rcuscale.c
@@ -522,89 +522,6 @@ rcu_scale_print_module_parms(struct rcu_scale_ops *cur_ops, const char *tag)
 		 scale_type, tag, nrealreaders, nrealwriters, verbose, shutdown);
 }
 
-static void
-rcu_scale_cleanup(void)
-{
-	int i;
-	int j;
-	int ngps = 0;
-	u64 *wdp;
-	u64 *wdpp;
-
-	/*
-	 * Would like warning at start, but everything is expedited
-	 * during the mid-boot phase, so have to wait till the end.
-	 */
-	if (rcu_gp_is_expedited() && !rcu_gp_is_normal() && !gp_exp)
-		SCALEOUT_ERRSTRING("All grace periods expedited, no normal ones to measure!");
-	if (rcu_gp_is_normal() && gp_exp)
-		SCALEOUT_ERRSTRING("All grace periods normal, no expedited ones to measure!");
-	if (gp_exp && gp_async)
-		SCALEOUT_ERRSTRING("No expedited async GPs, so went with async!");
-
-	if (torture_cleanup_begin())
-		return;
-	if (!cur_ops) {
-		torture_cleanup_end();
-		return;
-	}
-
-	if (reader_tasks) {
-		for (i = 0; i < nrealreaders; i++)
-			torture_stop_kthread(rcu_scale_reader,
-					     reader_tasks[i]);
-		kfree(reader_tasks);
-	}
-
-	if (writer_tasks) {
-		for (i = 0; i < nrealwriters; i++) {
-			torture_stop_kthread(rcu_scale_writer,
-					     writer_tasks[i]);
-			if (!writer_n_durations)
-				continue;
-			j = writer_n_durations[i];
-			pr_alert("%s%s writer %d gps: %d\n",
-				 scale_type, SCALE_FLAG, i, j);
-			ngps += j;
-		}
-		pr_alert("%s%s start: %llu end: %llu duration: %llu gps: %d batches: %ld\n",
-			 scale_type, SCALE_FLAG,
-			 t_rcu_scale_writer_started, t_rcu_scale_writer_finished,
-			 t_rcu_scale_writer_finished -
-			 t_rcu_scale_writer_started,
-			 ngps,
-			 rcuscale_seq_diff(b_rcu_gp_test_finished,
-					   b_rcu_gp_test_started));
-		for (i = 0; i < nrealwriters; i++) {
-			if (!writer_durations)
-				break;
-			if (!writer_n_durations)
-				continue;
-			wdpp = writer_durations[i];
-			if (!wdpp)
-				continue;
-			for (j = 0; j < writer_n_durations[i]; j++) {
-				wdp = &wdpp[j];
-				pr_alert("%s%s %4d writer-duration: %5d %llu\n",
-					scale_type, SCALE_FLAG,
-					i, j, *wdp);
-				if (j % 100 == 0)
-					schedule_timeout_uninterruptible(1);
-			}
-			kfree(writer_durations[i]);
-		}
-		kfree(writer_tasks);
-		kfree(writer_durations);
-		kfree(writer_n_durations);
-	}
-
-	/* Do torture-type-specific cleanup operations.  */
-	if (cur_ops->cleanup != NULL)
-		cur_ops->cleanup();
-
-	torture_cleanup_end();
-}
-
 /*
  * Return the number if non-negative.  If -1, the number of CPUs.
  * If less than -1, that much less than the number of CPUs, but
@@ -624,20 +541,6 @@ static int compute_real(int n)
 	return nr;
 }
 
-/*
- * RCU scalability shutdown kthread.  Just waits to be awakened, then shuts
- * down system.
- */
-static int
-rcu_scale_shutdown(void *arg)
-{
-	wait_event_idle(shutdown_wq, atomic_read(&n_rcu_scale_writer_finished) >= nrealwriters);
-	smp_mb(); /* Wake before output. */
-	rcu_scale_cleanup();
-	kernel_power_off();
-	return -EINVAL;
-}
-
 /*
  * kfree_rcu() scalability tests: Start a kfree_rcu() loop on all CPUs for number
  * of iterations and measure total time and number of GP for all iterations to complete.
@@ -874,6 +777,103 @@ kfree_scale_init(void)
 	return firsterr;
 }
 
+static void
+rcu_scale_cleanup(void)
+{
+	int i;
+	int j;
+	int ngps = 0;
+	u64 *wdp;
+	u64 *wdpp;
+
+	/*
+	 * Would like warning at start, but everything is expedited
+	 * during the mid-boot phase, so have to wait till the end.
+	 */
+	if (rcu_gp_is_expedited() && !rcu_gp_is_normal() && !gp_exp)
+		SCALEOUT_ERRSTRING("All grace periods expedited, no normal ones to measure!");
+	if (rcu_gp_is_normal() && gp_exp)
+		SCALEOUT_ERRSTRING("All grace periods normal, no expedited ones to measure!");
+	if (gp_exp && gp_async)
+		SCALEOUT_ERRSTRING("No expedited async GPs, so went with async!");
+
+	if (torture_cleanup_begin())
+		return;
+	if (!cur_ops) {
+		torture_cleanup_end();
+		return;
+	}
+
+	if (reader_tasks) {
+		for (i = 0; i < nrealreaders; i++)
+			torture_stop_kthread(rcu_scale_reader,
+					     reader_tasks[i]);
+		kfree(reader_tasks);
+	}
+
+	if (writer_tasks) {
+		for (i = 0; i < nrealwriters; i++) {
+			torture_stop_kthread(rcu_scale_writer,
+					     writer_tasks[i]);
+			if (!writer_n_durations)
+				continue;
+			j = writer_n_durations[i];
+			pr_alert("%s%s writer %d gps: %d\n",
+				 scale_type, SCALE_FLAG, i, j);
+			ngps += j;
+		}
+		pr_alert("%s%s start: %llu end: %llu duration: %llu gps: %d batches: %ld\n",
+			 scale_type, SCALE_FLAG,
+			 t_rcu_scale_writer_started, t_rcu_scale_writer_finished,
+			 t_rcu_scale_writer_finished -
+			 t_rcu_scale_writer_started,
+			 ngps,
+			 rcuscale_seq_diff(b_rcu_gp_test_finished,
+					   b_rcu_gp_test_started));
+		for (i = 0; i < nrealwriters; i++) {
+			if (!writer_durations)
+				break;
+			if (!writer_n_durations)
+				continue;
+			wdpp = writer_durations[i];
+			if (!wdpp)
+				continue;
+			for (j = 0; j < writer_n_durations[i]; j++) {
+				wdp = &wdpp[j];
+				pr_alert("%s%s %4d writer-duration: %5d %llu\n",
+					scale_type, SCALE_FLAG,
+					i, j, *wdp);
+				if (j % 100 == 0)
+					schedule_timeout_uninterruptible(1);
+			}
+			kfree(writer_durations[i]);
+		}
+		kfree(writer_tasks);
+		kfree(writer_durations);
+		kfree(writer_n_durations);
+	}
+
+	/* Do torture-type-specific cleanup operations.  */
+	if (cur_ops->cleanup != NULL)
+		cur_ops->cleanup();
+
+	torture_cleanup_end();
+}
+
+/*
+ * RCU scalability shutdown kthread.  Just waits to be awakened, then shuts
+ * down system.
+ */
+static int
+rcu_scale_shutdown(void *arg)
+{
+	wait_event_idle(shutdown_wq, atomic_read(&n_rcu_scale_writer_finished) >= nrealwriters);
+	smp_mb(); /* Wake before output. */
+	rcu_scale_cleanup();
+	kernel_power_off();
+	return -EINVAL;
+}
+
 static int __init
 rcu_scale_init(void)
 {
-- 
2.40.1

[PATCH rcu 4/6] rcu/rcuscale: Stop kfree_scale_thread thread(s) after unloading rcuscale

[Paul E. McKenney]

From: Qiuxu Zhuo <qiuxu.zhuo@intel.com>

Running the 'kfree_rcu_test' test case [1] results in a splat [2].
The root cause is the kfree_scale_thread thread(s) continue running
after unloading the rcuscale module.  This commit fixes that isue by
invoking kfree_scale_cleanup() from rcu_scale_cleanup() when removing
the rcuscale module.

[1] modprobe rcuscale kfree_rcu_test=1
    // After some time
    rmmod rcuscale
    rmmod torture

[2] BUG: unable to handle page fault for address: ffffffffc0601a87
    #PF: supervisor instruction fetch in kernel mode
    #PF: error_code(0x0010) - not-present page
    PGD 11de4f067 P4D 11de4f067 PUD 11de51067 PMD 112f4d067 PTE 0
    Oops: 0010 [#1] PREEMPT SMP NOPTI
    CPU: 1 PID: 1798 Comm: kfree_scale_thr Not tainted 6.3.0-rc1-rcu+ #1
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015
    RIP: 0010:0xffffffffc0601a87
    Code: Unable to access opcode bytes at 0xffffffffc0601a5d.
    RSP: 0018:ffffb25bc2e57e18 EFLAGS: 00010297
    RAX: 0000000000000000 RBX: ffffffffc061f0b6 RCX: 0000000000000000
    RDX: 0000000000000000 RSI: ffffffff962fd0de RDI: ffffffff962fd0de
    RBP: ffffb25bc2e57ea8 R08: 0000000000000000 R09: 0000000000000000
    R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000
    R13: 0000000000000000 R14: 000000000000000a R15: 00000000001c1dbe
    FS:  0000000000000000(0000) GS:ffff921fa2200000(0000) knlGS:0000000000000000
    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: ffffffffc0601a5d CR3: 000000011de4c006 CR4: 0000000000370ee0
    DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
    DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
    Call Trace:
     <TASK>
     ? kvfree_call_rcu+0xf0/0x3a0
     ? kthread+0xf3/0x120
     ? kthread_complete_and_exit+0x20/0x20
     ? ret_from_fork+0x1f/0x30
     </TASK>
    Modules linked in: rfkill sunrpc ... [last unloaded: torture]
    CR2: ffffffffc0601a87
    ---[ end trace 0000000000000000 ]---

Fixes: e6e78b004fa7 ("rcuperf: Add kfree_rcu() performance Tests")
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/rcu/rcuscale.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
index 7e8965b0827a..d1221731c7cf 100644
--- a/kernel/rcu/rcuscale.c
+++ b/kernel/rcu/rcuscale.c
@@ -797,6 +797,11 @@ rcu_scale_cleanup(void)
 	if (gp_exp && gp_async)
 		SCALEOUT_ERRSTRING("No expedited async GPs, so went with async!");
 
+	if (kfree_rcu_test) {
+		kfree_scale_cleanup();
+		return;
+	}
+
 	if (torture_cleanup_begin())
 		return;
 	if (!cur_ops) {
-- 
2.40.1

[PATCH rcu 5/6] doc/rcutorture: Add description of rcutorture.stall_cpu_block

[Paul E. McKenney]

From: Zqiang <qiang1.zhang@intel.com>

If you build a kernel with CONFIG_PREEMPTION=n and CONFIG_PREEMPT_COUNT=y,
then run the rcutorture tests specifying stalls as follows:

runqemu kvm slirp nographic qemuparams="-m 1024 -smp 4" \
	bootparams="console=ttyS0 rcutorture.stall_cpu=30 \
	rcutorture.stall_no_softlockup=1 rcutorture.stall_cpu_block=1" -d

The tests will produce the following splat:

[   10.841071] rcu-torture: rcu_torture_stall begin CPU stall
[   10.841073] rcu_torture_stall start on CPU 3.
[   10.841077] BUG: scheduling while atomic: rcu_torture_sta/66/0x0000000
....
[   10.841108] Call Trace:
[   10.841110]  <TASK>
[   10.841112]  dump_stack_lvl+0x64/0xb0
[   10.841118]  dump_stack+0x10/0x20
[   10.841121]  __schedule_bug+0x8b/0xb0
[   10.841126]  __schedule+0x2172/0x2940
[   10.841157]  schedule+0x9b/0x150
[   10.841160]  schedule_timeout+0x2e8/0x4f0
[   10.841192]  schedule_timeout_uninterruptible+0x47/0x50
[   10.841195]  rcu_torture_stall+0x2e8/0x300
[   10.841199]  kthread+0x175/0x1a0
[   10.841206]  ret_from_fork+0x2c/0x50

This is because the rcutorture.stall_cpu_block=1 module parameter causes
rcu_torture_stall() to invoke schedule_timeout_uninterruptible() within
an RCU read-side critical section.  This in turn results in a quiescent
state (which prevents the stall) and a sleep in an atomic context (which
produces the above splat).

Although this code is operating as designed, the design has proven to
be counterintuitive to many.  This commit therefore updates the description
in kernel-parameters.txt accordingly.

Signed-off-by: Zqiang <qiang1.zhang@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 Documentation/admin-guide/kernel-parameters.txt | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 9e5bab29685f..eaffe0f8771d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5087,8 +5087,16 @@
 
 	rcutorture.stall_cpu_block= [KNL]
 			Sleep while stalling if set.  This will result
-			in warnings from preemptible RCU in addition
-			to any other stall-related activity.
+			in warnings from preemptible RCU in addition to
+			any other stall-related activity.  Note that
+			in kernels built with CONFIG_PREEMPTION=n and
+			CONFIG_PREEMPT_COUNT=y, this parameter will
+			cause the CPU to pass through a quiescent state.
+			Any such quiescent states will suppress RCU CPU
+			stall warnings, but the time-based sleep will
+			also result in scheduling-while-atomic splats.
+			Which might or might not be what you want.
+
 
 	rcutorture.stall_cpu_holdoff= [KNL]
 			Time to wait (s) after boot before inducing stall.
-- 
2.40.1

[PATCH rcu 6/6] torture: Remove duplicated argument -enable-kvm for ppc64

[Paul E. McKenney]

From: Zhouyi Zhou <zhouzhouyi@gmail.com>

The qemu argument -enable-kvm is duplicated because the qemu_args bash
variable in kvm-test-1-run.sh has already provides it.  This commit
therefore removes the ppc64-specific copy in functions.sh.

Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 tools/testing/selftests/rcutorture/bin/functions.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/rcutorture/bin/functions.sh b/tools/testing/selftests/rcutorture/bin/functions.sh
index b52d5069563c..48b9147e8c91 100644
--- a/tools/testing/selftests/rcutorture/bin/functions.sh
+++ b/tools/testing/selftests/rcutorture/bin/functions.sh
@@ -250,7 +250,7 @@ identify_qemu_args () {
 		echo -machine virt,gic-version=host -cpu host
 		;;
 	qemu-system-ppc64)
-		echo -enable-kvm -M pseries -nodefaults
+		echo -M pseries -nodefaults
 		echo -device spapr-vscsi
 		if test -n "$TORTURE_QEMU_INTERACTIVE" -a -n "$TORTURE_QEMU_MAC"
 		then
-- 
2.40.1

Re: [PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup()

[Joel Fernandes]

On Wed, May 10, 2023 at 10:12 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> From: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
>
> This code-movement-only commit moves the rcu_scale_cleanup() and
> rcu_scale_shutdown() functions to follow kfree_scale_cleanup().
> This is code movement is in preparation for a bug-fix patch that invokes
> kfree_scale_cleanup() from rcu_scale_cleanup().
>
> Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---
>  kernel/rcu/rcuscale.c | 194 +++++++++++++++++++++---------------------
>  1 file changed, 97 insertions(+), 97 deletions(-)

I wish diff was better at showing what really changed. The meld tool
can help but its gui...

Should I run meld later (I'm out at a conference so no access to
meld-capable machines) or are we sufficiently confident that the lines
were moved as-is ? :)

 - Joel


>
> diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
> index e82ec9f9a5d8..7e8965b0827a 100644
> --- a/kernel/rcu/rcuscale.c
> +++ b/kernel/rcu/rcuscale.c
> @@ -522,89 +522,6 @@ rcu_scale_print_module_parms(struct rcu_scale_ops *cur_ops, const char *tag)
>                  scale_type, tag, nrealreaders, nrealwriters, verbose, shutdown);
>  }
>
> -static void
> -rcu_scale_cleanup(void)
> -{
> -       int i;
> -       int j;
> -       int ngps = 0;
> -       u64 *wdp;
> -       u64 *wdpp;
> -
> -       /*
> -        * Would like warning at start, but everything is expedited
> -        * during the mid-boot phase, so have to wait till the end.
> -        */
> -       if (rcu_gp_is_expedited() && !rcu_gp_is_normal() && !gp_exp)
> -               SCALEOUT_ERRSTRING("All grace periods expedited, no normal ones to measure!");
> -       if (rcu_gp_is_normal() && gp_exp)
> -               SCALEOUT_ERRSTRING("All grace periods normal, no expedited ones to measure!");
> -       if (gp_exp && gp_async)
> -               SCALEOUT_ERRSTRING("No expedited async GPs, so went with async!");
> -
> -       if (torture_cleanup_begin())
> -               return;
> -       if (!cur_ops) {
> -               torture_cleanup_end();
> -               return;
> -       }
> -
> -       if (reader_tasks) {
> -               for (i = 0; i < nrealreaders; i++)
> -                       torture_stop_kthread(rcu_scale_reader,
> -                                            reader_tasks[i]);
> -               kfree(reader_tasks);
> -       }
> -
> -       if (writer_tasks) {
> -               for (i = 0; i < nrealwriters; i++) {
> -                       torture_stop_kthread(rcu_scale_writer,
> -                                            writer_tasks[i]);
> -                       if (!writer_n_durations)
> -                               continue;
> -                       j = writer_n_durations[i];
> -                       pr_alert("%s%s writer %d gps: %d\n",
> -                                scale_type, SCALE_FLAG, i, j);
> -                       ngps += j;
> -               }
> -               pr_alert("%s%s start: %llu end: %llu duration: %llu gps: %d batches: %ld\n",
> -                        scale_type, SCALE_FLAG,
> -                        t_rcu_scale_writer_started, t_rcu_scale_writer_finished,
> -                        t_rcu_scale_writer_finished -
> -                        t_rcu_scale_writer_started,
> -                        ngps,
> -                        rcuscale_seq_diff(b_rcu_gp_test_finished,
> -                                          b_rcu_gp_test_started));
> -               for (i = 0; i < nrealwriters; i++) {
> -                       if (!writer_durations)
> -                               break;
> -                       if (!writer_n_durations)
> -                               continue;
> -                       wdpp = writer_durations[i];
> -                       if (!wdpp)
> -                               continue;
> -                       for (j = 0; j < writer_n_durations[i]; j++) {
> -                               wdp = &wdpp[j];
> -                               pr_alert("%s%s %4d writer-duration: %5d %llu\n",
> -                                       scale_type, SCALE_FLAG,
> -                                       i, j, *wdp);
> -                               if (j % 100 == 0)
> -                                       schedule_timeout_uninterruptible(1);
> -                       }
> -                       kfree(writer_durations[i]);
> -               }
> -               kfree(writer_tasks);
> -               kfree(writer_durations);
> -               kfree(writer_n_durations);
> -       }
> -
> -       /* Do torture-type-specific cleanup operations.  */
> -       if (cur_ops->cleanup != NULL)
> -               cur_ops->cleanup();
> -
> -       torture_cleanup_end();
> -}
> -
>  /*
>   * Return the number if non-negative.  If -1, the number of CPUs.
>   * If less than -1, that much less than the number of CPUs, but
> @@ -624,20 +541,6 @@ static int compute_real(int n)
>         return nr;
>  }
>
> -/*
> - * RCU scalability shutdown kthread.  Just waits to be awakened, then shuts
> - * down system.
> - */
> -static int
> -rcu_scale_shutdown(void *arg)
> -{
> -       wait_event_idle(shutdown_wq, atomic_read(&n_rcu_scale_writer_finished) >= nrealwriters);
> -       smp_mb(); /* Wake before output. */
> -       rcu_scale_cleanup();
> -       kernel_power_off();
> -       return -EINVAL;
> -}
> -
>  /*
>   * kfree_rcu() scalability tests: Start a kfree_rcu() loop on all CPUs for number
>   * of iterations and measure total time and number of GP for all iterations to complete.
> @@ -874,6 +777,103 @@ kfree_scale_init(void)
>         return firsterr;
>  }
>
> +static void
> +rcu_scale_cleanup(void)
> +{
> +       int i;
> +       int j;
> +       int ngps = 0;
> +       u64 *wdp;
> +       u64 *wdpp;
> +
> +       /*
> +        * Would like warning at start, but everything is expedited
> +        * during the mid-boot phase, so have to wait till the end.
> +        */
> +       if (rcu_gp_is_expedited() && !rcu_gp_is_normal() && !gp_exp)
> +               SCALEOUT_ERRSTRING("All grace periods expedited, no normal ones to measure!");
> +       if (rcu_gp_is_normal() && gp_exp)
> +               SCALEOUT_ERRSTRING("All grace periods normal, no expedited ones to measure!");
> +       if (gp_exp && gp_async)
> +               SCALEOUT_ERRSTRING("No expedited async GPs, so went with async!");
> +
> +       if (torture_cleanup_begin())
> +               return;
> +       if (!cur_ops) {
> +               torture_cleanup_end();
> +               return;
> +       }
> +
> +       if (reader_tasks) {
> +               for (i = 0; i < nrealreaders; i++)
> +                       torture_stop_kthread(rcu_scale_reader,
> +                                            reader_tasks[i]);
> +               kfree(reader_tasks);
> +       }
> +
> +       if (writer_tasks) {
> +               for (i = 0; i < nrealwriters; i++) {
> +                       torture_stop_kthread(rcu_scale_writer,
> +                                            writer_tasks[i]);
> +                       if (!writer_n_durations)
> +                               continue;
> +                       j = writer_n_durations[i];
> +                       pr_alert("%s%s writer %d gps: %d\n",
> +                                scale_type, SCALE_FLAG, i, j);
> +                       ngps += j;
> +               }
> +               pr_alert("%s%s start: %llu end: %llu duration: %llu gps: %d batches: %ld\n",
> +                        scale_type, SCALE_FLAG,
> +                        t_rcu_scale_writer_started, t_rcu_scale_writer_finished,
> +                        t_rcu_scale_writer_finished -
> +                        t_rcu_scale_writer_started,
> +                        ngps,
> +                        rcuscale_seq_diff(b_rcu_gp_test_finished,
> +                                          b_rcu_gp_test_started));
> +               for (i = 0; i < nrealwriters; i++) {
> +                       if (!writer_durations)
> +                               break;
> +                       if (!writer_n_durations)
> +                               continue;
> +                       wdpp = writer_durations[i];
> +                       if (!wdpp)
> +                               continue;
> +                       for (j = 0; j < writer_n_durations[i]; j++) {
> +                               wdp = &wdpp[j];
> +                               pr_alert("%s%s %4d writer-duration: %5d %llu\n",
> +                                       scale_type, SCALE_FLAG,
> +                                       i, j, *wdp);
> +                               if (j % 100 == 0)
> +                                       schedule_timeout_uninterruptible(1);
> +                       }
> +                       kfree(writer_durations[i]);
> +               }
> +               kfree(writer_tasks);
> +               kfree(writer_durations);
> +               kfree(writer_n_durations);
> +       }
> +
> +       /* Do torture-type-specific cleanup operations.  */
> +       if (cur_ops->cleanup != NULL)
> +               cur_ops->cleanup();
> +
> +       torture_cleanup_end();
> +}
> +
> +/*
> + * RCU scalability shutdown kthread.  Just waits to be awakened, then shuts
> + * down system.
> + */
> +static int
> +rcu_scale_shutdown(void *arg)
> +{
> +       wait_event_idle(shutdown_wq, atomic_read(&n_rcu_scale_writer_finished) >= nrealwriters);
> +       smp_mb(); /* Wake before output. */
> +       rcu_scale_cleanup();
> +       kernel_power_off();
> +       return -EINVAL;
> +}
> +
>  static int __init
>  rcu_scale_init(void)
>  {
> --
> 2.40.1
>

Re: [PATCH rcu 6/6] torture: Remove duplicated argument -enable-kvm for ppc64

[Joel Fernandes]

On Wed, May 10, 2023 at 10:13 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> From: Zhouyi Zhou <zhouzhouyi@gmail.com>
>
> The qemu argument -enable-kvm is duplicated because the qemu_args bash
> variable in kvm-test-1-run.sh has already provides it.  This commit

drop the has.

 - Joel

> therefore removes the ppc64-specific copy in functions.sh.
>
> Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---
>  tools/testing/selftests/rcutorture/bin/functions.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/rcutorture/bin/functions.sh b/tools/testing/selftests/rcutorture/bin/functions.sh
> index b52d5069563c..48b9147e8c91 100644
> --- a/tools/testing/selftests/rcutorture/bin/functions.sh
> +++ b/tools/testing/selftests/rcutorture/bin/functions.sh
> @@ -250,7 +250,7 @@ identify_qemu_args () {
>                 echo -machine virt,gic-version=host -cpu host
>                 ;;
>         qemu-system-ppc64)
> -               echo -enable-kvm -M pseries -nodefaults
> +               echo -M pseries -nodefaults
>                 echo -device spapr-vscsi
>                 if test -n "$TORTURE_QEMU_INTERACTIVE" -a -n "$TORTURE_QEMU_MAC"
>                 then
> --
> 2.40.1
>

Re: [PATCH rcu 5/6] doc/rcutorture: Add description of rcutorture.stall_cpu_block

[Joel Fernandes]

On Wed, May 10, 2023 at 10:12 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> From: Zqiang <qiang1.zhang@intel.com>
>
> If you build a kernel with CONFIG_PREEMPTION=n and CONFIG_PREEMPT_COUNT=y,
> then run the rcutorture tests specifying stalls as follows:
>
> runqemu kvm slirp nographic qemuparams="-m 1024 -smp 4" \
>         bootparams="console=ttyS0 rcutorture.stall_cpu=30 \
>         rcutorture.stall_no_softlockup=1 rcutorture.stall_cpu_block=1" -d
>
> The tests will produce the following splat:
>
> [   10.841071] rcu-torture: rcu_torture_stall begin CPU stall
> [   10.841073] rcu_torture_stall start on CPU 3.
> [   10.841077] BUG: scheduling while atomic: rcu_torture_sta/66/0x0000000
> ....
> [   10.841108] Call Trace:
> [   10.841110]  <TASK>
> [   10.841112]  dump_stack_lvl+0x64/0xb0
> [   10.841118]  dump_stack+0x10/0x20
> [   10.841121]  __schedule_bug+0x8b/0xb0
> [   10.841126]  __schedule+0x2172/0x2940
> [   10.841157]  schedule+0x9b/0x150
> [   10.841160]  schedule_timeout+0x2e8/0x4f0
> [   10.841192]  schedule_timeout_uninterruptible+0x47/0x50
> [   10.841195]  rcu_torture_stall+0x2e8/0x300
> [   10.841199]  kthread+0x175/0x1a0
> [   10.841206]  ret_from_fork+0x2c/0x50

Another way to get rid of the warning would be to replace the
cur_ops->readlock() with rcu_read_lock(). Though perhaps that will not
test whether the particular RCU flavor under testing is capable of
causing a stall :-).

>         rcutorture.stall_cpu_block= [KNL]
>                         Sleep while stalling if set.  This will result
> -                       in warnings from preemptible RCU in addition
> -                       to any other stall-related activity.
> +                       in warnings from preemptible RCU in addition to
> +                       any other stall-related activity.  Note that
> +                       in kernels built with CONFIG_PREEMPTION=n and
> +                       CONFIG_PREEMPT_COUNT=y, this parameter will
> +                       cause the CPU to pass through a quiescent state.
> +                       Any such quiescent states will suppress RCU CPU
> +                       stall warnings, but the time-based sleep will
> +                       also result in scheduling-while-atomic splats.

Could change last part to "but may also result in
scheduling-while-atomic splats as preemption might be disabled for
certain RCU flavors in order to cause the stall".

> +                       Which might or might not be what you want.
> +

Suggest drop this line ;-).

 - Joel

>         rcutorture.stall_cpu_holdoff= [KNL]
>                         Time to wait (s) after boot before inducing stall.
> --
> 2.40.1
>

Re: [PATCH rcu 0/6] Torture-test updates for v6.5

[Joel Fernandes]

On Wed, May 10, 2023 at 10:12 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> Hello!
>
> This series contains torture-test and torture-test-scripting updates
> for v6.5:
>
> 1.      Add long_hold to adjust lock-hold delays.
>
> 2.      Correct name of use_softirq module parameter.
>
> 3.      rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup(),
>         courtesy of Qiuxu Zhuo.
>
> 4.      rcu/rcuscale: Stop kfree_scale_thread thread(s) after unloading
>         rcuscale, courtesy of Qiuxu Zhuo.
>
> 5.      doc/rcutorture: Add description of rcutorture.stall_cpu_block,
>         courtesy of Zqiang.
>
> 6.      Remove duplicated argument -enable-kvm for ppc64, courtesy of
>         Zhouyi Zhou.

Other than the small nits I mentioned, this series LGTM. Feel free to add:

Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>

 - Joel


>
>                                                 Thanx, Paul
>
> ------------------------------------------------------------------------
>
>  b/Documentation/admin-guide/kernel-parameters.txt                  |   12
>  b/kernel/locking/locktorture.c                                     |   51 +-
>  b/kernel/rcu/rcuscale.c                                            |  194 +++++-----
>  b/tools/testing/selftests/rcutorture/bin/functions.sh              |    2
>  b/tools/testing/selftests/rcutorture/configs/rcu/BUSTED-BOOST.boot |    2
>  b/tools/testing/selftests/rcutorture/configs/rcu/TREE03.boot       |    2
>  kernel/rcu/rcuscale.c                                              |    5
>  7 files changed, 137 insertions(+), 131 deletions(-)

Re: [PATCH rcu 6/6] torture: Remove duplicated argument -enable-kvm for ppc64

[Zhouyi Zhou]

On Thu, May 11, 2023 at 1:26 PM Joel Fernandes <joel@joelfernandes.org> wrote:
>
> On Wed, May 10, 2023 at 10:13 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > From: Zhouyi Zhou <zhouzhouyi@gmail.com>
> >
> > The qemu argument -enable-kvm is duplicated because the qemu_args bash
> > variable in kvm-test-1-run.sh has already provides it.  This commit
>
> drop the has.
Thank Joel for the fix ;-)
Should I resend the patch, or would Paul do me the favor instead ;-)

Thank you all
Zhouyi
>
>  - Joel
>
> > therefore removes the ppc64-specific copy in functions.sh.
> >
> > Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > ---
> >  tools/testing/selftests/rcutorture/bin/functions.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/rcutorture/bin/functions.sh b/tools/testing/selftests/rcutorture/bin/functions.sh
> > index b52d5069563c..48b9147e8c91 100644
> > --- a/tools/testing/selftests/rcutorture/bin/functions.sh
> > +++ b/tools/testing/selftests/rcutorture/bin/functions.sh
> > @@ -250,7 +250,7 @@ identify_qemu_args () {
> >                 echo -machine virt,gic-version=host -cpu host
> >                 ;;
> >         qemu-system-ppc64)
> > -               echo -enable-kvm -M pseries -nodefaults
> > +               echo -M pseries -nodefaults
> >                 echo -device spapr-vscsi
> >                 if test -n "$TORTURE_QEMU_INTERACTIVE" -a -n "$TORTURE_QEMU_MAC"
> >                 then
> > --
> > 2.40.1
> >

RE: [PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup()

[Zhuo, Qiuxu]

> From: Joel Fernandes <joel@joelfernandes.org>
> Sent: Thursday, May 11, 2023 1:23 PM
> To: Paul E. McKenney <paulmck@kernel.org>
> Cc: rcu@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-
> team@meta.com; rostedt@goodmis.org; Zhuo, Qiuxu
> <qiuxu.zhuo@intel.com>
> Subject: Re: [PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after
> kfree_scale_cleanup()
> 
> On Wed, May 10, 2023 at 10:12 AM Paul E. McKenney <paulmck@kernel.org>
> wrote:
> >
> > From: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> >
> > This code-movement-only commit moves the rcu_scale_cleanup() and
> > rcu_scale_shutdown() functions to follow kfree_scale_cleanup().
> > This is code movement is in preparation for a bug-fix patch that
> > invokes
> > kfree_scale_cleanup() from rcu_scale_cleanup().
> >
> > Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > ---
> >  kernel/rcu/rcuscale.c | 194
> > +++++++++++++++++++++---------------------
> >  1 file changed, 97 insertions(+), 97 deletions(-)
> 
> I wish diff was better at showing what really changed. The meld tool can help
> but its gui...
> 
> Should I run meld later (I'm out at a conference so no access to meld-capable
> machines) or are we sufficiently confident that the lines were moved as-is ? :)
> 

Thank you, Joel for this concern. Good to know the meld diff GUI tool. 
I just run the command below and confirmed that the lines were moved 
as-is: rcu_scale_{cleanup,shutdown}() follows kfree_scale_cleanup().
You may double check it ;-).

      meld --diff ./rcuscale.c.before ./rcuscale.c.after

-Qiuxu

>  - Joel

Re: [PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup()

[Paul E. McKenney]

On Thu, May 11, 2023 at 07:01:59AM +0000, Zhuo, Qiuxu wrote:
> > From: Joel Fernandes <joel@joelfernandes.org>
> > Sent: Thursday, May 11, 2023 1:23 PM
> > To: Paul E. McKenney <paulmck@kernel.org>
> > Cc: rcu@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-
> > team@meta.com; rostedt@goodmis.org; Zhuo, Qiuxu
> > <qiuxu.zhuo@intel.com>
> > Subject: Re: [PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after
> > kfree_scale_cleanup()
> > 
> > On Wed, May 10, 2023 at 10:12 AM Paul E. McKenney <paulmck@kernel.org>
> > wrote:
> > >
> > > From: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> > >
> > > This code-movement-only commit moves the rcu_scale_cleanup() and
> > > rcu_scale_shutdown() functions to follow kfree_scale_cleanup().
> > > This is code movement is in preparation for a bug-fix patch that
> > > invokes
> > > kfree_scale_cleanup() from rcu_scale_cleanup().
> > >
> > > Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > ---
> > >  kernel/rcu/rcuscale.c | 194
> > > +++++++++++++++++++++---------------------
> > >  1 file changed, 97 insertions(+), 97 deletions(-)
> > 
> > I wish diff was better at showing what really changed. The meld tool can help
> > but its gui...
> > 
> > Should I run meld later (I'm out at a conference so no access to meld-capable
> > machines) or are we sufficiently confident that the lines were moved as-is ? :)
> > 
> 
> Thank you, Joel for this concern. Good to know the meld diff GUI tool. 
> I just run the command below and confirmed that the lines were moved 
> as-is: rcu_scale_{cleanup,shutdown}() follows kfree_scale_cleanup().
> You may double check it ;-).
> 
>       meld --diff ./rcuscale.c.before ./rcuscale.c.after

Nice, thank you both!

Another option is to check out the commit corresponding to this patch,
then do "git blame -M kernel/rcu/rcuscale.c".  Given a move-only commit,
there should be no line tagged with this commit's SHA-1.

They say that another option is to use "git diff --color-moved", which
colors the changes.  That it does, but I am hard-pressed to work out
exactly what distinguishes a moved hunk from an added or removed hunk.
Fall colors vs. winter colors?  Exterior vs. interior?  Any particular
decade in the endless rush of changes to fashion?  Perhaps someone with
normal color vision (to say nothing of better fashion sense) could try it.

On the other hand: "default: Is a synonym for zebra. This may change to
a more sensible mode in the future."  So maybe it is not just me.  ;-)

You can also apparently choose colors using "color.diff.newMoved" and
"color.diff.oldMoved" when using "--color-moved=plain".

But "git diff --color-moved=dimmed-zebra" might be more to the point for
someone like me.  I would need to experiment with it more in order to
confirm my hypotheses about what it is doing.  To say nothing of building
trust in it.  Plus I have to open a color terminal to use it effectively.
So maybe "git blame -M" continues to be the tool for me?

							Thanx, Paul

Re: [PATCH rcu 6/6] torture: Remove duplicated argument -enable-kvm for ppc64

[Paul E. McKenney]

On Thu, May 11, 2023 at 02:18:57PM +0800, Zhouyi Zhou wrote:
> On Thu, May 11, 2023 at 1:26 PM Joel Fernandes <joel@joelfernandes.org> wrote:
> >
> > On Wed, May 10, 2023 at 10:13 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > From: Zhouyi Zhou <zhouzhouyi@gmail.com>
> > >
> > > The qemu argument -enable-kvm is duplicated because the qemu_args bash
> > > variable in kvm-test-1-run.sh has already provides it.  This commit
> >
> > drop the has.

Good eyes, and thank you!

> Thank Joel for the fix ;-)
> Should I resend the patch, or would Paul do me the favor instead ;-)
> 
> Thank you all

I will do it on my next rebase.  ;-)

							Thanx, Paul

> Zhouyi
> >
> >  - Joel
> >
> > > therefore removes the ppc64-specific copy in functions.sh.
> > >
> > > Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > ---
> > >  tools/testing/selftests/rcutorture/bin/functions.sh | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/testing/selftests/rcutorture/bin/functions.sh b/tools/testing/selftests/rcutorture/bin/functions.sh
> > > index b52d5069563c..48b9147e8c91 100644
> > > --- a/tools/testing/selftests/rcutorture/bin/functions.sh
> > > +++ b/tools/testing/selftests/rcutorture/bin/functions.sh
> > > @@ -250,7 +250,7 @@ identify_qemu_args () {
> > >                 echo -machine virt,gic-version=host -cpu host
> > >                 ;;
> > >         qemu-system-ppc64)
> > > -               echo -enable-kvm -M pseries -nodefaults
> > > +               echo -M pseries -nodefaults
> > >                 echo -device spapr-vscsi
> > >                 if test -n "$TORTURE_QEMU_INTERACTIVE" -a -n "$TORTURE_QEMU_MAC"
> > >                 then
> > > --
> > > 2.40.1
> > >

Re: [PATCH rcu 5/6] doc/rcutorture: Add description of rcutorture.stall_cpu_block

[Paul E. McKenney]

On Wed, May 10, 2023 at 10:47:36PM -0700, Joel Fernandes wrote:
> On Wed, May 10, 2023 at 10:12 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > From: Zqiang <qiang1.zhang@intel.com>
> >
> > If you build a kernel with CONFIG_PREEMPTION=n and CONFIG_PREEMPT_COUNT=y,
> > then run the rcutorture tests specifying stalls as follows:
> >
> > runqemu kvm slirp nographic qemuparams="-m 1024 -smp 4" \
> >         bootparams="console=ttyS0 rcutorture.stall_cpu=30 \
> >         rcutorture.stall_no_softlockup=1 rcutorture.stall_cpu_block=1" -d
> >
> > The tests will produce the following splat:
> >
> > [   10.841071] rcu-torture: rcu_torture_stall begin CPU stall
> > [   10.841073] rcu_torture_stall start on CPU 3.
> > [   10.841077] BUG: scheduling while atomic: rcu_torture_sta/66/0x0000000
> > ....
> > [   10.841108] Call Trace:
> > [   10.841110]  <TASK>
> > [   10.841112]  dump_stack_lvl+0x64/0xb0
> > [   10.841118]  dump_stack+0x10/0x20
> > [   10.841121]  __schedule_bug+0x8b/0xb0
> > [   10.841126]  __schedule+0x2172/0x2940
> > [   10.841157]  schedule+0x9b/0x150
> > [   10.841160]  schedule_timeout+0x2e8/0x4f0
> > [   10.841192]  schedule_timeout_uninterruptible+0x47/0x50
> > [   10.841195]  rcu_torture_stall+0x2e8/0x300
> > [   10.841199]  kthread+0x175/0x1a0
> > [   10.841206]  ret_from_fork+0x2c/0x50
> 
> Another way to get rid of the warning would be to replace the
> cur_ops->readlock() with rcu_read_lock(). Though perhaps that will not
> test whether the particular RCU flavor under testing is capable of
> causing a stall :-).

Exactly!

> >         rcutorture.stall_cpu_block= [KNL]
> >                         Sleep while stalling if set.  This will result
> > -                       in warnings from preemptible RCU in addition
> > -                       to any other stall-related activity.
> > +                       in warnings from preemptible RCU in addition to
> > +                       any other stall-related activity.  Note that
> > +                       in kernels built with CONFIG_PREEMPTION=n and
> > +                       CONFIG_PREEMPT_COUNT=y, this parameter will
> > +                       cause the CPU to pass through a quiescent state.
> > +                       Any such quiescent states will suppress RCU CPU
> > +                       stall warnings, but the time-based sleep will
> > +                       also result in scheduling-while-atomic splats.
> 
> Could change last part to "but may also result in
> scheduling-while-atomic splats as preemption might be disabled for
> certain RCU flavors in order to cause the stall".

Is that needed given the earlier "in kernels built with
CONFIG_PREEMPTION=n and CONFIG_PREEMPT_COUNT=y"?

> > +                       Which might or might not be what you want.
> > +
> 
> Suggest drop this line ;-).

OK, I will bite.  ;-)

What is your concern with this line?

							Thanx, Paul

>  - Joel
> 
> >         rcutorture.stall_cpu_holdoff= [KNL]
> >                         Time to wait (s) after boot before inducing stall.
> > --
> > 2.40.1
> >

Re: [PATCH rcu 0/6] Torture-test updates for v6.5

[Paul E. McKenney]

On Wed, May 10, 2023 at 10:48:21PM -0700, Joel Fernandes wrote:
> On Wed, May 10, 2023 at 10:12 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > Hello!
> >
> > This series contains torture-test and torture-test-scripting updates
> > for v6.5:
> >
> > 1.      Add long_hold to adjust lock-hold delays.
> >
> > 2.      Correct name of use_softirq module parameter.
> >
> > 3.      rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup(),
> >         courtesy of Qiuxu Zhuo.
> >
> > 4.      rcu/rcuscale: Stop kfree_scale_thread thread(s) after unloading
> >         rcuscale, courtesy of Qiuxu Zhuo.
> >
> > 5.      doc/rcutorture: Add description of rcutorture.stall_cpu_block,
> >         courtesy of Zqiang.
> >
> > 6.      Remove duplicated argument -enable-kvm for ppc64, courtesy of
> >         Zhouyi Zhou.
> 
> Other than the small nits I mentioned, this series LGTM. Feel free to add:
> 
> Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>

I will do 1-4 and 6 right on my next rebase, and 5 once we come to
agreement.

							Thanx, Paul

>  - Joel
> 
> 
> >
> >                                                 Thanx, Paul
> >
> > ------------------------------------------------------------------------
> >
> >  b/Documentation/admin-guide/kernel-parameters.txt                  |   12
> >  b/kernel/locking/locktorture.c                                     |   51 +-
> >  b/kernel/rcu/rcuscale.c                                            |  194 +++++-----
> >  b/tools/testing/selftests/rcutorture/bin/functions.sh              |    2
> >  b/tools/testing/selftests/rcutorture/configs/rcu/BUSTED-BOOST.boot |    2
> >  b/tools/testing/selftests/rcutorture/configs/rcu/TREE03.boot       |    2
> >  kernel/rcu/rcuscale.c                                              |    5
> >  7 files changed, 137 insertions(+), 131 deletions(-)

RE: [PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup()

[Zhuo, Qiuxu]

> From: Paul E. McKenney <paulmck@kernel.org>
> ...
> > > I wish diff was better at showing what really changed. The meld tool
> > > can help but its gui...
> > >
> > > Should I run meld later (I'm out at a conference so no access to
> > > meld-capable
> > > machines) or are we sufficiently confident that the lines were moved
> > > as-is ? :)
> > >
> >
> > Thank you, Joel for this concern. Good to know the meld diff GUI tool.
> > I just run the command below and confirmed that the lines were moved
> > as-is: rcu_scale_{cleanup,shutdown}() follows kfree_scale_cleanup().
> > You may double check it ;-).
> >
> >       meld --diff ./rcuscale.c.before ./rcuscale.c.after
> 
> Nice, thank you both!
> 
> Another option is to check out the commit corresponding to this patch, then
> do "git blame -M kernel/rcu/rcuscale.c".  Given a move-only commit, there
> should be no line tagged with this commit's SHA-1.

Just had a good experiment with the "git blame -M" option: 
- Used this option to prove a move-only commit quickly (no line tagged with that commit) (the fastest method to me). 
- Then just only needed to quickly check the positions of the moved code chunk by myself (easy).  

Thank you, Paul for sharing this. It's very useful to me. 

> They say that another option is to use "git diff --color-moved", which colors
> the changes.  That it does, but I am hard-pressed to work out exactly what
> distinguishes a moved hunk from an added or removed hunk.
> Fall colors vs. winter colors?  Exterior vs. interior?  Any particular decade in
> the endless rush of changes to fashion?  Perhaps someone with normal color
> vision (to say nothing of better fashion sense) could try it.
> 
> On the other hand: "default: Is a synonym for zebra. This may change to a
> more sensible mode in the future."  So maybe it is not just me.  ;-)
> 
> You can also apparently choose colors using "color.diff.newMoved" and
> "color.diff.oldMoved" when using "--color-moved=plain".
> 
> But "git diff --color-moved=dimmed-zebra" might be more to the point for
> someone like me.  I would need to experiment with it more in order to
> confirm my hypotheses about what it is doing.  To say nothing of building

Yup, this looks a bit painful for me too (need experiments to confirm hypotheses ...). 

> trust in it.  Plus I have to open a color terminal to use it effectively.
> So maybe "git blame -M" continues to be the tool for me?
> 
> 							Thanx, Paul

Re: [PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup()

[Joel Fernandes]

> On May 11, 2023, at 8:20 PM, Zhuo, Qiuxu <qiuxu.zhuo@intel.com> wrote:
> 
> 
>> 
>> From: Paul E. McKenney <paulmck@kernel.org>
>> ...
>>>> I wish diff was better at showing what really changed. The meld tool
>>>> can help but its gui...
>>>> 
>>>> Should I run meld later (I'm out at a conference so no access to
>>>> meld-capable
>>>> machines) or are we sufficiently confident that the lines were moved
>>>> as-is ? :)
>>>> 
>>> 
>>> Thank you, Joel for this concern. Good to know the meld diff GUI tool.
>>> I just run the command below and confirmed that the lines were moved
>>> as-is: rcu_scale_{cleanup,shutdown}() follows kfree_scale_cleanup().
>>> You may double check it ;-).
>>> 
>>>      meld --diff ./rcuscale.c.before ./rcuscale.c.after
>> 
>> Nice, thank you both!
>> 
>> Another option is to check out the commit corresponding to this patch, then
>> do "git blame -M kernel/rcu/rcuscale.c".  Given a move-only commit, there
>> should be no line tagged with this commit's SHA-1.
> 
> Just had a good experiment with the "git blame -M" option: 
> - Used this option to prove a move-only commit quickly (no line tagged with that commit) (the fastest method to me). 
> - Then just only needed to quickly check the positions of the moved code chunk by myself (easy).  
> 
> Thank you, Paul for sharing this. It's very useful to me. 

Looks good to me as well and thank you both for sharing the tips.

 - Joel



> 
>> They say that another option is to use "git diff --color-moved", which colors
>> the changes.  That it does, but I am hard-pressed to work out exactly what
>> distinguishes a moved hunk from an added or removed hunk.
>> Fall colors vs. winter colors?  Exterior vs. interior?  Any particular decade in
>> the endless rush of changes to fashion?  Perhaps someone with normal color
>> vision (to say nothing of better fashion sense) could try it.
>> 
>> On the other hand: "default: Is a synonym for zebra. This may change to a
>> more sensible mode in the future."  So maybe it is not just me.  ;-)
>> 
>> You can also apparently choose colors using "color.diff.newMoved" and
>> "color.diff.oldMoved" when using "--color-moved=plain".
>> 
>> But "git diff --color-moved=dimmed-zebra" might be more to the point for
>> someone like me.  I would need to experiment with it more in order to
>> confirm my hypotheses about what it is doing.  To say nothing of building
> 
> Yup, this looks a bit painful for me too (need experiments to confirm hypotheses ...). 
> 
>> trust in it.  Plus I have to open a color terminal to use it effectively.
>> So maybe "git blame -M" continues to be the tool for me?
>> 
>>                            Thanx, Paul

Re: [PATCH rcu 5/6] doc/rcutorture: Add description of rcutorture.stall_cpu_block

[Joel Fernandes]

On Thu, May 11, 2023 at 11:11 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Wed, May 10, 2023 at 10:47:36PM -0700, Joel Fernandes wrote:
> > On Wed, May 10, 2023 at 10:12 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > From: Zqiang <qiang1.zhang@intel.com>
> > >
> > > If you build a kernel with CONFIG_PREEMPTION=n and CONFIG_PREEMPT_COUNT=y,
> > > then run the rcutorture tests specifying stalls as follows:
> > >
> > > runqemu kvm slirp nographic qemuparams="-m 1024 -smp 4" \
> > >         bootparams="console=ttyS0 rcutorture.stall_cpu=30 \
> > >         rcutorture.stall_no_softlockup=1 rcutorture.stall_cpu_block=1" -d
> > >
> > > The tests will produce the following splat:
> > >
> > > [   10.841071] rcu-torture: rcu_torture_stall begin CPU stall
> > > [   10.841073] rcu_torture_stall start on CPU 3.
> > > [   10.841077] BUG: scheduling while atomic: rcu_torture_sta/66/0x0000000
> > > ....
> > > [   10.841108] Call Trace:
> > > [   10.841110]  <TASK>
> > > [   10.841112]  dump_stack_lvl+0x64/0xb0
> > > [   10.841118]  dump_stack+0x10/0x20
> > > [   10.841121]  __schedule_bug+0x8b/0xb0
> > > [   10.841126]  __schedule+0x2172/0x2940
> > > [   10.841157]  schedule+0x9b/0x150
> > > [   10.841160]  schedule_timeout+0x2e8/0x4f0
> > > [   10.841192]  schedule_timeout_uninterruptible+0x47/0x50
> > > [   10.841195]  rcu_torture_stall+0x2e8/0x300
> > > [   10.841199]  kthread+0x175/0x1a0
> > > [   10.841206]  ret_from_fork+0x2c/0x50
> >
> > Another way to get rid of the warning would be to replace the
> > cur_ops->readlock() with rcu_read_lock(). Though perhaps that will not
> > test whether the particular RCU flavor under testing is capable of
> > causing a stall :-).
>
> Exactly!
>
> > >         rcutorture.stall_cpu_block= [KNL]
> > >                         Sleep while stalling if set.  This will result
> > > -                       in warnings from preemptible RCU in addition
> > > -                       to any other stall-related activity.
> > > +                       in warnings from preemptible RCU in addition to
> > > +                       any other stall-related activity.  Note that
> > > +                       in kernels built with CONFIG_PREEMPTION=n and
> > > +                       CONFIG_PREEMPT_COUNT=y, this parameter will
> > > +                       cause the CPU to pass through a quiescent state.
> > > +                       Any such quiescent states will suppress RCU CPU
> > > +                       stall warnings, but the time-based sleep will
> > > +                       also result in scheduling-while-atomic splats.
> >
> > Could change last part to "but may also result in
> > scheduling-while-atomic splats as preemption might be disabled for
> > certain RCU flavors in order to cause the stall".
>
> Is that needed given the earlier "in kernels built with
> CONFIG_PREEMPTION=n and CONFIG_PREEMPT_COUNT=y"?

Hmm, I guess is not clear to the reader without code reading about why
preempt got disabled. So I would add that last part I mentioned, but I
am Ok either way, it is just a suggestion.

>
> > > +                       Which might or might not be what you want.
> > > +
> >
> > Suggest drop this line ;-).
>
> OK, I will bite.  ;-)
>
> What is your concern with this line?

It is not needed IMO.

thanks,

 - Joel


> > >         rcutorture.stall_cpu_holdoff= [KNL]
> > >                         Time to wait (s) after boot before inducing stall.
> > > --
> > > 2.40.1
> > >

Re: [PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup()

[Paul E. McKenney]

On Thu, May 11, 2023 at 09:15:17PM -0700, Joel Fernandes wrote:
> 
> 
> > On May 11, 2023, at 8:20 PM, Zhuo, Qiuxu <qiuxu.zhuo@intel.com> wrote:
> > 
> > 
> >> 
> >> From: Paul E. McKenney <paulmck@kernel.org>
> >> ...
> >>>> I wish diff was better at showing what really changed. The meld tool
> >>>> can help but its gui...
> >>>> 
> >>>> Should I run meld later (I'm out at a conference so no access to
> >>>> meld-capable
> >>>> machines) or are we sufficiently confident that the lines were moved
> >>>> as-is ? :)
> >>>> 
> >>> 
> >>> Thank you, Joel for this concern. Good to know the meld diff GUI tool.
> >>> I just run the command below and confirmed that the lines were moved
> >>> as-is: rcu_scale_{cleanup,shutdown}() follows kfree_scale_cleanup().
> >>> You may double check it ;-).
> >>> 
> >>>      meld --diff ./rcuscale.c.before ./rcuscale.c.after
> >> 
> >> Nice, thank you both!
> >> 
> >> Another option is to check out the commit corresponding to this patch, then
> >> do "git blame -M kernel/rcu/rcuscale.c".  Given a move-only commit, there
> >> should be no line tagged with this commit's SHA-1.
> > 
> > Just had a good experiment with the "git blame -M" option: 
> > - Used this option to prove a move-only commit quickly (no line tagged with that commit) (the fastest method to me). 
> > - Then just only needed to quickly check the positions of the moved code chunk by myself (easy).  
> > 
> > Thank you, Paul for sharing this. It's very useful to me. 
> 
> Looks good to me as well and thank you both for sharing the tips.

Here is one way to script this, where "SHA" identifies the commit to
be checked and PATHS the affected pathnames:

	git checkout SHA^
	git show SHA | git apply -
	git blame -M PATHS | grep '^0* '

If there is no output, there were no non-move changes.  

Or just do the "git blame -M PATHS | grep '^0* '" before doing the
checking.

And yes, you can derive PATHS using "git status" if you want.  ;-)

							Thanx, Paul

RE: [PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup()

[Zhuo, Qiuxu]

> From: Paul E. McKenney <paulmck@kernel.org>
> ...
> > >>>> I wish diff was better at showing what really changed. The meld
> > >>>> tool can help but its gui...
> > >>>>
> > >>>> Should I run meld later (I'm out at a conference so no access to
> > >>>> meld-capable
> > >>>> machines) or are we sufficiently confident that the lines were
> > >>>> moved as-is ? :)
> > >>>>
> > >>>
> > >>> Thank you, Joel for this concern. Good to know the meld diff GUI tool.
> > >>> I just run the command below and confirmed that the lines were
> > >>> moved
> > >>> as-is: rcu_scale_{cleanup,shutdown}() follows kfree_scale_cleanup().
> > >>> You may double check it ;-).
> > >>>
> > >>>      meld --diff ./rcuscale.c.before ./rcuscale.c.after
> > >>
> > >> Nice, thank you both!
> > >>
> > >> Another option is to check out the commit corresponding to this
> > >> patch, then do "git blame -M kernel/rcu/rcuscale.c".  Given a
> > >> move-only commit, there should be no line tagged with this commit's
> SHA-1.
> > >
> > > Just had a good experiment with the "git blame -M" option:
> > > - Used this option to prove a move-only commit quickly (no line tagged
> with that commit) (the fastest method to me).
> > > - Then just only needed to quickly check the positions of the moved code
> chunk by myself (easy).
> > >
> > > Thank you, Paul for sharing this. It's very useful to me.
> >
> > Looks good to me as well and thank you both for sharing the tips.
> 
> Here is one way to script this, where "SHA" identifies the commit to be
> checked and PATHS the affected pathnames:
> 
> 	git checkout SHA^
> 	git show SHA | git apply -
> 	git blame -M PATHS | grep '^0* '

Cool ~. Thank you, Paul.  
I took them and made them into a script below for future use ;-)

#!/bin/bash

SHA=$1

if [ -z "$SHA" ]; then
    echo "Usage: $0 <commit-id>"
    exit 1
fi

if ! git cat-file -t "$SHA" &> /dev/null; then
        echo "$SHA does not exist in the repository"
        exit 1
fi

git checkout ${SHA}^ &> /dev/null
git show ${SHA} | git apply - &> /dev/null

PATHS=`git status| grep "modified:" | cut -d: -f2 | xargs`

for P in ${PATHS}; do
        R=`git blame -M $P | grep '^0* '`
        if test -n "$R"; then
                echo "$SHA is NOT a move-only commit"
                exit 1
        fi
done

echo "$SHA is a move-only commit"

> If there is no output, there were no non-move changes.
> 
> Or just do the "git blame -M PATHS | grep '^0* '" before doing the checking.
> 
> And yes, you can derive PATHS using "git status" if you want.  ;-)
> 							Thanx, Paul

Re: [PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup()

[Paul E. McKenney]

On Sat, May 13, 2023 at 09:52:46AM +0000, Zhuo, Qiuxu wrote:
> > From: Paul E. McKenney <paulmck@kernel.org>
> > ...
> > > >>>> I wish diff was better at showing what really changed. The meld
> > > >>>> tool can help but its gui...
> > > >>>>
> > > >>>> Should I run meld later (I'm out at a conference so no access to
> > > >>>> meld-capable
> > > >>>> machines) or are we sufficiently confident that the lines were
> > > >>>> moved as-is ? :)
> > > >>>>
> > > >>>
> > > >>> Thank you, Joel for this concern. Good to know the meld diff GUI tool.
> > > >>> I just run the command below and confirmed that the lines were
> > > >>> moved
> > > >>> as-is: rcu_scale_{cleanup,shutdown}() follows kfree_scale_cleanup().
> > > >>> You may double check it ;-).
> > > >>>
> > > >>>      meld --diff ./rcuscale.c.before ./rcuscale.c.after
> > > >>
> > > >> Nice, thank you both!
> > > >>
> > > >> Another option is to check out the commit corresponding to this
> > > >> patch, then do "git blame -M kernel/rcu/rcuscale.c".  Given a
> > > >> move-only commit, there should be no line tagged with this commit's
> > SHA-1.
> > > >
> > > > Just had a good experiment with the "git blame -M" option:
> > > > - Used this option to prove a move-only commit quickly (no line tagged
> > with that commit) (the fastest method to me).
> > > > - Then just only needed to quickly check the positions of the moved code
> > chunk by myself (easy).
> > > >
> > > > Thank you, Paul for sharing this. It's very useful to me.
> > >
> > > Looks good to me as well and thank you both for sharing the tips.
> > 
> > Here is one way to script this, where "SHA" identifies the commit to be
> > checked and PATHS the affected pathnames:
> > 
> > 	git checkout SHA^
> > 	git show SHA | git apply -
> > 	git blame -M PATHS | grep '^0* '
> 
> Cool ~. Thank you, Paul.  
> I took them and made them into a script below for future use ;-)

Nice!!!

> #!/bin/bash
> 
> SHA=$1
> 
> if [ -z "$SHA" ]; then
>     echo "Usage: $0 <commit-id>"
>     exit 1
> fi
> 
> if ! git cat-file -t "$SHA" &> /dev/null; then
>         echo "$SHA does not exist in the repository"
>         exit 1
> fi

You might want to record the current position so that you can return
to it automatically.  One approach is to parse the output of
"git status".

> git checkout ${SHA}^ &> /dev/null
> git show ${SHA} | git apply - &> /dev/null
> 
> PATHS=`git status| grep "modified:" | cut -d: -f2 | xargs`

The '--porcelain' argument makes 'git status' is a bit easier to parse
robustly.

> for P in ${PATHS}; do
>         R=`git blame -M $P | grep '^0* '`

You can avoid any bash-variable length limitations by using
'grep -q' and capturing the exit status using "$?".

							Thanx, Paul

>         if test -n "$R"; then
>                 echo "$SHA is NOT a move-only commit"
>                 exit 1
>         fi
> done
> 
> echo "$SHA is a move-only commit"
> 
> > If there is no output, there were no non-move changes.
> > 
> > Or just do the "git blame -M PATHS | grep '^0* '" before doing the checking.
> > 
> > And yes, you can derive PATHS using "git status" if you want.  ;-)
> > 							Thanx, Paul

RE: [PATCH rcu 3/6] rcu/rcuscale: Move rcu_scale_*() after kfree_scale_cleanup()

[Zhuo, Qiuxu]

> From: Paul E. McKenney <paulmck@kernel.org>
> ...
> > > Here is one way to script this, where "SHA" identifies the commit to
> > > be checked and PATHS the affected pathnames:
> > >
> > > 	git checkout SHA^
> > > 	git show SHA | git apply -
> > > 	git blame -M PATHS | grep '^0* '
> >
> > Cool ~. Thank you, Paul.
> > I took them and made them into a script below for future use ;-)
> 
> Nice!!!
> 
> > #!/bin/bash
> >
> > SHA=$1
> >
> > if [ -z "$SHA" ]; then
> >     echo "Usage: $0 <commit-id>"
> >     exit 1
> > fi
> >
> > if ! git cat-file -t "$SHA" &> /dev/null; then
> >         echo "$SHA does not exist in the repository"
> >         exit 1
> > fi
> 
> You might want to record the current position so that you can return to it
> automatically.  One approach is to parse the output of "git status".
> 
> > git checkout ${SHA}^ &> /dev/null
> > git show ${SHA} | git apply - &> /dev/null
> >
> > PATHS=`git status| grep "modified:" | cut -d: -f2 | xargs`
> 
> The '--porcelain' argument makes 'git status' is a bit easier to parse robustly.
> 
> > for P in ${PATHS}; do
> >         R=`git blame -M $P | grep '^0* '`
> 
> You can avoid any bash-variable length limitations by using 'grep -q' and
> capturing the exit status using "$?".
> 

Thank you, Paul, for all the enhancement suggestions. ;-)

> 							Thanx, Paul
> 
> ...

Re: [PATCH rcu 5/6] doc/rcutorture: Add description of rcutorture.stall_cpu_block

[Paul E. McKenney]

On Thu, May 11, 2023 at 10:00:18PM -0700, Joel Fernandes wrote:
> On Thu, May 11, 2023 at 11:11 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Wed, May 10, 2023 at 10:47:36PM -0700, Joel Fernandes wrote:
> > > On Wed, May 10, 2023 at 10:12 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > >
> > > > From: Zqiang <qiang1.zhang@intel.com>
> > > >
> > > > If you build a kernel with CONFIG_PREEMPTION=n and CONFIG_PREEMPT_COUNT=y,
> > > > then run the rcutorture tests specifying stalls as follows:
> > > >
> > > > runqemu kvm slirp nographic qemuparams="-m 1024 -smp 4" \
> > > >         bootparams="console=ttyS0 rcutorture.stall_cpu=30 \
> > > >         rcutorture.stall_no_softlockup=1 rcutorture.stall_cpu_block=1" -d
> > > >
> > > > The tests will produce the following splat:
> > > >
> > > > [   10.841071] rcu-torture: rcu_torture_stall begin CPU stall
> > > > [   10.841073] rcu_torture_stall start on CPU 3.
> > > > [   10.841077] BUG: scheduling while atomic: rcu_torture_sta/66/0x0000000
> > > > ....
> > > > [   10.841108] Call Trace:
> > > > [   10.841110]  <TASK>
> > > > [   10.841112]  dump_stack_lvl+0x64/0xb0
> > > > [   10.841118]  dump_stack+0x10/0x20
> > > > [   10.841121]  __schedule_bug+0x8b/0xb0
> > > > [   10.841126]  __schedule+0x2172/0x2940
> > > > [   10.841157]  schedule+0x9b/0x150
> > > > [   10.841160]  schedule_timeout+0x2e8/0x4f0
> > > > [   10.841192]  schedule_timeout_uninterruptible+0x47/0x50
> > > > [   10.841195]  rcu_torture_stall+0x2e8/0x300
> > > > [   10.841199]  kthread+0x175/0x1a0
> > > > [   10.841206]  ret_from_fork+0x2c/0x50
> > >
> > > Another way to get rid of the warning would be to replace the
> > > cur_ops->readlock() with rcu_read_lock(). Though perhaps that will not
> > > test whether the particular RCU flavor under testing is capable of
> > > causing a stall :-).
> >
> > Exactly!
> >
> > > >         rcutorture.stall_cpu_block= [KNL]
> > > >                         Sleep while stalling if set.  This will result
> > > > -                       in warnings from preemptible RCU in addition
> > > > -                       to any other stall-related activity.
> > > > +                       in warnings from preemptible RCU in addition to
> > > > +                       any other stall-related activity.  Note that
> > > > +                       in kernels built with CONFIG_PREEMPTION=n and
> > > > +                       CONFIG_PREEMPT_COUNT=y, this parameter will
> > > > +                       cause the CPU to pass through a quiescent state.
> > > > +                       Any such quiescent states will suppress RCU CPU
> > > > +                       stall warnings, but the time-based sleep will
> > > > +                       also result in scheduling-while-atomic splats.
> > >
> > > Could change last part to "but may also result in
> > > scheduling-while-atomic splats as preemption might be disabled for
> > > certain RCU flavors in order to cause the stall".
> >
> > Is that needed given the earlier "in kernels built with
> > CONFIG_PREEMPTION=n and CONFIG_PREEMPT_COUNT=y"?
> 
> Hmm, I guess is not clear to the reader without code reading about why
> preempt got disabled. So I would add that last part I mentioned, but I
> am Ok either way, it is just a suggestion.

I will figure something out to more tightly tie this to the previous
CONFIG_PREEMPTION=n.

> > > > +                       Which might or might not be what you want.
> > > > +
> > >
> > > Suggest drop this line ;-).
> >
> > OK, I will bite.  ;-)
> >
> > What is your concern with this line?
> 
> It is not needed IMO.

It actually is, otherwise the various testing services complain about
getting splats.  I will upgrade it to something more explicit.

							Thanx, Paul

> thanks,
> 
>  - Joel
> 
> 
> > > >         rcutorture.stall_cpu_holdoff= [KNL]
> > > >                         Time to wait (s) after boot before inducing stall.
> > > > --
> > > > 2.40.1
> > > >