[PATCH tip/core/rcu 0/5] rcutorture updates for 3.12

[PATCH tip/core/rcu 0/5] rcutorture updates for 3.12

[Paul E. McKenney]

Hello!

This series provides some rcutorture updates:

1.	Add a debug_object option to rcutorture to allow testing
	of RCU's debug-objects facilities.

2.	Increase rcutorture test coverage by concurrently testing
	synchronous, asynchronous, and expedited grace periods.

3.	Sort the extensive list of rcutorture module parameters.

4.	Remove the unused "oldbatch" variable.

5.	Emit CPU-online failure message to the console.

							Thanx, Paul


 b/Documentation/RCU/torture.txt |   10 +
 b/kernel/rcutorture.c           |  341 ++++++++++++++++------------------------
 2 files changed, 151 insertions(+), 200 deletions(-)

[PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

This commit adds a object_debug option to rcutorture to allow the
debug-object-based checks for duplicate call_rcu() invocations to
be deterministically tested.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 kernel/rcutorture.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 3d936f0f..efb1c74 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
 static int fqs_holdoff;		/* Hold time within burst (us). */
 static int fqs_stutter = 3;	/* Wait time between bursts (s). */
 static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
+static int object_debug;	/* Test object-debug double call_rcu()?. */
 static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
 static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
 static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */
@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
 module_param(onoff_interval, int, 0444);
 MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
 module_param(onoff_holdoff, int, 0444);
@@ -1934,6 +1937,18 @@ rcu_torture_cleanup(void)
 		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
 }
 
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+static void rcu_torture_leak_cb(struct rcu_head *rhp)
+{
+}
+
+static void rcu_torture_err_cb(struct rcu_head *rhp)
+{
+	/* This -might- happen due to race conditions, but is unlikely. */
+	pr_alert("rcutorture: duplicated callback was invoked.\n");
+}
+#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+
 static int __init
 rcu_torture_init(void)
 {
@@ -2163,6 +2178,28 @@ rcu_torture_init(void)
 		firsterr = retval;
 		goto unwind;
 	}
+	if (object_debug) {
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+		struct rcu_head rh1;
+		struct rcu_head rh2;
+
+		init_rcu_head_on_stack(&rh1);
+		init_rcu_head_on_stack(&rh2);
+		pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
+		local_irq_disable(); /* Make it hard to finish grace period. */
+		call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
+		call_rcu(&rh2, rcu_torture_err_cb);
+		call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
+		local_irq_enable();
+		rcu_barrier();
+		pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
+		destroy_rcu_head_on_stack(&rh1);
+		destroy_rcu_head_on_stack(&rh2);
+#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+		pr_alert("rcutorture: !%s, not testing duplicate call_rcu()\n",
+			 "CONFIG_DEBUG_OBJECTS_RCU_HEAD");
+#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+	}
 	rcutorture_record_test_transition();
 	mutex_unlock(&fullstop_mutex);
 	return 0;
-- 
1.8.1.5

[PATCH tip/core/rcu 2/5] rcu: Increase rcutorture test coverage

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

Currently, rcutorture has separate torture_types to test synchronous,
asynchronous, and expedited grace-period primitives.  This has
two disadvantages: (1) Three times the number of runs to cover the
combinations and (2) Little testing of concurrent combinations of the
three options.  This commit therefore adds a pair of module parameters
that control normal and expedited state, with the default being both
types, randomly selected, by the fakewriter processes, thus reducing
source-code size and increasing test coverage.  In addtion, the writer
task switches between asynchronous-normal and expedited grace-period
primitives driven by the same pair of module parameters.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 Documentation/RCU/torture.txt |  10 ++
 kernel/rcutorture.c           | 226 ++++++++++++------------------------------
 2 files changed, 73 insertions(+), 163 deletions(-)

diff --git a/Documentation/RCU/torture.txt b/Documentation/RCU/torture.txt
index d8a5023..dac02a6 100644
--- a/Documentation/RCU/torture.txt
+++ b/Documentation/RCU/torture.txt
@@ -42,6 +42,16 @@ fqs_holdoff	Holdoff time (in microseconds) between consecutive calls
 fqs_stutter	Wait time (in seconds) between consecutive bursts
 		of calls to force_quiescent_state().
 
+gp_normal	Make the fake writers use normal synchronous grace-period
+		primitives.
+
+gp_exp		Make the fake writers use expedited synchronous grace-period
+		primitives.  If both gp_normal and gp_exp are set, or
+		if neither gp_normal nor gp_exp are set, then randomly
+		choose the primitive so that about 50% are normal and
+		50% expedited.  By default, neither are set, which
+		gives best overall test coverage.
+
 irqreader	Says to invoke RCU readers from irq level.  This is currently
 		done via timers.  Defaults to "1" for variants of RCU that
 		permit this.  (Or, more accurately, variants of RCU that do
diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index efb1c74..be38d4e 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -65,6 +65,8 @@ static int irqreader = 1;	/* RCU readers from irq (timers). */
 static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
 static int fqs_holdoff;		/* Hold time within burst (us). */
 static int fqs_stutter = 3;	/* Wait time between bursts (s). */
+static bool gp_exp;		/* Use expedited GP wait primitives. */
+static bool gp_normal;		/* Use normal GP wait primitives. */
 static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
 static int object_debug;	/* Test object-debug double call_rcu()?. */
 static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
@@ -99,6 +101,10 @@ module_param(fqs_holdoff, int, 0444);
 MODULE_PARM_DESC(fqs_holdoff, "Holdoff time within fqs bursts (us)");
 module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
+module_param(gp_normal, bool, 0444);
+MODULE_PARM_DESC(gp_normal, "Use normal (non-expedited) GP wait primitives");
+module_param(gp_exp, bool, 0444);
+MODULE_PARM_DESC(gp_exp, "Use expedited GP wait primitives");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
 module_param(object_debug, int, 0444);
@@ -363,6 +369,7 @@ struct rcu_torture_ops {
 	int (*completed)(void);
 	void (*deferred_free)(struct rcu_torture *p);
 	void (*sync)(void);
+	void (*exp_sync)(void);
 	void (*call)(struct rcu_head *head, void (*func)(struct rcu_head *rcu));
 	void (*cb_barrier)(void);
 	void (*fqs)(void);
@@ -446,81 +453,27 @@ static void rcu_torture_deferred_free(struct rcu_torture *p)
 	call_rcu(&p->rtort_rcu, rcu_torture_cb);
 }
 
-static struct rcu_torture_ops rcu_ops = {
-	.init		= NULL,
-	.readlock	= rcu_torture_read_lock,
-	.read_delay	= rcu_read_delay,
-	.readunlock	= rcu_torture_read_unlock,
-	.completed	= rcu_torture_completed,
-	.deferred_free	= rcu_torture_deferred_free,
-	.sync		= synchronize_rcu,
-	.call		= call_rcu,
-	.cb_barrier	= rcu_barrier,
-	.fqs		= rcu_force_quiescent_state,
-	.stats		= NULL,
-	.irq_capable	= 1,
-	.can_boost	= rcu_can_boost(),
-	.name		= "rcu"
-};
-
-static void rcu_sync_torture_deferred_free(struct rcu_torture *p)
-{
-	int i;
-	struct rcu_torture *rp;
-	struct rcu_torture *rp1;
-
-	cur_ops->sync();
-	list_add(&p->rtort_free, &rcu_torture_removed);
-	list_for_each_entry_safe(rp, rp1, &rcu_torture_removed, rtort_free) {
-		i = rp->rtort_pipe_count;
-		if (i > RCU_TORTURE_PIPE_LEN)
-			i = RCU_TORTURE_PIPE_LEN;
-		atomic_inc(&rcu_torture_wcount[i]);
-		if (++rp->rtort_pipe_count >= RCU_TORTURE_PIPE_LEN) {
-			rp->rtort_mbtest = 0;
-			list_del(&rp->rtort_free);
-			rcu_torture_free(rp);
-		}
-	}
-}
-
 static void rcu_sync_torture_init(void)
 {
 	INIT_LIST_HEAD(&rcu_torture_removed);
 }
 
-static struct rcu_torture_ops rcu_sync_ops = {
+static struct rcu_torture_ops rcu_ops = {
 	.init		= rcu_sync_torture_init,
 	.readlock	= rcu_torture_read_lock,
 	.read_delay	= rcu_read_delay,
 	.readunlock	= rcu_torture_read_unlock,
 	.completed	= rcu_torture_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
+	.deferred_free	= rcu_torture_deferred_free,
 	.sync		= synchronize_rcu,
-	.call		= NULL,
-	.cb_barrier	= NULL,
-	.fqs		= rcu_force_quiescent_state,
-	.stats		= NULL,
-	.irq_capable	= 1,
-	.can_boost	= rcu_can_boost(),
-	.name		= "rcu_sync"
-};
-
-static struct rcu_torture_ops rcu_expedited_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= rcu_torture_read_lock,
-	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
-	.readunlock	= rcu_torture_read_unlock,
-	.completed	= rcu_no_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= synchronize_rcu_expedited,
-	.call		= NULL,
-	.cb_barrier	= NULL,
+	.exp_sync	= synchronize_rcu_expedited,
+	.call		= call_rcu,
+	.cb_barrier	= rcu_barrier,
 	.fqs		= rcu_force_quiescent_state,
 	.stats		= NULL,
 	.irq_capable	= 1,
 	.can_boost	= rcu_can_boost(),
-	.name		= "rcu_expedited"
+	.name		= "rcu"
 };
 
 /*
@@ -549,13 +502,14 @@ static void rcu_bh_torture_deferred_free(struct rcu_torture *p)
 }
 
 static struct rcu_torture_ops rcu_bh_ops = {
-	.init		= NULL,
+	.init		= rcu_sync_torture_init,
 	.readlock	= rcu_bh_torture_read_lock,
 	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
 	.readunlock	= rcu_bh_torture_read_unlock,
 	.completed	= rcu_bh_torture_completed,
 	.deferred_free	= rcu_bh_torture_deferred_free,
 	.sync		= synchronize_rcu_bh,
+	.exp_sync	= synchronize_rcu_bh_expedited,
 	.call		= call_rcu_bh,
 	.cb_barrier	= rcu_barrier_bh,
 	.fqs		= rcu_bh_force_quiescent_state,
@@ -564,38 +518,6 @@ static struct rcu_torture_ops rcu_bh_ops = {
 	.name		= "rcu_bh"
 };
 
-static struct rcu_torture_ops rcu_bh_sync_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= rcu_bh_torture_read_lock,
-	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
-	.readunlock	= rcu_bh_torture_read_unlock,
-	.completed	= rcu_bh_torture_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= synchronize_rcu_bh,
-	.call		= NULL,
-	.cb_barrier	= NULL,
-	.fqs		= rcu_bh_force_quiescent_state,
-	.stats		= NULL,
-	.irq_capable	= 1,
-	.name		= "rcu_bh_sync"
-};
-
-static struct rcu_torture_ops rcu_bh_expedited_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= rcu_bh_torture_read_lock,
-	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
-	.readunlock	= rcu_bh_torture_read_unlock,
-	.completed	= rcu_bh_torture_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= synchronize_rcu_bh_expedited,
-	.call		= NULL,
-	.cb_barrier	= NULL,
-	.fqs		= rcu_bh_force_quiescent_state,
-	.stats		= NULL,
-	.irq_capable	= 1,
-	.name		= "rcu_bh_expedited"
-};
-
 /*
  * Definitions for srcu torture testing.
  */
@@ -670,6 +592,11 @@ static int srcu_torture_stats(char *page)
 	return cnt;
 }
 
+static void srcu_torture_synchronize_expedited(void)
+{
+	synchronize_srcu_expedited(&srcu_ctl);
+}
+
 static struct rcu_torture_ops srcu_ops = {
 	.init		= rcu_sync_torture_init,
 	.readlock	= srcu_torture_read_lock,
@@ -678,45 +605,13 @@ static struct rcu_torture_ops srcu_ops = {
 	.completed	= srcu_torture_completed,
 	.deferred_free	= srcu_torture_deferred_free,
 	.sync		= srcu_torture_synchronize,
+	.exp_sync	= srcu_torture_synchronize_expedited,
 	.call		= srcu_torture_call,
 	.cb_barrier	= srcu_torture_barrier,
 	.stats		= srcu_torture_stats,
 	.name		= "srcu"
 };
 
-static struct rcu_torture_ops srcu_sync_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= srcu_torture_read_lock,
-	.read_delay	= srcu_read_delay,
-	.readunlock	= srcu_torture_read_unlock,
-	.completed	= srcu_torture_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= srcu_torture_synchronize,
-	.call		= NULL,
-	.cb_barrier	= NULL,
-	.stats		= srcu_torture_stats,
-	.name		= "srcu_sync"
-};
-
-static void srcu_torture_synchronize_expedited(void)
-{
-	synchronize_srcu_expedited(&srcu_ctl);
-}
-
-static struct rcu_torture_ops srcu_expedited_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= srcu_torture_read_lock,
-	.read_delay	= srcu_read_delay,
-	.readunlock	= srcu_torture_read_unlock,
-	.completed	= srcu_torture_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= srcu_torture_synchronize_expedited,
-	.call		= NULL,
-	.cb_barrier	= NULL,
-	.stats		= srcu_torture_stats,
-	.name		= "srcu_expedited"
-};
-
 /*
  * Definitions for sched torture testing.
  */
@@ -745,6 +640,8 @@ static struct rcu_torture_ops sched_ops = {
 	.completed	= rcu_no_completed,
 	.deferred_free	= rcu_sched_torture_deferred_free,
 	.sync		= synchronize_sched,
+	.exp_sync	= synchronize_sched_expedited,
+	.call		= call_rcu_sched,
 	.cb_barrier	= rcu_barrier_sched,
 	.fqs		= rcu_sched_force_quiescent_state,
 	.stats		= NULL,
@@ -752,35 +649,6 @@ static struct rcu_torture_ops sched_ops = {
 	.name		= "sched"
 };
 
-static struct rcu_torture_ops sched_sync_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= sched_torture_read_lock,
-	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
-	.readunlock	= sched_torture_read_unlock,
-	.completed	= rcu_no_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= synchronize_sched,
-	.cb_barrier	= NULL,
-	.fqs		= rcu_sched_force_quiescent_state,
-	.stats		= NULL,
-	.name		= "sched_sync"
-};
-
-static struct rcu_torture_ops sched_expedited_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= sched_torture_read_lock,
-	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
-	.readunlock	= sched_torture_read_unlock,
-	.completed	= rcu_no_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= synchronize_sched_expedited,
-	.cb_barrier	= NULL,
-	.fqs		= rcu_sched_force_quiescent_state,
-	.stats		= NULL,
-	.irq_capable	= 1,
-	.name		= "sched_expedited"
-};
-
 /*
  * RCU torture priority-boost testing.  Runs one real-time thread per
  * CPU for moderate bursts, repeatedly registering RCU callbacks and
@@ -930,9 +798,11 @@ rcu_torture_fqs(void *arg)
 static int
 rcu_torture_writer(void *arg)
 {
+	bool exp;
 	int i;
 	long oldbatch = rcu_batches_completed();
 	struct rcu_torture *rp;
+	struct rcu_torture *rp1;
 	struct rcu_torture *old_rp;
 	static DEFINE_RCU_RANDOM(rand);
 
@@ -957,7 +827,31 @@ rcu_torture_writer(void *arg)
 				i = RCU_TORTURE_PIPE_LEN;
 			atomic_inc(&rcu_torture_wcount[i]);
 			old_rp->rtort_pipe_count++;
-			cur_ops->deferred_free(old_rp);
+			if (gp_normal == gp_exp)
+				exp = !!(rcu_random(&rand) & 0x80);
+			else
+				exp = gp_exp;
+			if (!exp) {
+				cur_ops->deferred_free(old_rp);
+			} else {
+				cur_ops->exp_sync();
+				list_add(&old_rp->rtort_free,
+					 &rcu_torture_removed);
+				list_for_each_entry_safe(rp, rp1,
+							 &rcu_torture_removed,
+							 rtort_free) {
+					i = rp->rtort_pipe_count;
+					if (i > RCU_TORTURE_PIPE_LEN)
+						i = RCU_TORTURE_PIPE_LEN;
+					atomic_inc(&rcu_torture_wcount[i]);
+					if (++rp->rtort_pipe_count >=
+					    RCU_TORTURE_PIPE_LEN) {
+						rp->rtort_mbtest = 0;
+						list_del(&rp->rtort_free);
+						rcu_torture_free(rp);
+					}
+				 }
+			}
 		}
 		rcutorture_record_progress(++rcu_torture_current_version);
 		oldbatch = cur_ops->completed();
@@ -986,10 +880,18 @@ rcu_torture_fakewriter(void *arg)
 		schedule_timeout_uninterruptible(1 + rcu_random(&rand)%10);
 		udelay(rcu_random(&rand) & 0x3ff);
 		if (cur_ops->cb_barrier != NULL &&
-		    rcu_random(&rand) % (nfakewriters * 8) == 0)
+		    rcu_random(&rand) % (nfakewriters * 8) == 0) {
 			cur_ops->cb_barrier();
-		else
+		} else if (gp_normal == gp_exp) {
+			if (rcu_random(&rand) & 0x80)
+				cur_ops->sync();
+			else
+				cur_ops->exp_sync();
+		} else if (gp_normal) {
 			cur_ops->sync();
+		} else {
+			cur_ops->exp_sync();
+		}
 		rcu_stutter_wait("rcu_torture_fakewriter");
 	} while (!kthread_should_stop() && fullstop == FULLSTOP_DONTSTOP);
 
@@ -1956,11 +1858,9 @@ rcu_torture_init(void)
 	int cpu;
 	int firsterr = 0;
 	int retval;
-	static struct rcu_torture_ops *torture_ops[] =
-		{ &rcu_ops, &rcu_sync_ops, &rcu_expedited_ops,
-		  &rcu_bh_ops, &rcu_bh_sync_ops, &rcu_bh_expedited_ops,
-		  &srcu_ops, &srcu_sync_ops, &srcu_expedited_ops,
-		  &sched_ops, &sched_sync_ops, &sched_expedited_ops, };
+	static struct rcu_torture_ops *torture_ops[] = {
+		&rcu_ops, &rcu_bh_ops, &srcu_ops, &sched_ops,
+	};
 
 	mutex_lock(&fullstop_mutex);
 
-- 
1.8.1.5

[PATCH tip/core/rcu 3/5] rcu: Sort rcutorture module parameters

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

There are getting to be too many module parameters to permit the current
semi-random order, so this patch orders them.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutorture.c | 68 ++++++++++++++++++++++++++---------------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index be38d4e..be50ebe 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -52,81 +52,81 @@
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Paul E. McKenney <paulmck@us.ibm.com> and Josh Triplett <josh@freedesktop.org>");
 
-static int nreaders = -1;	/* # reader threads, defaults to 2*ncpus */
-static int nfakewriters = 4;	/* # fake writer threads */
-static int stat_interval = 60;	/* Interval between stats, in seconds. */
-				/*  Zero means "only at end of test". */
-static bool verbose;		/* Print more debug info. */
-static bool test_no_idle_hz = true;
-				/* Test RCU support for tickless idle CPUs. */
-static int shuffle_interval = 3; /* Interval between shuffles (in sec)*/
-static int stutter = 5;		/* Start/stop testing interval (in sec) */
-static int irqreader = 1;	/* RCU readers from irq (timers). */
 static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
 static int fqs_holdoff;		/* Hold time within burst (us). */
 static int fqs_stutter = 3;	/* Wait time between bursts (s). */
 static bool gp_exp;		/* Use expedited GP wait primitives. */
 static bool gp_normal;		/* Use normal GP wait primitives. */
+static int irqreader = 1;	/* RCU readers from irq (timers). */
 static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
+static int nfakewriters = 4;	/* # fake writer threads */
+static int nreaders = -1;	/* # reader threads, defaults to 2*ncpus */
 static int object_debug;	/* Test object-debug double call_rcu()?. */
-static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
 static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
+static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
+static int shuffle_interval = 3; /* Interval between shuffles (in sec)*/
 static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */
 static int stall_cpu;		/* CPU-stall duration (s).  0 for no stall. */
 static int stall_cpu_holdoff = 10; /* Time to wait until stall (s).  */
+static int stat_interval = 60;	/* Interval between stats, in seconds. */
+				/*  Zero means "only at end of test". */
+static int stutter = 5;		/* Start/stop testing interval (in sec) */
 static int test_boost = 1;	/* Test RCU prio boost: 0=no, 1=maybe, 2=yes. */
-static int test_boost_interval = 7; /* Interval between boost tests, seconds. */
 static int test_boost_duration = 4; /* Duration of each boost test, seconds. */
+static int test_boost_interval = 7; /* Interval between boost tests, seconds. */
+static bool test_no_idle_hz = true;
+				/* Test RCU support for tickless idle CPUs. */
 static char *torture_type = "rcu"; /* What RCU implementation to torture. */
+static bool verbose;		/* Print more debug info. */
 
-module_param(nreaders, int, 0444);
-MODULE_PARM_DESC(nreaders, "Number of RCU reader threads");
-module_param(nfakewriters, int, 0444);
-MODULE_PARM_DESC(nfakewriters, "Number of RCU fake writer threads");
-module_param(stat_interval, int, 0644);
-MODULE_PARM_DESC(stat_interval, "Number of seconds between stats printk()s");
-module_param(verbose, bool, 0444);
-MODULE_PARM_DESC(verbose, "Enable verbose debugging printk()s");
-module_param(test_no_idle_hz, bool, 0444);
-MODULE_PARM_DESC(test_no_idle_hz, "Test support for tickless idle CPUs");
-module_param(shuffle_interval, int, 0444);
-MODULE_PARM_DESC(shuffle_interval, "Number of seconds between shuffles");
-module_param(stutter, int, 0444);
-MODULE_PARM_DESC(stutter, "Number of seconds to run/halt test");
-module_param(irqreader, int, 0444);
-MODULE_PARM_DESC(irqreader, "Allow RCU readers from irq handlers");
 module_param(fqs_duration, int, 0444);
 MODULE_PARM_DESC(fqs_duration, "Duration of fqs bursts (us)");
 module_param(fqs_holdoff, int, 0444);
 MODULE_PARM_DESC(fqs_holdoff, "Holdoff time within fqs bursts (us)");
 module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
-module_param(gp_normal, bool, 0444);
-MODULE_PARM_DESC(gp_normal, "Use normal (non-expedited) GP wait primitives");
 module_param(gp_exp, bool, 0444);
 MODULE_PARM_DESC(gp_exp, "Use expedited GP wait primitives");
+module_param(gp_normal, bool, 0444);
+MODULE_PARM_DESC(gp_normal, "Use normal (non-expedited) GP wait primitives");
+module_param(irqreader, int, 0444);
+MODULE_PARM_DESC(irqreader, "Allow RCU readers from irq handlers");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(nfakewriters, int, 0444);
+MODULE_PARM_DESC(nfakewriters, "Number of RCU fake writer threads");
+module_param(nreaders, int, 0444);
+MODULE_PARM_DESC(nreaders, "Number of RCU reader threads");
 module_param(object_debug, int, 0444);
 MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
-module_param(onoff_interval, int, 0444);
-MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
 module_param(onoff_holdoff, int, 0444);
 MODULE_PARM_DESC(onoff_holdoff, "Time after boot before CPU hotplugs (s)");
+module_param(onoff_interval, int, 0444);
+MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
+module_param(shuffle_interval, int, 0444);
+MODULE_PARM_DESC(shuffle_interval, "Number of seconds between shuffles");
 module_param(shutdown_secs, int, 0444);
 MODULE_PARM_DESC(shutdown_secs, "Shutdown time (s), zero to disable.");
 module_param(stall_cpu, int, 0444);
 MODULE_PARM_DESC(stall_cpu, "Stall duration (s), zero to disable.");
 module_param(stall_cpu_holdoff, int, 0444);
 MODULE_PARM_DESC(stall_cpu_holdoff, "Time to wait before starting stall (s).");
+module_param(stat_interval, int, 0644);
+MODULE_PARM_DESC(stat_interval, "Number of seconds between stats printk()s");
+module_param(stutter, int, 0444);
+MODULE_PARM_DESC(stutter, "Number of seconds to run/halt test");
 module_param(test_boost, int, 0444);
 MODULE_PARM_DESC(test_boost, "Test RCU prio boost: 0=no, 1=maybe, 2=yes.");
-module_param(test_boost_interval, int, 0444);
-MODULE_PARM_DESC(test_boost_interval, "Interval between boost tests, seconds.");
 module_param(test_boost_duration, int, 0444);
 MODULE_PARM_DESC(test_boost_duration, "Duration of each boost test, seconds.");
+module_param(test_boost_interval, int, 0444);
+MODULE_PARM_DESC(test_boost_interval, "Interval between boost tests, seconds.");
+module_param(test_no_idle_hz, bool, 0444);
+MODULE_PARM_DESC(test_no_idle_hz, "Test support for tickless idle CPUs");
 module_param(torture_type, charp, 0444);
 MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, rcu_bh, srcu)");
+module_param(verbose, bool, 0444);
+MODULE_PARM_DESC(verbose, "Enable verbose debugging printk()s");
 
 #define TORTURE_FLAG "-torture:"
 #define PRINTK_STRING(s) \
-- 
1.8.1.5

Re: [PATCH tip/core/rcu 3/5] rcu: Sort rcutorture module parameters

[Josh Triplett]

On Sat, Aug 17, 2013 at 07:25:15PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> There are getting to be too many module parameters to permit the current
> semi-random order, so this patch orders them.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

As long as you're reordering them anyway, how about grouping each of the
variables together with the corresponding module parameter macros, and
then dropping the comments that duplicate the module parameter
documentation?

With that change:
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

Re: [PATCH tip/core/rcu 3/5] rcu: Sort rcutorture module parameters

[Paul E. McKenney]

On Sat, Aug 17, 2013 at 07:57:40PM -0700, Josh Triplett wrote:
> On Sat, Aug 17, 2013 at 07:25:15PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > There are getting to be too many module parameters to permit the current
> > semi-random order, so this patch orders them.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> As long as you're reordering them anyway, how about grouping each of the
> variables together with the corresponding module parameter macros, and
> then dropping the comments that duplicate the module parameter
> documentation?
> 
> With that change:
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>

Good point, done!

							Thanx, Paul

[PATCH tip/core/rcu 4/5] rcu: Remove unused variable from rcu_torture_writer()

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

The oldbatch variable in rcu_torture_writer() is stored to, but never
loaded from.  This commit therefore removes it.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutorture.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index be50ebe..7d42d13 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -800,7 +800,6 @@ rcu_torture_writer(void *arg)
 {
 	bool exp;
 	int i;
-	long oldbatch = rcu_batches_completed();
 	struct rcu_torture *rp;
 	struct rcu_torture *rp1;
 	struct rcu_torture *old_rp;
@@ -854,7 +853,6 @@ rcu_torture_writer(void *arg)
 			}
 		}
 		rcutorture_record_progress(++rcu_torture_current_version);
-		oldbatch = cur_ops->completed();
 		rcu_stutter_wait("rcu_torture_writer");
 	} while (!kthread_should_stop() && fullstop == FULLSTOP_DONTSTOP);
 	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
-- 
1.8.1.5

[PATCH tip/core/rcu 5/5] rcu: Make rcutorture emit online failures if verbose

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

Although rcutorture counts CPU-hotplug online failures, it does
not explicitly record which CPUs were having trouble coming online.
This commit therefore emits a console message when online failure occurs.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutorture.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 7d42d13..15bec39 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -1437,7 +1437,13 @@ rcu_torture_onoff(void *arg)
 					 torture_type, cpu);
 			starttime = jiffies;
 			n_online_attempts++;
-			if (cpu_up(cpu) == 0) {
+			ret = cpu_up(cpu);
+			if (ret != 0) {
+				if (verbose)
+					pr_alert("%s" TORTURE_FLAG
+						 "rcu_torture_onoff task: online %d failed: errno %d\n",
+						 torture_type, cpu, ret);
+			} else {
 				if (verbose)
 					pr_alert("%s" TORTURE_FLAG
 						 "rcu_torture_onoff task: onlined %d\n",
-- 
1.8.1.5

Re: [PATCH tip/core/rcu 5/5] rcu: Make rcutorture emit online failures if verbose

[Josh Triplett]

On Sat, Aug 17, 2013 at 07:25:17PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> Although rcutorture counts CPU-hotplug online failures, it does
> not explicitly record which CPUs were having trouble coming online.
> This commit therefore emits a console message when online failure occurs.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

One completely optional note below; with or without that change,
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

>  kernel/rcutorture.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> index 7d42d13..15bec39 100644
> --- a/kernel/rcutorture.c
> +++ b/kernel/rcutorture.c
> @@ -1437,7 +1437,13 @@ rcu_torture_onoff(void *arg)
>  					 torture_type, cpu);
>  			starttime = jiffies;
>  			n_online_attempts++;
> -			if (cpu_up(cpu) == 0) {
> +			ret = cpu_up(cpu);
> +			if (ret != 0) {

Or just "if (ret) {"

- Josh Triplett

Re: [PATCH tip/core/rcu 5/5] rcu: Make rcutorture emit online failures if verbose

[Paul E. McKenney]

On Sat, Aug 17, 2013 at 07:59:05PM -0700, Josh Triplett wrote:
> On Sat, Aug 17, 2013 at 07:25:17PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > Although rcutorture counts CPU-hotplug online failures, it does
> > not explicitly record which CPUs were having trouble coming online.
> > This commit therefore emits a console message when online failure occurs.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> One completely optional note below; with or without that change,
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> 
> >  kernel/rcutorture.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> > index 7d42d13..15bec39 100644
> > --- a/kernel/rcutorture.c
> > +++ b/kernel/rcutorture.c
> > @@ -1437,7 +1437,13 @@ rcu_torture_onoff(void *arg)
> >  					 torture_type, cpu);
> >  			starttime = jiffies;
> >  			n_online_attempts++;
> > -			if (cpu_up(cpu) == 0) {
> > +			ret = cpu_up(cpu);
> > +			if (ret != 0) {
> 
> Or just "if (ret) {"

Makes sense, fixed!

							Thanx, Paul

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Josh Triplett]

On Sat, Aug 17, 2013 at 07:25:13PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> This commit adds a object_debug option to rcutorture to allow the
> debug-object-based checks for duplicate call_rcu() invocations to
> be deterministically tested.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>

Two comments below; with those fixed,
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

> ---
> @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
>  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
>  module_param(n_barrier_cbs, int, 0444);
>  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
> +module_param(object_debug, int, 0444);
> +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");

modules-next has a change to ignore and warn about
unknown module parameters.  Thus, I'd suggest wrapping the ifdef around
this module parameter, so it doesn't exist at all without
CONFIG_DEBUG_OBJECTS_RCU_HEAD.

Alternatively, consider providing the test unconditionally, and just
printing a big warning message saying that it's going to cause
corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.

> @@ -2163,6 +2178,28 @@ rcu_torture_init(void)
>  		firsterr = retval;
>  		goto unwind;
>  	}
> +	if (object_debug) {
> +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> +		struct rcu_head rh1;
> +		struct rcu_head rh2;
> +
> +		init_rcu_head_on_stack(&rh1);
> +		init_rcu_head_on_stack(&rh2);
> +		pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> +		local_irq_disable(); /* Make it hard to finish grace period. */
> +		call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
> +		call_rcu(&rh2, rcu_torture_err_cb);
> +		call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
> +		local_irq_enable();
> +		rcu_barrier();
> +		pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> +		destroy_rcu_head_on_stack(&rh1);
> +		destroy_rcu_head_on_stack(&rh2);
> +#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> +		pr_alert("rcutorture: !%s, not testing duplicate call_rcu()\n",
> +			 "CONFIG_DEBUG_OBJECTS_RCU_HEAD");

Why put this parameter in a separate string?  That makes it harder to
grep for the full error message.  (That's assuming you keep the error
message, given the comment above.)

- Josh Triplett

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Paul E. McKenney]

On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
> On Sat, Aug 17, 2013 at 07:25:13PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > This commit adds a object_debug option to rcutorture to allow the
> > debug-object-based checks for duplicate call_rcu() invocations to
> > be deterministically tested.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> > Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
> > Cc: Rik van Riel <riel@surriel.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> 
> Two comments below; with those fixed,
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> 
> > ---
> > @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
> >  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
> >  module_param(n_barrier_cbs, int, 0444);
> >  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
> > +module_param(object_debug, int, 0444);
> > +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
> 
> modules-next has a change to ignore and warn about
> unknown module parameters.  Thus, I'd suggest wrapping the ifdef around
> this module parameter, so it doesn't exist at all without
> CONFIG_DEBUG_OBJECTS_RCU_HEAD.
> 
> Alternatively, consider providing the test unconditionally, and just
> printing a big warning message saying that it's going to cause
> corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.

I currently do something like the above.  The module parameter
is defined unconditionally, but the actual tests are under #ifdef
CONFIG_DEBUG_OBJECTS_RCU_HEAD.  If you specify object_debug for a
!CONFIG_DEBUG_OBJECTS_RCU_HEAD kernel, the pr_alert() below happens,
and the test is omitted, thus avoiding the list corruption.

Seem reasonable?

> > @@ -2163,6 +2178,28 @@ rcu_torture_init(void)
> >  		firsterr = retval;
> >  		goto unwind;
> >  	}
> > +	if (object_debug) {
> > +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> > +		struct rcu_head rh1;
> > +		struct rcu_head rh2;
> > +
> > +		init_rcu_head_on_stack(&rh1);
> > +		init_rcu_head_on_stack(&rh2);
> > +		pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> > +		local_irq_disable(); /* Make it hard to finish grace period. */
> > +		call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
> > +		call_rcu(&rh2, rcu_torture_err_cb);
> > +		call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
> > +		local_irq_enable();
> > +		rcu_barrier();
> > +		pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> > +		destroy_rcu_head_on_stack(&rh1);
> > +		destroy_rcu_head_on_stack(&rh2);
> > +#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > +		pr_alert("rcutorture: !%s, not testing duplicate call_rcu()\n",
> > +			 "CONFIG_DEBUG_OBJECTS_RCU_HEAD");
> 
> Why put this parameter in a separate string?  That makes it harder to
> grep for the full error message.  (That's assuming you keep the error
> message, given the comment above.)

Force of habit, fixed.  ;-)

							Thanx, Paul

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Josh Triplett]

On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
> On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
> > On Sat, Aug 17, 2013 at 07:25:13PM -0700, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > 
> > > This commit adds a object_debug option to rcutorture to allow the
> > > debug-object-based checks for duplicate call_rcu() invocations to
> > > be deterministically tested.
> > > 
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> > > Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
> > > Cc: Rik van Riel <riel@surriel.com>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> > 
> > Two comments below; with those fixed,
> > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > 
> > > ---
> > > @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
> > >  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
> > >  module_param(n_barrier_cbs, int, 0444);
> > >  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
> > > +module_param(object_debug, int, 0444);
> > > +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
> > 
> > modules-next has a change to ignore and warn about
> > unknown module parameters.  Thus, I'd suggest wrapping the ifdef around
> > this module parameter, so it doesn't exist at all without
> > CONFIG_DEBUG_OBJECTS_RCU_HEAD.
> > 
> > Alternatively, consider providing the test unconditionally, and just
> > printing a big warning message saying that it's going to cause
> > corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.
> 
> I currently do something like the above.  The module parameter
> is defined unconditionally, but the actual tests are under #ifdef
> CONFIG_DEBUG_OBJECTS_RCU_HEAD.  If you specify object_debug for a
> !CONFIG_DEBUG_OBJECTS_RCU_HEAD kernel, the pr_alert() below happens,
> and the test is omitted, thus avoiding the list corruption.
> 
> Seem reasonable?

That's exactly the bit I was commenting on.  I'm saying that you should
either make the test unconditional (perhaps with a warning saying it's
about to cause list corruption), or you should compile out the module
parameter as well and then you don't need the pr_alert (since current
kernels will emit a warning when you pass a non-existent module
parameter).

Personally, I'd go with the latter.

- Josh Triplett

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Paul E. McKenney]

On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
> On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
> > On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
> > > On Sat, Aug 17, 2013 at 07:25:13PM -0700, Paul E. McKenney wrote:
> > > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > > 
> > > > This commit adds a object_debug option to rcutorture to allow the
> > > > debug-object-based checks for duplicate call_rcu() invocations to
> > > > be deterministically tested.
> > > > 
> > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > > > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> > > > Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
> > > > Cc: Rik van Riel <riel@surriel.com>
> > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> > > 
> > > Two comments below; with those fixed,
> > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > > 
> > > > ---
> > > > @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
> > > >  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
> > > >  module_param(n_barrier_cbs, int, 0444);
> > > >  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
> > > > +module_param(object_debug, int, 0444);
> > > > +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
> > > 
> > > modules-next has a change to ignore and warn about
> > > unknown module parameters.  Thus, I'd suggest wrapping the ifdef around
> > > this module parameter, so it doesn't exist at all without
> > > CONFIG_DEBUG_OBJECTS_RCU_HEAD.
> > > 
> > > Alternatively, consider providing the test unconditionally, and just
> > > printing a big warning message saying that it's going to cause
> > > corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.
> > 
> > I currently do something like the above.  The module parameter
> > is defined unconditionally, but the actual tests are under #ifdef
> > CONFIG_DEBUG_OBJECTS_RCU_HEAD.  If you specify object_debug for a
> > !CONFIG_DEBUG_OBJECTS_RCU_HEAD kernel, the pr_alert() below happens,
> > and the test is omitted, thus avoiding the list corruption.
> > 
> > Seem reasonable?
> 
> That's exactly the bit I was commenting on.  I'm saying that you should
> either make the test unconditional (perhaps with a warning saying it's
> about to cause list corruption), or you should compile out the module
> parameter as well and then you don't need the pr_alert (since current
> kernels will emit a warning when you pass a non-existent module
> parameter).
> 
> Personally, I'd go with the latter.

Ah, the problem is the ugly ifdef in the middle of a function.  Yeah,
that is a problem in need of fixing.  No idea what I was thinking...

How about if I pull that block of code out into its own function, and
#ifdef the function body.  For example, something like that shown below.

							Thanx, Paul

static void rcu_test_debug_objects(void)
{
#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
	struct rcu_head rh1;
	struct rcu_head rh2;

	init_rcu_head_on_stack(&rh1);
	init_rcu_head_on_stack(&rh2);
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
	local_irq_disable(); /* Make it hard to finish grace period. */
	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
	call_rcu(&rh2, rcu_torture_err_cb);
	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
	local_irq_enable();
	rcu_barrier();
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
	destroy_rcu_head_on_stack(&rh1);
	destroy_rcu_head_on_stack(&rh2);
#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
}

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Josh Triplett]

On Mon, Aug 19, 2013 at 09:09:45AM -0700, Paul E. McKenney wrote:
> On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
> > On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
> > > On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
> > > > On Sat, Aug 17, 2013 at 07:25:13PM -0700, Paul E. McKenney wrote:
> > > > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > > > 
> > > > > This commit adds a object_debug option to rcutorture to allow the
> > > > > debug-object-based checks for duplicate call_rcu() invocations to
> > > > > be deterministically tested.
> > > > > 
> > > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > > > > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> > > > > Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
> > > > > Cc: Rik van Riel <riel@surriel.com>
> > > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > > > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> > > > 
> > > > Two comments below; with those fixed,
> > > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > > > 
> > > > > ---
> > > > > @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
> > > > >  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
> > > > >  module_param(n_barrier_cbs, int, 0444);
> > > > >  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
> > > > > +module_param(object_debug, int, 0444);
> > > > > +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
> > > > 
> > > > modules-next has a change to ignore and warn about
> > > > unknown module parameters.  Thus, I'd suggest wrapping the ifdef around
> > > > this module parameter, so it doesn't exist at all without
> > > > CONFIG_DEBUG_OBJECTS_RCU_HEAD.
> > > > 
> > > > Alternatively, consider providing the test unconditionally, and just
> > > > printing a big warning message saying that it's going to cause
> > > > corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.
> > > 
> > > I currently do something like the above.  The module parameter
> > > is defined unconditionally, but the actual tests are under #ifdef
> > > CONFIG_DEBUG_OBJECTS_RCU_HEAD.  If you specify object_debug for a
> > > !CONFIG_DEBUG_OBJECTS_RCU_HEAD kernel, the pr_alert() below happens,
> > > and the test is omitted, thus avoiding the list corruption.
> > > 
> > > Seem reasonable?
> > 
> > That's exactly the bit I was commenting on.  I'm saying that you should
> > either make the test unconditional (perhaps with a warning saying it's
> > about to cause list corruption), or you should compile out the module
> > parameter as well and then you don't need the pr_alert (since current
> > kernels will emit a warning when you pass a non-existent module
> > parameter).
> > 
> > Personally, I'd go with the latter.
> 
> Ah, the problem is the ugly ifdef in the middle of a function.  Yeah,
> that is a problem in need of fixing.  No idea what I was thinking...
> 
> How about if I pull that block of code out into its own function, and
> #ifdef the function body.  For example, something like that shown below.
[...]
> static void rcu_test_debug_objects(void)
> {
> #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> 	struct rcu_head rh1;
> 	struct rcu_head rh2;
> 
> 	init_rcu_head_on_stack(&rh1);
> 	init_rcu_head_on_stack(&rh2);
> 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> 	local_irq_disable(); /* Make it hard to finish grace period. */
> 	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
> 	call_rcu(&rh2, rcu_torture_err_cb);
> 	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
> 	local_irq_enable();
> 	rcu_barrier();
> 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> 	destroy_rcu_head_on_stack(&rh1);
> 	destroy_rcu_head_on_stack(&rh2);
> #else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> 	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> #endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> }
> 

That's a major improvement, but I'd still suggest a couple more changes:
move the if for the config option into that function inside the ifdef,
wrap the config parameter itself in an ifdef so it doesn't exist without
CONFIG_DEBUG_OBJECTS_RCU_HEAD, and drop the #else branch since the
kernel will already emit a warning if you use the module parameter with
!CONFIG_DEBUG_OBJECTS_RCU_HEAD.

- Josh Triplett

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Paul E. McKenney]

On Mon, Aug 19, 2013 at 10:16:52AM -0700, Josh Triplett wrote:
> On Mon, Aug 19, 2013 at 09:09:45AM -0700, Paul E. McKenney wrote:
> > On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
> > > On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
> > > > On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
> > > > > On Sat, Aug 17, 2013 at 07:25:13PM -0700, Paul E. McKenney wrote:
> > > > > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > > > > 
> > > > > > This commit adds a object_debug option to rcutorture to allow the
> > > > > > debug-object-based checks for duplicate call_rcu() invocations to
> > > > > > be deterministically tested.
> > > > > > 
> > > > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > > > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > > > > > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> > > > > > Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
> > > > > > Cc: Rik van Riel <riel@surriel.com>
> > > > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > > > > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > > > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> > > > > 
> > > > > Two comments below; with those fixed,
> > > > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > > > > 
> > > > > > ---
> > > > > > @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
> > > > > >  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
> > > > > >  module_param(n_barrier_cbs, int, 0444);
> > > > > >  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
> > > > > > +module_param(object_debug, int, 0444);
> > > > > > +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
> > > > > 
> > > > > modules-next has a change to ignore and warn about
> > > > > unknown module parameters.  Thus, I'd suggest wrapping the ifdef around
> > > > > this module parameter, so it doesn't exist at all without
> > > > > CONFIG_DEBUG_OBJECTS_RCU_HEAD.
> > > > > 
> > > > > Alternatively, consider providing the test unconditionally, and just
> > > > > printing a big warning message saying that it's going to cause
> > > > > corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.
> > > > 
> > > > I currently do something like the above.  The module parameter
> > > > is defined unconditionally, but the actual tests are under #ifdef
> > > > CONFIG_DEBUG_OBJECTS_RCU_HEAD.  If you specify object_debug for a
> > > > !CONFIG_DEBUG_OBJECTS_RCU_HEAD kernel, the pr_alert() below happens,
> > > > and the test is omitted, thus avoiding the list corruption.
> > > > 
> > > > Seem reasonable?
> > > 
> > > That's exactly the bit I was commenting on.  I'm saying that you should
> > > either make the test unconditional (perhaps with a warning saying it's
> > > about to cause list corruption), or you should compile out the module
> > > parameter as well and then you don't need the pr_alert (since current
> > > kernels will emit a warning when you pass a non-existent module
> > > parameter).
> > > 
> > > Personally, I'd go with the latter.
> > 
> > Ah, the problem is the ugly ifdef in the middle of a function.  Yeah,
> > that is a problem in need of fixing.  No idea what I was thinking...
> > 
> > How about if I pull that block of code out into its own function, and
> > #ifdef the function body.  For example, something like that shown below.
> [...]
> > static void rcu_test_debug_objects(void)
> > {
> > #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> > 	struct rcu_head rh1;
> > 	struct rcu_head rh2;
> > 
> > 	init_rcu_head_on_stack(&rh1);
> > 	init_rcu_head_on_stack(&rh2);
> > 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> > 	local_irq_disable(); /* Make it hard to finish grace period. */
> > 	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
> > 	call_rcu(&rh2, rcu_torture_err_cb);
> > 	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
> > 	local_irq_enable();
> > 	rcu_barrier();
> > 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> > 	destroy_rcu_head_on_stack(&rh1);
> > 	destroy_rcu_head_on_stack(&rh2);
> > #else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > 	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> > #endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > }
> 
> That's a major improvement, but I'd still suggest a couple more changes:
> move the if for the config option into that function inside the ifdef,
> wrap the config parameter itself in an ifdef so it doesn't exist without
> CONFIG_DEBUG_OBJECTS_RCU_HEAD, and drop the #else branch since the
> kernel will already emit a warning if you use the module parameter with
> !CONFIG_DEBUG_OBJECTS_RCU_HEAD.

I really feel the need to distinguish between "that is never a valid
parameter to rcutorture" (given by the module-parameter parser) and
"that parameter is valid, but cannot be used in this case, and here
are the consequences", so I will keep the pr_alert().  But definitely
fix the mid-function #ifdef!

							Thanx, Paul

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Josh Triplett]

On Mon, Aug 19, 2013 at 07:05:58PM -0700, Paul E. McKenney wrote:
> On Mon, Aug 19, 2013 at 10:16:52AM -0700, Josh Triplett wrote:
> > On Mon, Aug 19, 2013 at 09:09:45AM -0700, Paul E. McKenney wrote:
> > > On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
> > > > On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
> > > > > On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
> > > > > > On Sat, Aug 17, 2013 at 07:25:13PM -0700, Paul E. McKenney wrote:
> > > > > > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > > > > > 
> > > > > > > This commit adds a object_debug option to rcutorture to allow the
> > > > > > > debug-object-based checks for duplicate call_rcu() invocations to
> > > > > > > be deterministically tested.
> > > > > > > 
> > > > > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > > > > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > > > > > > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> > > > > > > Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
> > > > > > > Cc: Rik van Riel <riel@surriel.com>
> > > > > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > > > > > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > > > > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> > > > > > 
> > > > > > Two comments below; with those fixed,
> > > > > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > > > > > 
> > > > > > > ---
> > > > > > > @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
> > > > > > >  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
> > > > > > >  module_param(n_barrier_cbs, int, 0444);
> > > > > > >  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
> > > > > > > +module_param(object_debug, int, 0444);
> > > > > > > +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
> > > > > > 
> > > > > > modules-next has a change to ignore and warn about
> > > > > > unknown module parameters.  Thus, I'd suggest wrapping the ifdef around
> > > > > > this module parameter, so it doesn't exist at all without
> > > > > > CONFIG_DEBUG_OBJECTS_RCU_HEAD.
> > > > > > 
> > > > > > Alternatively, consider providing the test unconditionally, and just
> > > > > > printing a big warning message saying that it's going to cause
> > > > > > corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.
> > > > > 
> > > > > I currently do something like the above.  The module parameter
> > > > > is defined unconditionally, but the actual tests are under #ifdef
> > > > > CONFIG_DEBUG_OBJECTS_RCU_HEAD.  If you specify object_debug for a
> > > > > !CONFIG_DEBUG_OBJECTS_RCU_HEAD kernel, the pr_alert() below happens,
> > > > > and the test is omitted, thus avoiding the list corruption.
> > > > > 
> > > > > Seem reasonable?
> > > > 
> > > > That's exactly the bit I was commenting on.  I'm saying that you should
> > > > either make the test unconditional (perhaps with a warning saying it's
> > > > about to cause list corruption), or you should compile out the module
> > > > parameter as well and then you don't need the pr_alert (since current
> > > > kernels will emit a warning when you pass a non-existent module
> > > > parameter).
> > > > 
> > > > Personally, I'd go with the latter.
> > > 
> > > Ah, the problem is the ugly ifdef in the middle of a function.  Yeah,
> > > that is a problem in need of fixing.  No idea what I was thinking...
> > > 
> > > How about if I pull that block of code out into its own function, and
> > > #ifdef the function body.  For example, something like that shown below.
> > [...]
> > > static void rcu_test_debug_objects(void)
> > > {
> > > #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> > > 	struct rcu_head rh1;
> > > 	struct rcu_head rh2;
> > > 
> > > 	init_rcu_head_on_stack(&rh1);
> > > 	init_rcu_head_on_stack(&rh2);
> > > 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> > > 	local_irq_disable(); /* Make it hard to finish grace period. */
> > > 	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
> > > 	call_rcu(&rh2, rcu_torture_err_cb);
> > > 	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
> > > 	local_irq_enable();
> > > 	rcu_barrier();
> > > 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> > > 	destroy_rcu_head_on_stack(&rh1);
> > > 	destroy_rcu_head_on_stack(&rh2);
> > > #else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > > 	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> > > #endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > > }
> > 
> > That's a major improvement, but I'd still suggest a couple more changes:
> > move the if for the config option into that function inside the ifdef,
> > wrap the config parameter itself in an ifdef so it doesn't exist without
> > CONFIG_DEBUG_OBJECTS_RCU_HEAD, and drop the #else branch since the
> > kernel will already emit a warning if you use the module parameter with
> > !CONFIG_DEBUG_OBJECTS_RCU_HEAD.
> 
> I really feel the need to distinguish between "that is never a valid
> parameter to rcutorture" (given by the module-parameter parser) and
> "that parameter is valid, but cannot be used in this case, and here
> are the consequences", so I will keep the pr_alert().  But definitely
> fix the mid-function #ifdef!

OK; I can live with that.
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

Re: [PATCH tip/core/rcu 0/5] rcutorture updates for 3.12

[Josh Triplett]

On Sat, Aug 17, 2013 at 07:24:53PM -0700, Paul E. McKenney wrote:
> Hello!
> 
> This series provides some rcutorture updates:
> 
> 1.	Add a debug_object option to rcutorture to allow testing
> 	of RCU's debug-objects facilities.
> 
> 2.	Increase rcutorture test coverage by concurrently testing
> 	synchronous, asynchronous, and expedited grace periods.
> 
> 3.	Sort the extensive list of rcutorture module parameters.
> 
> 4.	Remove the unused "oldbatch" variable.
> 
> 5.	Emit CPU-online failure message to the console.

Comments on 1 and 3; for 2, 4, and 5:
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

[PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

This commit adds a object_debug option to rcutorture to allow the
debug-object-based checks for duplicate call_rcu() invocations to
be deterministically tested.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
[ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ]
---
 kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 3d936f0f..f5cf2bb 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
 static int fqs_holdoff;		/* Hold time within burst (us). */
 static int fqs_stutter = 3;	/* Wait time between bursts (s). */
 static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
+static int object_debug;	/* Test object-debug double call_rcu()?. */
 static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
 static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
 static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */
@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
 module_param(onoff_interval, int, 0444);
 MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
 module_param(onoff_holdoff, int, 0444);
@@ -1934,6 +1937,46 @@ rcu_torture_cleanup(void)
 		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
 }
 
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+static void rcu_torture_leak_cb(struct rcu_head *rhp)
+{
+}
+
+static void rcu_torture_err_cb(struct rcu_head *rhp)
+{
+	/* This -might- happen due to race conditions, but is unlikely. */
+	pr_alert("rcutorture: duplicated callback was invoked.\n");
+}
+#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+
+/*
+ * Verify that double-free causes debug-objects to complain, but only
+ * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
+ * cannot be carried out.
+ */
+static void rcu_test_debug_objects(void)
+{
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+	struct rcu_head rh1;
+	struct rcu_head rh2;
+
+	init_rcu_head_on_stack(&rh1);
+	init_rcu_head_on_stack(&rh2);
+	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
+	local_irq_disable(); /* Make it hard to finish grace period. */
+	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
+	call_rcu(&rh2, rcu_torture_err_cb);
+	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
+	local_irq_enable();
+	rcu_barrier();
+	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
+	destroy_rcu_head_on_stack(&rh1);
+	destroy_rcu_head_on_stack(&rh2);
+#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
+#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+}
+
 static int __init
 rcu_torture_init(void)
 {
@@ -2163,6 +2206,8 @@ rcu_torture_init(void)
 		firsterr = retval;
 		goto unwind;
 	}
+	if (object_debug)
+		rcu_test_debug_objects();
 	rcutorture_record_test_transition();
 	mutex_unlock(&fullstop_mutex);
 	return 0;
-- 
1.8.1.5

[PATCH tip/core/rcu 2/5] rcu: Increase rcutorture test coverage

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

Currently, rcutorture has separate torture_types to test synchronous,
asynchronous, and expedited grace-period primitives.  This has
two disadvantages: (1) Three times the number of runs to cover the
combinations and (2) Little testing of concurrent combinations of the
three options.  This commit therefore adds a pair of module parameters
that control normal and expedited state, with the default being both
types, randomly selected, by the fakewriter processes, thus reducing
source-code size and increasing test coverage.  In addtion, the writer
task switches between asynchronous-normal and expedited grace-period
primitives driven by the same pair of module parameters.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
---
 Documentation/RCU/torture.txt |  10 ++
 kernel/rcutorture.c           | 226 ++++++++++++------------------------------
 2 files changed, 73 insertions(+), 163 deletions(-)

diff --git a/Documentation/RCU/torture.txt b/Documentation/RCU/torture.txt
index d8a5023..dac02a6 100644
--- a/Documentation/RCU/torture.txt
+++ b/Documentation/RCU/torture.txt
@@ -42,6 +42,16 @@ fqs_holdoff	Holdoff time (in microseconds) between consecutive calls
 fqs_stutter	Wait time (in seconds) between consecutive bursts
 		of calls to force_quiescent_state().
 
+gp_normal	Make the fake writers use normal synchronous grace-period
+		primitives.
+
+gp_exp		Make the fake writers use expedited synchronous grace-period
+		primitives.  If both gp_normal and gp_exp are set, or
+		if neither gp_normal nor gp_exp are set, then randomly
+		choose the primitive so that about 50% are normal and
+		50% expedited.  By default, neither are set, which
+		gives best overall test coverage.
+
 irqreader	Says to invoke RCU readers from irq level.  This is currently
 		done via timers.  Defaults to "1" for variants of RCU that
 		permit this.  (Or, more accurately, variants of RCU that do
diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index f5cf2bb..10820b7 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -65,6 +65,8 @@ static int irqreader = 1;	/* RCU readers from irq (timers). */
 static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
 static int fqs_holdoff;		/* Hold time within burst (us). */
 static int fqs_stutter = 3;	/* Wait time between bursts (s). */
+static bool gp_exp;		/* Use expedited GP wait primitives. */
+static bool gp_normal;		/* Use normal GP wait primitives. */
 static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
 static int object_debug;	/* Test object-debug double call_rcu()?. */
 static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
@@ -99,6 +101,10 @@ module_param(fqs_holdoff, int, 0444);
 MODULE_PARM_DESC(fqs_holdoff, "Holdoff time within fqs bursts (us)");
 module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
+module_param(gp_normal, bool, 0444);
+MODULE_PARM_DESC(gp_normal, "Use normal (non-expedited) GP wait primitives");
+module_param(gp_exp, bool, 0444);
+MODULE_PARM_DESC(gp_exp, "Use expedited GP wait primitives");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
 module_param(object_debug, int, 0444);
@@ -363,6 +369,7 @@ struct rcu_torture_ops {
 	int (*completed)(void);
 	void (*deferred_free)(struct rcu_torture *p);
 	void (*sync)(void);
+	void (*exp_sync)(void);
 	void (*call)(struct rcu_head *head, void (*func)(struct rcu_head *rcu));
 	void (*cb_barrier)(void);
 	void (*fqs)(void);
@@ -446,81 +453,27 @@ static void rcu_torture_deferred_free(struct rcu_torture *p)
 	call_rcu(&p->rtort_rcu, rcu_torture_cb);
 }
 
-static struct rcu_torture_ops rcu_ops = {
-	.init		= NULL,
-	.readlock	= rcu_torture_read_lock,
-	.read_delay	= rcu_read_delay,
-	.readunlock	= rcu_torture_read_unlock,
-	.completed	= rcu_torture_completed,
-	.deferred_free	= rcu_torture_deferred_free,
-	.sync		= synchronize_rcu,
-	.call		= call_rcu,
-	.cb_barrier	= rcu_barrier,
-	.fqs		= rcu_force_quiescent_state,
-	.stats		= NULL,
-	.irq_capable	= 1,
-	.can_boost	= rcu_can_boost(),
-	.name		= "rcu"
-};
-
-static void rcu_sync_torture_deferred_free(struct rcu_torture *p)
-{
-	int i;
-	struct rcu_torture *rp;
-	struct rcu_torture *rp1;
-
-	cur_ops->sync();
-	list_add(&p->rtort_free, &rcu_torture_removed);
-	list_for_each_entry_safe(rp, rp1, &rcu_torture_removed, rtort_free) {
-		i = rp->rtort_pipe_count;
-		if (i > RCU_TORTURE_PIPE_LEN)
-			i = RCU_TORTURE_PIPE_LEN;
-		atomic_inc(&rcu_torture_wcount[i]);
-		if (++rp->rtort_pipe_count >= RCU_TORTURE_PIPE_LEN) {
-			rp->rtort_mbtest = 0;
-			list_del(&rp->rtort_free);
-			rcu_torture_free(rp);
-		}
-	}
-}
-
 static void rcu_sync_torture_init(void)
 {
 	INIT_LIST_HEAD(&rcu_torture_removed);
 }
 
-static struct rcu_torture_ops rcu_sync_ops = {
+static struct rcu_torture_ops rcu_ops = {
 	.init		= rcu_sync_torture_init,
 	.readlock	= rcu_torture_read_lock,
 	.read_delay	= rcu_read_delay,
 	.readunlock	= rcu_torture_read_unlock,
 	.completed	= rcu_torture_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
+	.deferred_free	= rcu_torture_deferred_free,
 	.sync		= synchronize_rcu,
-	.call		= NULL,
-	.cb_barrier	= NULL,
-	.fqs		= rcu_force_quiescent_state,
-	.stats		= NULL,
-	.irq_capable	= 1,
-	.can_boost	= rcu_can_boost(),
-	.name		= "rcu_sync"
-};
-
-static struct rcu_torture_ops rcu_expedited_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= rcu_torture_read_lock,
-	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
-	.readunlock	= rcu_torture_read_unlock,
-	.completed	= rcu_no_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= synchronize_rcu_expedited,
-	.call		= NULL,
-	.cb_barrier	= NULL,
+	.exp_sync	= synchronize_rcu_expedited,
+	.call		= call_rcu,
+	.cb_barrier	= rcu_barrier,
 	.fqs		= rcu_force_quiescent_state,
 	.stats		= NULL,
 	.irq_capable	= 1,
 	.can_boost	= rcu_can_boost(),
-	.name		= "rcu_expedited"
+	.name		= "rcu"
 };
 
 /*
@@ -549,13 +502,14 @@ static void rcu_bh_torture_deferred_free(struct rcu_torture *p)
 }
 
 static struct rcu_torture_ops rcu_bh_ops = {
-	.init		= NULL,
+	.init		= rcu_sync_torture_init,
 	.readlock	= rcu_bh_torture_read_lock,
 	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
 	.readunlock	= rcu_bh_torture_read_unlock,
 	.completed	= rcu_bh_torture_completed,
 	.deferred_free	= rcu_bh_torture_deferred_free,
 	.sync		= synchronize_rcu_bh,
+	.exp_sync	= synchronize_rcu_bh_expedited,
 	.call		= call_rcu_bh,
 	.cb_barrier	= rcu_barrier_bh,
 	.fqs		= rcu_bh_force_quiescent_state,
@@ -564,38 +518,6 @@ static struct rcu_torture_ops rcu_bh_ops = {
 	.name		= "rcu_bh"
 };
 
-static struct rcu_torture_ops rcu_bh_sync_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= rcu_bh_torture_read_lock,
-	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
-	.readunlock	= rcu_bh_torture_read_unlock,
-	.completed	= rcu_bh_torture_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= synchronize_rcu_bh,
-	.call		= NULL,
-	.cb_barrier	= NULL,
-	.fqs		= rcu_bh_force_quiescent_state,
-	.stats		= NULL,
-	.irq_capable	= 1,
-	.name		= "rcu_bh_sync"
-};
-
-static struct rcu_torture_ops rcu_bh_expedited_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= rcu_bh_torture_read_lock,
-	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
-	.readunlock	= rcu_bh_torture_read_unlock,
-	.completed	= rcu_bh_torture_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= synchronize_rcu_bh_expedited,
-	.call		= NULL,
-	.cb_barrier	= NULL,
-	.fqs		= rcu_bh_force_quiescent_state,
-	.stats		= NULL,
-	.irq_capable	= 1,
-	.name		= "rcu_bh_expedited"
-};
-
 /*
  * Definitions for srcu torture testing.
  */
@@ -670,6 +592,11 @@ static int srcu_torture_stats(char *page)
 	return cnt;
 }
 
+static void srcu_torture_synchronize_expedited(void)
+{
+	synchronize_srcu_expedited(&srcu_ctl);
+}
+
 static struct rcu_torture_ops srcu_ops = {
 	.init		= rcu_sync_torture_init,
 	.readlock	= srcu_torture_read_lock,
@@ -678,45 +605,13 @@ static struct rcu_torture_ops srcu_ops = {
 	.completed	= srcu_torture_completed,
 	.deferred_free	= srcu_torture_deferred_free,
 	.sync		= srcu_torture_synchronize,
+	.exp_sync	= srcu_torture_synchronize_expedited,
 	.call		= srcu_torture_call,
 	.cb_barrier	= srcu_torture_barrier,
 	.stats		= srcu_torture_stats,
 	.name		= "srcu"
 };
 
-static struct rcu_torture_ops srcu_sync_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= srcu_torture_read_lock,
-	.read_delay	= srcu_read_delay,
-	.readunlock	= srcu_torture_read_unlock,
-	.completed	= srcu_torture_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= srcu_torture_synchronize,
-	.call		= NULL,
-	.cb_barrier	= NULL,
-	.stats		= srcu_torture_stats,
-	.name		= "srcu_sync"
-};
-
-static void srcu_torture_synchronize_expedited(void)
-{
-	synchronize_srcu_expedited(&srcu_ctl);
-}
-
-static struct rcu_torture_ops srcu_expedited_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= srcu_torture_read_lock,
-	.read_delay	= srcu_read_delay,
-	.readunlock	= srcu_torture_read_unlock,
-	.completed	= srcu_torture_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= srcu_torture_synchronize_expedited,
-	.call		= NULL,
-	.cb_barrier	= NULL,
-	.stats		= srcu_torture_stats,
-	.name		= "srcu_expedited"
-};
-
 /*
  * Definitions for sched torture testing.
  */
@@ -745,6 +640,8 @@ static struct rcu_torture_ops sched_ops = {
 	.completed	= rcu_no_completed,
 	.deferred_free	= rcu_sched_torture_deferred_free,
 	.sync		= synchronize_sched,
+	.exp_sync	= synchronize_sched_expedited,
+	.call		= call_rcu_sched,
 	.cb_barrier	= rcu_barrier_sched,
 	.fqs		= rcu_sched_force_quiescent_state,
 	.stats		= NULL,
@@ -752,35 +649,6 @@ static struct rcu_torture_ops sched_ops = {
 	.name		= "sched"
 };
 
-static struct rcu_torture_ops sched_sync_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= sched_torture_read_lock,
-	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
-	.readunlock	= sched_torture_read_unlock,
-	.completed	= rcu_no_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= synchronize_sched,
-	.cb_barrier	= NULL,
-	.fqs		= rcu_sched_force_quiescent_state,
-	.stats		= NULL,
-	.name		= "sched_sync"
-};
-
-static struct rcu_torture_ops sched_expedited_ops = {
-	.init		= rcu_sync_torture_init,
-	.readlock	= sched_torture_read_lock,
-	.read_delay	= rcu_read_delay,  /* just reuse rcu's version. */
-	.readunlock	= sched_torture_read_unlock,
-	.completed	= rcu_no_completed,
-	.deferred_free	= rcu_sync_torture_deferred_free,
-	.sync		= synchronize_sched_expedited,
-	.cb_barrier	= NULL,
-	.fqs		= rcu_sched_force_quiescent_state,
-	.stats		= NULL,
-	.irq_capable	= 1,
-	.name		= "sched_expedited"
-};
-
 /*
  * RCU torture priority-boost testing.  Runs one real-time thread per
  * CPU for moderate bursts, repeatedly registering RCU callbacks and
@@ -930,9 +798,11 @@ rcu_torture_fqs(void *arg)
 static int
 rcu_torture_writer(void *arg)
 {
+	bool exp;
 	int i;
 	long oldbatch = rcu_batches_completed();
 	struct rcu_torture *rp;
+	struct rcu_torture *rp1;
 	struct rcu_torture *old_rp;
 	static DEFINE_RCU_RANDOM(rand);
 
@@ -957,7 +827,31 @@ rcu_torture_writer(void *arg)
 				i = RCU_TORTURE_PIPE_LEN;
 			atomic_inc(&rcu_torture_wcount[i]);
 			old_rp->rtort_pipe_count++;
-			cur_ops->deferred_free(old_rp);
+			if (gp_normal == gp_exp)
+				exp = !!(rcu_random(&rand) & 0x80);
+			else
+				exp = gp_exp;
+			if (!exp) {
+				cur_ops->deferred_free(old_rp);
+			} else {
+				cur_ops->exp_sync();
+				list_add(&old_rp->rtort_free,
+					 &rcu_torture_removed);
+				list_for_each_entry_safe(rp, rp1,
+							 &rcu_torture_removed,
+							 rtort_free) {
+					i = rp->rtort_pipe_count;
+					if (i > RCU_TORTURE_PIPE_LEN)
+						i = RCU_TORTURE_PIPE_LEN;
+					atomic_inc(&rcu_torture_wcount[i]);
+					if (++rp->rtort_pipe_count >=
+					    RCU_TORTURE_PIPE_LEN) {
+						rp->rtort_mbtest = 0;
+						list_del(&rp->rtort_free);
+						rcu_torture_free(rp);
+					}
+				 }
+			}
 		}
 		rcutorture_record_progress(++rcu_torture_current_version);
 		oldbatch = cur_ops->completed();
@@ -986,10 +880,18 @@ rcu_torture_fakewriter(void *arg)
 		schedule_timeout_uninterruptible(1 + rcu_random(&rand)%10);
 		udelay(rcu_random(&rand) & 0x3ff);
 		if (cur_ops->cb_barrier != NULL &&
-		    rcu_random(&rand) % (nfakewriters * 8) == 0)
+		    rcu_random(&rand) % (nfakewriters * 8) == 0) {
 			cur_ops->cb_barrier();
-		else
+		} else if (gp_normal == gp_exp) {
+			if (rcu_random(&rand) & 0x80)
+				cur_ops->sync();
+			else
+				cur_ops->exp_sync();
+		} else if (gp_normal) {
 			cur_ops->sync();
+		} else {
+			cur_ops->exp_sync();
+		}
 		rcu_stutter_wait("rcu_torture_fakewriter");
 	} while (!kthread_should_stop() && fullstop == FULLSTOP_DONTSTOP);
 
@@ -1984,11 +1886,9 @@ rcu_torture_init(void)
 	int cpu;
 	int firsterr = 0;
 	int retval;
-	static struct rcu_torture_ops *torture_ops[] =
-		{ &rcu_ops, &rcu_sync_ops, &rcu_expedited_ops,
-		  &rcu_bh_ops, &rcu_bh_sync_ops, &rcu_bh_expedited_ops,
-		  &srcu_ops, &srcu_sync_ops, &srcu_expedited_ops,
-		  &sched_ops, &sched_sync_ops, &sched_expedited_ops, };
+	static struct rcu_torture_ops *torture_ops[] = {
+		&rcu_ops, &rcu_bh_ops, &srcu_ops, &sched_ops,
+	};
 
 	mutex_lock(&fullstop_mutex);
 
-- 
1.8.1.5

[PATCH tip/core/rcu 3/5] rcu: Sort rcutorture module parameters

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

There are getting to be too many module parameters to permit the current
semi-random order, so this patch orders them.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
---
 kernel/rcutorture.c | 101 +++++++++++++++++++++++++---------------------------
 1 file changed, 49 insertions(+), 52 deletions(-)

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 10820b7..949a8f2 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -52,81 +52,78 @@
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Paul E. McKenney <paulmck@us.ibm.com> and Josh Triplett <josh@freedesktop.org>");
 
-static int nreaders = -1;	/* # reader threads, defaults to 2*ncpus */
-static int nfakewriters = 4;	/* # fake writer threads */
-static int stat_interval = 60;	/* Interval between stats, in seconds. */
-				/*  Zero means "only at end of test". */
-static bool verbose;		/* Print more debug info. */
-static bool test_no_idle_hz = true;
-				/* Test RCU support for tickless idle CPUs. */
-static int shuffle_interval = 3; /* Interval between shuffles (in sec)*/
-static int stutter = 5;		/* Start/stop testing interval (in sec) */
-static int irqreader = 1;	/* RCU readers from irq (timers). */
-static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
-static int fqs_holdoff;		/* Hold time within burst (us). */
-static int fqs_stutter = 3;	/* Wait time between bursts (s). */
-static bool gp_exp;		/* Use expedited GP wait primitives. */
-static bool gp_normal;		/* Use normal GP wait primitives. */
-static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
-static int object_debug;	/* Test object-debug double call_rcu()?. */
-static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
-static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
-static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */
-static int stall_cpu;		/* CPU-stall duration (s).  0 for no stall. */
-static int stall_cpu_holdoff = 10; /* Time to wait until stall (s).  */
-static int test_boost = 1;	/* Test RCU prio boost: 0=no, 1=maybe, 2=yes. */
-static int test_boost_interval = 7; /* Interval between boost tests, seconds. */
-static int test_boost_duration = 4; /* Duration of each boost test, seconds. */
-static char *torture_type = "rcu"; /* What RCU implementation to torture. */
-
-module_param(nreaders, int, 0444);
-MODULE_PARM_DESC(nreaders, "Number of RCU reader threads");
-module_param(nfakewriters, int, 0444);
-MODULE_PARM_DESC(nfakewriters, "Number of RCU fake writer threads");
-module_param(stat_interval, int, 0644);
-MODULE_PARM_DESC(stat_interval, "Number of seconds between stats printk()s");
-module_param(verbose, bool, 0444);
-MODULE_PARM_DESC(verbose, "Enable verbose debugging printk()s");
-module_param(test_no_idle_hz, bool, 0444);
-MODULE_PARM_DESC(test_no_idle_hz, "Test support for tickless idle CPUs");
-module_param(shuffle_interval, int, 0444);
-MODULE_PARM_DESC(shuffle_interval, "Number of seconds between shuffles");
-module_param(stutter, int, 0444);
-MODULE_PARM_DESC(stutter, "Number of seconds to run/halt test");
-module_param(irqreader, int, 0444);
-MODULE_PARM_DESC(irqreader, "Allow RCU readers from irq handlers");
+static int fqs_duration;
 module_param(fqs_duration, int, 0444);
-MODULE_PARM_DESC(fqs_duration, "Duration of fqs bursts (us)");
+MODULE_PARM_DESC(fqs_duration, "Duration of fqs bursts (us), 0 to disable");
+static int fqs_holdoff;
 module_param(fqs_holdoff, int, 0444);
 MODULE_PARM_DESC(fqs_holdoff, "Holdoff time within fqs bursts (us)");
+static int fqs_stutter = 3;
 module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
-module_param(gp_normal, bool, 0444);
-MODULE_PARM_DESC(gp_normal, "Use normal (non-expedited) GP wait primitives");
+static bool gp_exp;
 module_param(gp_exp, bool, 0444);
 MODULE_PARM_DESC(gp_exp, "Use expedited GP wait primitives");
+static bool gp_normal;
+module_param(gp_normal, bool, 0444);
+MODULE_PARM_DESC(gp_normal, "Use normal (non-expedited) GP wait primitives");
+static int irqreader = 1;
+module_param(irqreader, int, 0444);
+MODULE_PARM_DESC(irqreader, "Allow RCU readers from irq handlers");
+static int n_barrier_cbs;
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+static int nfakewriters = 4;
+module_param(nfakewriters, int, 0444);
+MODULE_PARM_DESC(nfakewriters, "Number of RCU fake writer threads");
+static int nreaders = -1;
+module_param(nreaders, int, 0444);
+MODULE_PARM_DESC(nreaders, "Number of RCU reader threads");
+static int object_debug;
 module_param(object_debug, int, 0444);
 MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
-module_param(onoff_interval, int, 0444);
-MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
+static int onoff_holdoff;
 module_param(onoff_holdoff, int, 0444);
 MODULE_PARM_DESC(onoff_holdoff, "Time after boot before CPU hotplugs (s)");
+static int onoff_interval;
+module_param(onoff_interval, int, 0444);
+MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
+static int shuffle_interval = 3;
+module_param(shuffle_interval, int, 0444);
+MODULE_PARM_DESC(shuffle_interval, "Number of seconds between shuffles");
+static int shutdown_secs;
 module_param(shutdown_secs, int, 0444);
-MODULE_PARM_DESC(shutdown_secs, "Shutdown time (s), zero to disable.");
+MODULE_PARM_DESC(shutdown_secs, "Shutdown time (s), <= zero to disable.");
+static int stall_cpu;
 module_param(stall_cpu, int, 0444);
 MODULE_PARM_DESC(stall_cpu, "Stall duration (s), zero to disable.");
+static int stall_cpu_holdoff = 10;
 module_param(stall_cpu_holdoff, int, 0444);
 MODULE_PARM_DESC(stall_cpu_holdoff, "Time to wait before starting stall (s).");
+static int stat_interval = 60;
+module_param(stat_interval, int, 0644);
+MODULE_PARM_DESC(stat_interval, "Number of seconds between stats printk()s");
+static int stutter = 5;
+module_param(stutter, int, 0444);
+MODULE_PARM_DESC(stutter, "Number of seconds to run/halt test");
+static int test_boost = 1;
 module_param(test_boost, int, 0444);
 MODULE_PARM_DESC(test_boost, "Test RCU prio boost: 0=no, 1=maybe, 2=yes.");
-module_param(test_boost_interval, int, 0444);
-MODULE_PARM_DESC(test_boost_interval, "Interval between boost tests, seconds.");
+static int test_boost_duration = 4;
 module_param(test_boost_duration, int, 0444);
 MODULE_PARM_DESC(test_boost_duration, "Duration of each boost test, seconds.");
+static int test_boost_interval = 7;
+module_param(test_boost_interval, int, 0444);
+MODULE_PARM_DESC(test_boost_interval, "Interval between boost tests, seconds.");
+static bool test_no_idle_hz = true;
+module_param(test_no_idle_hz, bool, 0444);
+MODULE_PARM_DESC(test_no_idle_hz, "Test support for tickless idle CPUs");
+static char *torture_type = "rcu";
 module_param(torture_type, charp, 0444);
-MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, rcu_bh, srcu)");
+MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, rcu_bh, ...)");
+static bool verbose;
+module_param(verbose, bool, 0444);
+MODULE_PARM_DESC(verbose, "Enable verbose debugging printk()s");
 
 #define TORTURE_FLAG "-torture:"
 #define PRINTK_STRING(s) \
-- 
1.8.1.5

[PATCH tip/core/rcu 4/5] rcu: Remove unused variable from rcu_torture_writer()

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

The oldbatch variable in rcu_torture_writer() is stored to, but never
loaded from.  This commit therefore removes it.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
---
 kernel/rcutorture.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 949a8f2..869dac2 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -797,7 +797,6 @@ rcu_torture_writer(void *arg)
 {
 	bool exp;
 	int i;
-	long oldbatch = rcu_batches_completed();
 	struct rcu_torture *rp;
 	struct rcu_torture *rp1;
 	struct rcu_torture *old_rp;
@@ -851,7 +850,6 @@ rcu_torture_writer(void *arg)
 			}
 		}
 		rcutorture_record_progress(++rcu_torture_current_version);
-		oldbatch = cur_ops->completed();
 		rcu_stutter_wait("rcu_torture_writer");
 	} while (!kthread_should_stop() && fullstop == FULLSTOP_DONTSTOP);
 	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
-- 
1.8.1.5

[PATCH tip/core/rcu 5/5] rcu: Make rcutorture emit online failures if verbose

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

Although rcutorture counts CPU-hotplug online failures, it does
not explicitly record which CPUs were having trouble coming online.
This commit therefore emits a console message when online failure occurs.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
---
 kernel/rcutorture.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 869dac2..a4a73d4 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -1434,7 +1434,13 @@ rcu_torture_onoff(void *arg)
 					 torture_type, cpu);
 			starttime = jiffies;
 			n_online_attempts++;
-			if (cpu_up(cpu) == 0) {
+			ret = cpu_up(cpu);
+			if (ret) {
+				if (verbose)
+					pr_alert("%s" TORTURE_FLAG
+						 "rcu_torture_onoff task: online %d failed: errno %d\n",
+						 torture_type, cpu, ret);
+			} else {
 				if (verbose)
 					pr_alert("%s" TORTURE_FLAG
 						 "rcu_torture_onoff task: onlined %d\n",
-- 
1.8.1.5

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Josh Triplett]

On Mon, Aug 19, 2013 at 07:51:10PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> This commit adds a object_debug option to rcutorture to allow the
> debug-object-based checks for duplicate call_rcu() invocations to
> be deterministically tested.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> [ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ]

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

> ---
>  kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> index 3d936f0f..f5cf2bb 100644
> --- a/kernel/rcutorture.c
> +++ b/kernel/rcutorture.c
> @@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
>  static int fqs_holdoff;		/* Hold time within burst (us). */
>  static int fqs_stutter = 3;	/* Wait time between bursts (s). */
>  static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
> +static int object_debug;	/* Test object-debug double call_rcu()?. */
>  static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
>  static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
>  static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */
> @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
>  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
>  module_param(n_barrier_cbs, int, 0444);
>  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
> +module_param(object_debug, int, 0444);
> +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
>  module_param(onoff_interval, int, 0444);
>  MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
>  module_param(onoff_holdoff, int, 0444);
> @@ -1934,6 +1937,46 @@ rcu_torture_cleanup(void)
>  		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
>  }
>  
> +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> +static void rcu_torture_leak_cb(struct rcu_head *rhp)
> +{
> +}
> +
> +static void rcu_torture_err_cb(struct rcu_head *rhp)
> +{
> +	/* This -might- happen due to race conditions, but is unlikely. */
> +	pr_alert("rcutorture: duplicated callback was invoked.\n");
> +}
> +#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> +
> +/*
> + * Verify that double-free causes debug-objects to complain, but only
> + * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
> + * cannot be carried out.
> + */
> +static void rcu_test_debug_objects(void)
> +{
> +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> +	struct rcu_head rh1;
> +	struct rcu_head rh2;
> +
> +	init_rcu_head_on_stack(&rh1);
> +	init_rcu_head_on_stack(&rh2);
> +	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> +	local_irq_disable(); /* Make it hard to finish grace period. */
> +	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
> +	call_rcu(&rh2, rcu_torture_err_cb);
> +	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
> +	local_irq_enable();
> +	rcu_barrier();
> +	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> +	destroy_rcu_head_on_stack(&rh1);
> +	destroy_rcu_head_on_stack(&rh2);
> +#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> +	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> +#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> +}
> +
>  static int __init
>  rcu_torture_init(void)
>  {
> @@ -2163,6 +2206,8 @@ rcu_torture_init(void)
>  		firsterr = retval;
>  		goto unwind;
>  	}
> +	if (object_debug)
> +		rcu_test_debug_objects();
>  	rcutorture_record_test_transition();
>  	mutex_unlock(&fullstop_mutex);
>  	return 0;
> -- 
> 1.8.1.5
> 

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Lai Jiangshan]

On 08/20/2013 10:51 AM, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> This commit adds a object_debug option to rcutorture to allow the
> debug-object-based checks for duplicate call_rcu() invocations to
> be deterministically tested.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> [ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ]
> ---
>  kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> index 3d936f0f..f5cf2bb 100644
> --- a/kernel/rcutorture.c
> +++ b/kernel/rcutorture.c
> @@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
>  static int fqs_holdoff;		/* Hold time within burst (us). */
>  static int fqs_stutter = 3;	/* Wait time between bursts (s). */
>  static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
> +static int object_debug;	/* Test object-debug double call_rcu()?. */
>  static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
>  static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
>  static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */
> @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
>  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
>  module_param(n_barrier_cbs, int, 0444);
>  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
> +module_param(object_debug, int, 0444);
> +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
>  module_param(onoff_interval, int, 0444);
>  MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
>  module_param(onoff_holdoff, int, 0444);
> @@ -1934,6 +1937,46 @@ rcu_torture_cleanup(void)
>  		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
>  }
>  
> +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> +static void rcu_torture_leak_cb(struct rcu_head *rhp)
> +{
> +}
> +
> +static void rcu_torture_err_cb(struct rcu_head *rhp)
> +{
> +	/* This -might- happen due to race conditions, but is unlikely. */
> +	pr_alert("rcutorture: duplicated callback was invoked.\n");
> +}
> +#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> +
> +/*
> + * Verify that double-free causes debug-objects to complain, but only
> + * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
> + * cannot be carried out.
> + */
> +static void rcu_test_debug_objects(void)
> +{
> +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> +	struct rcu_head rh1;
> +	struct rcu_head rh2;
> +
> +	init_rcu_head_on_stack(&rh1);
> +	init_rcu_head_on_stack(&rh2);
> +	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> +	local_irq_disable(); /* Make it hard to finish grace period. */

you can use rcu_read_lock() directly.

> +	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
> +	call_rcu(&rh2, rcu_torture_err_cb);
> +	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
> +	local_irq_enable();
> +	rcu_barrier();
> +	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> +	destroy_rcu_head_on_stack(&rh1);
> +	destroy_rcu_head_on_stack(&rh2);
> +#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> +	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> +#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> +}
> +
>  static int __init
>  rcu_torture_init(void)
>  {
> @@ -2163,6 +2206,8 @@ rcu_torture_init(void)
>  		firsterr = retval;
>  		goto unwind;
>  	}
> +	if (object_debug)
> +		rcu_test_debug_objects();
>  	rcutorture_record_test_transition();
>  	mutex_unlock(&fullstop_mutex);
>  	return 0;

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Paul E. McKenney]

On Tue, Aug 20, 2013 at 06:02:39PM +0800, Lai Jiangshan wrote:
> On 08/20/2013 10:51 AM, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > This commit adds a object_debug option to rcutorture to allow the
> > debug-object-based checks for duplicate call_rcu() invocations to
> > be deterministically tested.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> > Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
> > Cc: Rik van Riel <riel@surriel.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> > [ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ]
> > ---
> >  kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> > 
> > diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> > index 3d936f0f..f5cf2bb 100644
> > --- a/kernel/rcutorture.c
> > +++ b/kernel/rcutorture.c
> > @@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
> >  static int fqs_holdoff;		/* Hold time within burst (us). */
> >  static int fqs_stutter = 3;	/* Wait time between bursts (s). */
> >  static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
> > +static int object_debug;	/* Test object-debug double call_rcu()?. */
> >  static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
> >  static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
> >  static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */
> > @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
> >  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
> >  module_param(n_barrier_cbs, int, 0444);
> >  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
> > +module_param(object_debug, int, 0444);
> > +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
> >  module_param(onoff_interval, int, 0444);
> >  MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
> >  module_param(onoff_holdoff, int, 0444);
> > @@ -1934,6 +1937,46 @@ rcu_torture_cleanup(void)
> >  		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
> >  }
> >  
> > +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> > +static void rcu_torture_leak_cb(struct rcu_head *rhp)
> > +{
> > +}
> > +
> > +static void rcu_torture_err_cb(struct rcu_head *rhp)
> > +{
> > +	/* This -might- happen due to race conditions, but is unlikely. */
> > +	pr_alert("rcutorture: duplicated callback was invoked.\n");
> > +}
> > +#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > +
> > +/*
> > + * Verify that double-free causes debug-objects to complain, but only
> > + * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
> > + * cannot be carried out.
> > + */
> > +static void rcu_test_debug_objects(void)
> > +{
> > +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> > +	struct rcu_head rh1;
> > +	struct rcu_head rh2;
> > +
> > +	init_rcu_head_on_stack(&rh1);
> > +	init_rcu_head_on_stack(&rh2);
> > +	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> > +	local_irq_disable(); /* Make it hard to finish grace period. */
> 
> you can use rcu_read_lock() directly.

I could do that as well, but it doesn't do everything that local_irq_disable()
does.

Right, which means that my comment is bad.  Fixing both, thank you!

> > +	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */

And the one above cannot start a grace period due to irqs being enabled.
Which is -almost- always OK, but...

> > +	call_rcu(&rh2, rcu_torture_err_cb);

And this one should invoke rcu_torture_leak_cb instead of
rcu_torture_err_cb().  Just results in a confusing error message, but...

OK, a few more fixes, then!

> > +	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
> > +	local_irq_enable();
> > +	rcu_barrier();
> > +	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> > +	destroy_rcu_head_on_stack(&rh1);
> > +	destroy_rcu_head_on_stack(&rh2);
> > +#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > +	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> > +#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > +}

The result is as follows.  Better?

							Thanx, Paul

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
static void rcu_torture_leak_cb(struct rcu_head *rhp)
{
}

static void rcu_torture_err_cb(struct rcu_head *rhp)
{
	/*
	 * This -might- happen due to race conditions, but is unlikely.
	 * The scenario that leads to this happening is that the
	 * first of the pair of duplicate callbacks is queued,
	 * someone else starts a grace period that includes that
	 * callback, then the second of the pair must wait for the
	 * next grace period.  Unlikely, but can happen.  If it
	 * does happen, the debug-objects subsystem won't have splatted.
	 */
	pr_alert("rcutorture: duplicated callback was invoked.\n");
}
#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */

/*
 * Verify that double-free causes debug-objects to complain, but only
 * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
 * cannot be carried out.
 */
static void rcu_test_debug_objects(void)
{
#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
	struct rcu_head rh1;
	struct rcu_head rh2;

	init_rcu_head_on_stack(&rh1);
	init_rcu_head_on_stack(&rh2);
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
	preempt_disable(); /* Prevent preemption from interrupting test. */
	rcu_read_lock(); /* Make it impossible to finish a grace period. */
	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */
	local_irq_disable(); /* Make it harder to start a new grace period. */
	call_rcu(&rh2, rcu_torture_leak_cb);
	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
	local_irq_enable();
	rcu_read_unlock();
	preempt_enable();
	rcu_barrier();
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
	destroy_rcu_head_on_stack(&rh1);
	destroy_rcu_head_on_stack(&rh2);
#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
}

> > +
> >  static int __init
> >  rcu_torture_init(void)
> >  {
> > @@ -2163,6 +2206,8 @@ rcu_torture_init(void)
> >  		firsterr = retval;
> >  		goto unwind;
> >  	}
> > +	if (object_debug)
> > +		rcu_test_debug_objects();
> >  	rcutorture_record_test_transition();
> >  	mutex_unlock(&fullstop_mutex);
> >  	return 0;
> 

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Lai Jiangshan]

On 08/21/2013 02:38 AM, Paul E. McKenney wrote:
> On Tue, Aug 20, 2013 at 06:02:39PM +0800, Lai Jiangshan wrote:
>> On 08/20/2013 10:51 AM, Paul E. McKenney wrote:
>>> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>>>
>>> This commit adds a object_debug option to rcutorture to allow the
>>> debug-object-based checks for duplicate call_rcu() invocations to
>>> be deterministically tested.
>>>
>>> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>>> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>>> Cc: Sedat Dilek <sedat.dilek@gmail.com>
>>> Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
>>> Cc: Rik van Riel <riel@surriel.com>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>>> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
>>> [ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ]
>>> ---
>>>  kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 45 insertions(+)
>>>
>>> diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
>>> index 3d936f0f..f5cf2bb 100644
>>> --- a/kernel/rcutorture.c
>>> +++ b/kernel/rcutorture.c
>>> @@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
>>>  static int fqs_holdoff;		/* Hold time within burst (us). */
>>>  static int fqs_stutter = 3;	/* Wait time between bursts (s). */
>>>  static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
>>> +static int object_debug;	/* Test object-debug double call_rcu()?. */
>>>  static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
>>>  static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
>>>  static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */
>>> @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
>>>  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
>>>  module_param(n_barrier_cbs, int, 0444);
>>>  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
>>> +module_param(object_debug, int, 0444);
>>> +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
>>>  module_param(onoff_interval, int, 0444);
>>>  MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
>>>  module_param(onoff_holdoff, int, 0444);
>>> @@ -1934,6 +1937,46 @@ rcu_torture_cleanup(void)
>>>  		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
>>>  }
>>>  
>>> +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
>>> +static void rcu_torture_leak_cb(struct rcu_head *rhp)
>>> +{
>>> +}
>>> +
>>> +static void rcu_torture_err_cb(struct rcu_head *rhp)
>>> +{
>>> +	/* This -might- happen due to race conditions, but is unlikely. */
>>> +	pr_alert("rcutorture: duplicated callback was invoked.\n");
>>> +}
>>> +#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
>>> +
>>> +/*
>>> + * Verify that double-free causes debug-objects to complain, but only
>>> + * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
>>> + * cannot be carried out.
>>> + */
>>> +static void rcu_test_debug_objects(void)
>>> +{
>>> +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
>>> +	struct rcu_head rh1;
>>> +	struct rcu_head rh2;
>>> +
>>> +	init_rcu_head_on_stack(&rh1);
>>> +	init_rcu_head_on_stack(&rh2);
>>> +	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
>>> +	local_irq_disable(); /* Make it hard to finish grace period. */
>>
>> you can use rcu_read_lock() directly.
> 
> I could do that as well, but it doesn't do everything that local_irq_disable()
> does.
> 
> Right, which means that my comment is bad.  Fixing both, thank you!
> 
>>> +	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
> 
> And the one above cannot start a grace period due to irqs being enabled.
> Which is -almost- always OK, but...
> 
>>> +	call_rcu(&rh2, rcu_torture_err_cb);
> 
> And this one should invoke rcu_torture_leak_cb instead of
> rcu_torture_err_cb().  Just results in a confusing error message, but...

I still don't understand why rcu_torture_err_cb() will be called when:

rcu_read_lock();
call_rcu(&rh2, rcu_torture_leak_cb);
call_rcu(&rh2, rcu_torture_err_cb); // rh2 will be still queued here,
				    // debug-objects will find it and
				    // change it to rcu_leak_callback()
rcu_read_unlock();

> 
> OK, a few more fixes, then!
> 
>>> +	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
>>> +	local_irq_enable();
>>> +	rcu_barrier();
>>> +	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
>>> +	destroy_rcu_head_on_stack(&rh1);
>>> +	destroy_rcu_head_on_stack(&rh2);
>>> +#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
>>> +	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
>>> +#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
>>> +}
> 
> The result is as follows.  Better?
> 
> 							Thanx, Paul
> 
> #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> static void rcu_torture_leak_cb(struct rcu_head *rhp)
> {
> }
> 
> static void rcu_torture_err_cb(struct rcu_head *rhp)
> {
> 	/*
> 	 * This -might- happen due to race conditions, but is unlikely.
> 	 * The scenario that leads to this happening is that the
> 	 * first of the pair of duplicate callbacks is queued,
> 	 * someone else starts a grace period that includes that
> 	 * callback, then the second of the pair must wait for the
> 	 * next grace period.  Unlikely, but can happen.  If it
> 	 * does happen, the debug-objects subsystem won't have splatted.
> 	 */
> 	pr_alert("rcutorture: duplicated callback was invoked.\n");
> }
> #endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> 
> /*
>  * Verify that double-free causes debug-objects to complain, but only
>  * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
>  * cannot be carried out.
>  */
> static void rcu_test_debug_objects(void)
> {
> #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> 	struct rcu_head rh1;
> 	struct rcu_head rh2;
> 
> 	init_rcu_head_on_stack(&rh1);
> 	init_rcu_head_on_stack(&rh2);
> 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> 	preempt_disable(); /* Prevent preemption from interrupting test. */
> 	rcu_read_lock(); /* Make it impossible to finish a grace period. */
> 	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */
> 	local_irq_disable(); /* Make it harder to start a new grace period. */
> 	call_rcu(&rh2, rcu_torture_leak_cb);
> 	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
> 	local_irq_enable();
> 	rcu_read_unlock();
> 	preempt_enable();
> 	rcu_barrier();
> 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> 	destroy_rcu_head_on_stack(&rh1);
> 	destroy_rcu_head_on_stack(&rh2);
> #else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> 	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> #endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> }
> 
>>> +
>>>  static int __init
>>>  rcu_torture_init(void)
>>>  {
>>> @@ -2163,6 +2206,8 @@ rcu_torture_init(void)
>>>  		firsterr = retval;
>>>  		goto unwind;
>>>  	}
>>> +	if (object_debug)
>>> +		rcu_test_debug_objects();
>>>  	rcutorture_record_test_transition();
>>>  	mutex_unlock(&fullstop_mutex);
>>>  	return 0;
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Paul E. McKenney]

On Wed, Aug 21, 2013 at 10:40:15AM +0800, Lai Jiangshan wrote:
> On 08/21/2013 02:38 AM, Paul E. McKenney wrote:
> > On Tue, Aug 20, 2013 at 06:02:39PM +0800, Lai Jiangshan wrote:
> >> On 08/20/2013 10:51 AM, Paul E. McKenney wrote:
> >>> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> >>>
> >>> This commit adds a object_debug option to rcutorture to allow the
> >>> debug-object-based checks for duplicate call_rcu() invocations to
> >>> be deterministically tested.
> >>>
> >>> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >>> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> >>> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> >>> Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
> >>> Cc: Rik van Riel <riel@surriel.com>
> >>> Cc: Thomas Gleixner <tglx@linutronix.de>
> >>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> >>> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> >>> [ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ]
> >>> ---
> >>>  kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 45 insertions(+)
> >>>
> >>> diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> >>> index 3d936f0f..f5cf2bb 100644
> >>> --- a/kernel/rcutorture.c
> >>> +++ b/kernel/rcutorture.c
> >>> @@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
> >>>  static int fqs_holdoff;		/* Hold time within burst (us). */
> >>>  static int fqs_stutter = 3;	/* Wait time between bursts (s). */
> >>>  static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
> >>> +static int object_debug;	/* Test object-debug double call_rcu()?. */
> >>>  static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
> >>>  static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
> >>>  static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */
> >>> @@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
> >>>  MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
> >>>  module_param(n_barrier_cbs, int, 0444);
> >>>  MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
> >>> +module_param(object_debug, int, 0444);
> >>> +MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
> >>>  module_param(onoff_interval, int, 0444);
> >>>  MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
> >>>  module_param(onoff_holdoff, int, 0444);
> >>> @@ -1934,6 +1937,46 @@ rcu_torture_cleanup(void)
> >>>  		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
> >>>  }
> >>>  
> >>> +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> >>> +static void rcu_torture_leak_cb(struct rcu_head *rhp)
> >>> +{
> >>> +}
> >>> +
> >>> +static void rcu_torture_err_cb(struct rcu_head *rhp)
> >>> +{
> >>> +	/* This -might- happen due to race conditions, but is unlikely. */
> >>> +	pr_alert("rcutorture: duplicated callback was invoked.\n");
> >>> +}
> >>> +#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> >>> +
> >>> +/*
> >>> + * Verify that double-free causes debug-objects to complain, but only
> >>> + * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
> >>> + * cannot be carried out.
> >>> + */
> >>> +static void rcu_test_debug_objects(void)
> >>> +{
> >>> +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> >>> +	struct rcu_head rh1;
> >>> +	struct rcu_head rh2;
> >>> +
> >>> +	init_rcu_head_on_stack(&rh1);
> >>> +	init_rcu_head_on_stack(&rh2);
> >>> +	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> >>> +	local_irq_disable(); /* Make it hard to finish grace period. */
> >>
> >> you can use rcu_read_lock() directly.
> > 
> > I could do that as well, but it doesn't do everything that local_irq_disable()
> > does.
> > 
> > Right, which means that my comment is bad.  Fixing both, thank you!
> > 
> >>> +	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
> > 
> > And the one above cannot start a grace period due to irqs being enabled.
> > Which is -almost- always OK, but...
> > 
> >>> +	call_rcu(&rh2, rcu_torture_err_cb);
> > 
> > And this one should invoke rcu_torture_leak_cb instead of
> > rcu_torture_err_cb().  Just results in a confusing error message, but...
> 
> I still don't understand why rcu_torture_err_cb() will be called when:
> 
> rcu_read_lock();
> call_rcu(&rh2, rcu_torture_leak_cb);
> call_rcu(&rh2, rcu_torture_err_cb); // rh2 will be still queued here,
> 				    // debug-objects will find it and
> 				    // change it to rcu_leak_callback()
> rcu_read_unlock();

Fair point, no chance of the second rh2 callback being queued after the
first one is invoked!  I will leave the message.  Whoever sees it with
the current code will have something to tell their grandchildren.

							Thanx, Paul

> > OK, a few more fixes, then!
> > 
> >>> +	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
> >>> +	local_irq_enable();
> >>> +	rcu_barrier();
> >>> +	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> >>> +	destroy_rcu_head_on_stack(&rh1);
> >>> +	destroy_rcu_head_on_stack(&rh2);
> >>> +#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> >>> +	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> >>> +#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> >>> +}
> > 
> > The result is as follows.  Better?
> > 
> > 							Thanx, Paul
> > 
> > #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> > static void rcu_torture_leak_cb(struct rcu_head *rhp)
> > {
> > }
> > 
> > static void rcu_torture_err_cb(struct rcu_head *rhp)
> > {
> > 	/*
> > 	 * This -might- happen due to race conditions, but is unlikely.
> > 	 * The scenario that leads to this happening is that the
> > 	 * first of the pair of duplicate callbacks is queued,
> > 	 * someone else starts a grace period that includes that
> > 	 * callback, then the second of the pair must wait for the
> > 	 * next grace period.  Unlikely, but can happen.  If it
> > 	 * does happen, the debug-objects subsystem won't have splatted.
> > 	 */
> > 	pr_alert("rcutorture: duplicated callback was invoked.\n");
> > }
> > #endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > 
> > /*
> >  * Verify that double-free causes debug-objects to complain, but only
> >  * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
> >  * cannot be carried out.
> >  */
> > static void rcu_test_debug_objects(void)
> > {
> > #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> > 	struct rcu_head rh1;
> > 	struct rcu_head rh2;
> > 
> > 	init_rcu_head_on_stack(&rh1);
> > 	init_rcu_head_on_stack(&rh2);
> > 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> > 	preempt_disable(); /* Prevent preemption from interrupting test. */
> > 	rcu_read_lock(); /* Make it impossible to finish a grace period. */
> > 	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */
> > 	local_irq_disable(); /* Make it harder to start a new grace period. */
> > 	call_rcu(&rh2, rcu_torture_leak_cb);
> > 	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
> > 	local_irq_enable();
> > 	rcu_read_unlock();
> > 	preempt_enable();
> > 	rcu_barrier();
> > 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> > 	destroy_rcu_head_on_stack(&rh1);
> > 	destroy_rcu_head_on_stack(&rh2);
> > #else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > 	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> > #endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > }
> > 
> >>> +
> >>>  static int __init
> >>>  rcu_torture_init(void)
> >>>  {
> >>> @@ -2163,6 +2206,8 @@ rcu_torture_init(void)
> >>>  		firsterr = retval;
> >>>  		goto unwind;
> >>>  	}
> >>> +	if (object_debug)
> >>> +		rcu_test_debug_objects();
> >>>  	rcutorture_record_test_transition();
> >>>  	mutex_unlock(&fullstop_mutex);
> >>>  	return 0;
> >>
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> > 
> 

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
[...]
> The result is as follows.  Better?

Hi Paul,

Pitching in late in the thread, so that I can get a share of the fun ;-)

> 							Thanx, Paul
> 
> #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> static void rcu_torture_leak_cb(struct rcu_head *rhp)
> {
> }
> 
> static void rcu_torture_err_cb(struct rcu_head *rhp)
> {
> 	/*
> 	 * This -might- happen due to race conditions, but is unlikely.
> 	 * The scenario that leads to this happening is that the
> 	 * first of the pair of duplicate callbacks is queued,
> 	 * someone else starts a grace period that includes that
> 	 * callback, then the second of the pair must wait for the
> 	 * next grace period.  Unlikely, but can happen.  If it
> 	 * does happen, the debug-objects subsystem won't have splatted.
> 	 */
> 	pr_alert("rcutorture: duplicated callback was invoked.\n");
> }
> #endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> 

Hrm. Putting an #ifdef within a function when not utterly needed is
usually a bad idea. How about:

/*
 * Verify that double-free causes debug-objects to complain, but only
 * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
 * cannot be carried out.
 */
#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
static void rcu_test_debug_objects(void)
{
 	struct rcu_head rh1;
 	struct rcu_head rh2;
 
 	init_rcu_head_on_stack(&rh1);
 	init_rcu_head_on_stack(&rh2);
 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
 	preempt_disable(); /* Prevent preemption from interrupting test. */
 	rcu_read_lock(); /* Make it impossible to finish a grace period. */
 	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */
 	local_irq_disable(); /* Make it harder to start a new grace period. */
 	call_rcu(&rh2, rcu_torture_leak_cb);
 	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
 	local_irq_enable();
 	rcu_read_unlock();
 	preempt_enable();
 	rcu_barrier();
 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
 	destroy_rcu_head_on_stack(&rh1);
 	destroy_rcu_head_on_stack(&rh2);
}
#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
static void rcu_test_debug_objects(void)
{
 	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
}
#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */


More comments inlined in the code below,

> /*
>  * Verify that double-free causes debug-objects to complain, but only
>  * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
>  * cannot be carried out.
>  */
> static void rcu_test_debug_objects(void)
> {
> #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> 	struct rcu_head rh1;
> 	struct rcu_head rh2;
> 
> 	init_rcu_head_on_stack(&rh1);
> 	init_rcu_head_on_stack(&rh2);
> 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> 	preempt_disable(); /* Prevent preemption from interrupting test. */
> 	rcu_read_lock(); /* Make it impossible to finish a grace period. */
> 	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */

Are we really "starting" a grace period ? If rcu_test_debug_objects() is
executed after some callbacks are already queued, are we, by definition,
"starting" the grace period ?

Also, I find it weird to have, in that order:

1) preempt_disable()
2) rcu_read_lock()
3) local_irq_disable()

I would rather expect:

1) rcu_read_lock()
2) preempt_disable()
3) local_irq_disable()

So they come in increasing order of impact on the system: with
non-preemptable RCU, the read-lock and preempt disable mean the same
thing, however, with preemptable RCU, the impact of preempt disable
seems larger than the impact of RCU read lock: preemption is still
enabled when within a RCU critical section. Both will work, but I find
this call order slightly weird.

Also, if your goal is to increase the chances that call_rcu() enqueues
both callbacks into the same grace period, you might want to issue a
rcu_barrier() early in this function, so that call_rcu() has even more
chances to enqueue the callbacks into the same grace period.

However, if you care about testing enqueue into same _and_ different
grace periods, you might want to turn this single-shot test into a
stress-test by calling it repeatedly.

Thanks!

Mathieu

> 	local_irq_disable(); /* Make it harder to start a new grace period. */
> 	call_rcu(&rh2, rcu_torture_leak_cb);
> 	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
> 	local_irq_enable();
> 	rcu_read_unlock();
> 	preempt_enable();
> 	rcu_barrier();
> 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> 	destroy_rcu_head_on_stack(&rh1);
> 	destroy_rcu_head_on_stack(&rh2);
> #else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> 	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> #endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> }
> 
> > > +
> > >  static int __init
> > >  rcu_torture_init(void)
> > >  {
> > > @@ -2163,6 +2206,8 @@ rcu_torture_init(void)
> > >  		firsterr = retval;
> > >  		goto unwind;
> > >  	}
> > > +	if (object_debug)
> > > +		rcu_test_debug_objects();
> > >  	rcutorture_record_test_transition();
> > >  	mutex_unlock(&fullstop_mutex);
> > >  	return 0;
> > 
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture

[Paul E. McKenney]

On Sat, Aug 24, 2013 at 03:25:36PM -0400, Mathieu Desnoyers wrote:
> * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> [...]
> > The result is as follows.  Better?
> 
> Hi Paul,
> 
> Pitching in late in the thread, so that I can get a share of the fun ;-)
> 
> > 							Thanx, Paul
> > 
> > #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> > static void rcu_torture_leak_cb(struct rcu_head *rhp)
> > {
> > }
> > 
> > static void rcu_torture_err_cb(struct rcu_head *rhp)
> > {
> > 	/*
> > 	 * This -might- happen due to race conditions, but is unlikely.
> > 	 * The scenario that leads to this happening is that the
> > 	 * first of the pair of duplicate callbacks is queued,
> > 	 * someone else starts a grace period that includes that
> > 	 * callback, then the second of the pair must wait for the
> > 	 * next grace period.  Unlikely, but can happen.  If it
> > 	 * does happen, the debug-objects subsystem won't have splatted.
> > 	 */
> > 	pr_alert("rcutorture: duplicated callback was invoked.\n");
> > }
> > #endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > 
> 
> Hrm. Putting an #ifdef within a function when not utterly needed is
> usually a bad idea. How about:
> 
> /*
>  * Verify that double-free causes debug-objects to complain, but only
>  * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
>  * cannot be carried out.
>  */
> #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> static void rcu_test_debug_objects(void)
> {
>  	struct rcu_head rh1;
>  	struct rcu_head rh2;
> 
>  	init_rcu_head_on_stack(&rh1);
>  	init_rcu_head_on_stack(&rh2);
>  	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
>  	preempt_disable(); /* Prevent preemption from interrupting test. */
>  	rcu_read_lock(); /* Make it impossible to finish a grace period. */
>  	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */
>  	local_irq_disable(); /* Make it harder to start a new grace period. */
>  	call_rcu(&rh2, rcu_torture_leak_cb);
>  	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
>  	local_irq_enable();
>  	rcu_read_unlock();
>  	preempt_enable();
>  	rcu_barrier();
>  	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
>  	destroy_rcu_head_on_stack(&rh1);
>  	destroy_rcu_head_on_stack(&rh2);
> }
> #else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> static void rcu_test_debug_objects(void)
> {
>  	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> }
> #endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */

The objection to this is that it duplicates the function header, both
copies of which must be updated.  See Josh's and my discussion on this
point earlier in the thread.  Who knows, Josh might eventually convince
me to individually ifdef the functions in kernel/rcutree_plugin.h,
but I am not quite there yet.  ;-)

That said, I do see two benefits of individual ifdef:

1.	It is easy to see when a given function is included.
	As it is, there are runs of many hundreds of lines of
	code where it might not be obvious to the casual reader.

2.	Duplicated code is asking for silent rebase and merge errors.
	This can happen when a change to the function header collides
	with a change that duplicates the function under ifdef.

So perhaps I will eventually convert to the individual-ifdef style.
At the moment, I am in no hurry.

> More comments inlined in the code below,
> 
> > /*
> >  * Verify that double-free causes debug-objects to complain, but only
> >  * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
> >  * cannot be carried out.
> >  */
> > static void rcu_test_debug_objects(void)
> > {
> > #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> > 	struct rcu_head rh1;
> > 	struct rcu_head rh2;
> > 
> > 	init_rcu_head_on_stack(&rh1);
> > 	init_rcu_head_on_stack(&rh2);
> > 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> > 	preempt_disable(); /* Prevent preemption from interrupting test. */
> > 	rcu_read_lock(); /* Make it impossible to finish a grace period. */
> > 	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */
> 
> Are we really "starting" a grace period ? If rcu_test_debug_objects() is
> executed after some callbacks are already queued, are we, by definition,
> "starting" the grace period ?
> 
> Also, I find it weird to have, in that order:
> 
> 1) preempt_disable()
> 2) rcu_read_lock()
> 3) local_irq_disable()
> 
> I would rather expect:
> 
> 1) rcu_read_lock()
> 2) preempt_disable()
> 3) local_irq_disable()
> 
> So they come in increasing order of impact on the system: with
> non-preemptable RCU, the read-lock and preempt disable mean the same
> thing, however, with preemptable RCU, the impact of preempt disable
> seems larger than the impact of RCU read lock: preemption is still
> enabled when within a RCU critical section. Both will work, but I find
> this call order slightly weird.

If this was code that ran normally, I would agree with you.  Your
suggested ordering would reduce the scheduling-latency impact due to
this code, among other things.

However, this code is a one-off that runs only once during an rcutorture
run that explicitly calls for it using rcutorture's new object_debug
module parameter.

> Also, if your goal is to increase the chances that call_rcu() enqueues
> both callbacks into the same grace period, you might want to issue a
> rcu_barrier() early in this function, so that call_rcu() has even more
> chances to enqueue the callbacks into the same grace period.

The problem is that neither rcu_barrier() nor synchronize_rcu() clean
up at a well-defined point in time.  In both cases, the task will be
awakened at some time after the operation completes, by which time
any number of new grace periods might have started and completed.

> However, if you care about testing enqueue into same _and_ different
> grace periods, you might want to turn this single-shot test into a
> stress-test by calling it repeatedly.

Good point, and I might do that.  However, the above code has not yet
failed to produce a debug-objects splat.  When and if it does fail to
splat, then running the code repeatedly would definitely be one of the
fixes that I would consider.  (Another potential fix being to simply load
the rcutorture module a few times in succession, though your suggestion
would in fact work better for my current testing setup.)

Thank you for your review and comments!

							Thanx, Paul

> Thanks!
> 
> Mathieu
> 
> > 	local_irq_disable(); /* Make it harder to start a new grace period. */
> > 	call_rcu(&rh2, rcu_torture_leak_cb);
> > 	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
> > 	local_irq_enable();
> > 	rcu_read_unlock();
> > 	preempt_enable();
> > 	rcu_barrier();
> > 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
> > 	destroy_rcu_head_on_stack(&rh1);
> > 	destroy_rcu_head_on_stack(&rh2);
> > #else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > 	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
> > #endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> > }
> > 
> > > > +
> > > >  static int __init
> > > >  rcu_torture_init(void)
> > > >  {
> > > > @@ -2163,6 +2206,8 @@ rcu_torture_init(void)
> > > >  		firsterr = retval;
> > > >  		goto unwind;
> > > >  	}
> > > > +	if (object_debug)
> > > > +		rcu_test_debug_objects();
> > > >  	rcutorture_record_test_transition();
> > > >  	mutex_unlock(&fullstop_mutex);
> > > >  	return 0;
> > > 
> > 
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
>