rcutorture and documentation patches for 3.6

From: "Paul E. McKenney" <[email protected]>

Move the raw SRCU interfaces out of the middle of the normal SRCU interfaces.

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutorture.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

kernel/rcutorture.c +2 -2

index e66b34a..9850479 100644
@@ -1908,8 +1908,8 @@ rcu_torture_init(void)
 	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_raw_ops,
-		  &srcu_raw_sync_ops, &srcu_expedited_ops,
+		  &srcu_ops, &srcu_sync_ops, &srcu_expedited_ops,
+		  &srcu_raw_ops, &srcu_raw_sync_ops,
 		  &sched_ops, &sched_sync_ops, &sched_expedited_ops, };
 
 	mutex_lock(&fullstop_mutex);

Signature · 1 line

1.7.8

From: "Paul E. McKenney" <[email protected]>

SRCU now has a call_srcu() and an srcu_barrier(), but rcutorture does not test srcu_barrier(). This commit adds the machinery to allow rcutorture's existing rcu_barrier() tests to apply to SRCU.

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutorture.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

kernel/rcutorture.c +13 -2

index 9850479..7b6935e 100644
@@ -635,6 +635,17 @@ static void srcu_torture_synchronize(void)
 	synchronize_srcu(&srcu_ctl);
 }
 
+static void srcu_torture_call(struct rcu_head *head,
+			      void (*func)(struct rcu_head *head))
+{
+	call_srcu(&srcu_ctl, head, func);
+}
+
+static void srcu_torture_barrier(void)
+{
+	srcu_barrier(&srcu_ctl);
+}
+
 static int srcu_torture_stats(char *page)
 {
 	int cnt = 0;

@@ -661,8 +672,8 @@ static struct rcu_torture_ops srcu_ops = {
 	.completed	= srcu_torture_completed,
 	.deferred_free	= srcu_torture_deferred_free,
 	.sync		= srcu_torture_synchronize,
-	.call		= NULL,
-	.cb_barrier	= NULL,
+	.call		= srcu_torture_call,
+	.cb_barrier	= srcu_torture_barrier,
 	.stats		= srcu_torture_stats,
 	.name		= "srcu"
 };

Signature · 1 line

1.7.8

kernel/rcutorture.c +1 -0

@@ -1642,6 +1643,7 @@ void rcu_torture_barrier_cbf(struct rcu_head *rcu)
 static int rcu_torture_barrier_cbs(void *arg)
 {
 	long myid = (long)arg;
+	bool lastphase = 0;
 	struct rcu_head rcu;
 
 	init_rcu_head_on_stack(&rcu);

kernel/rcutorture.c +2 -1

 		if (kthread_should_stop() || fullstop != FULLSTOP_DONTSTOP)
 			break;
 		cur_ops->call(&rcu, rcu_torture_barrier_cbf);

@@ -1676,7 +1680,8 @@ static int rcu_torture_barrier(void *arg)
 	do {
 		atomic_set(&barrier_cbs_invoked, 0);
 		atomic_set(&barrier_cbs_count, n_barrier_cbs);
-		/* wake_up() path contains the required barriers. */
+		smp_mb();
+		barrier_phase = !barrier_phase;
 		for (i = 0; i < n_barrier_cbs; i++)
 			wake_up(&barrier_cbs_wq[i]);
 		wait_event(barrier_wq,

Signature · 1 line

1.7.8

* Paul E. McKenney ([email protected]) wrote:
[...]

Quoted 9 lines

kernel/rcutorture.c +1 -0

@@ -1642,6 +1643,7 @@ void rcu_torture_barrier_cbf(struct rcu_head *rcu)
 static int rcu_torture_barrier_cbs(void *arg)
 {
 	long myid = (long)arg;
+	bool lastphase = 0;
 	struct rcu_head rcu;
 
 	init_rcu_head_on_stack(&rcu);

kernel/rcutorture.c static int rcu_torture_barrier_cbs(void *arg) · line 1658

Quoted 9 lines

kernel/rcutorture.c +3 -1

@@ -1649,9 +1651,11 @@ static int rcu_torture_barrier_cbs(void *arg)
 	set_user_nice(current, 19);
 	do {
 		wait_event(barrier_cbs_wq[myid],
-			   atomic_read(&barrier_cbs_count) == n_barrier_cbs ||
+			   barrier_phase != lastphase ||
 			   kthread_should_stop() ||
 			   fullstop != FULLSTOP_DONTSTOP);
+		lastphase = barrier_phase;
+		smp_mb();

Hi Paul,

Documenting this barrier, and the barrier below, along with the variable accesses they order, would appear to be a good idea, especially since the bug this is correcting was caused by the lack of appropriate smp_mb().

Thanks,

Mathieu

Quoted 16 lines

kernel/rcutorture.c +2 -1

 		if (kthread_should_stop() || fullstop != FULLSTOP_DONTSTOP)
 			break;
 		cur_ops->call(&rcu, rcu_torture_barrier_cbf);

@@ -1676,7 +1680,8 @@ static int rcu_torture_barrier(void *arg)
 	do {
 		atomic_set(&barrier_cbs_invoked, 0);
 		atomic_set(&barrier_cbs_count, n_barrier_cbs);
-		/* wake_up() path contains the required barriers. */
+		smp_mb();
+		barrier_phase = !barrier_phase;
 		for (i = 0; i < n_barrier_cbs; i++)
 			wake_up(&barrier_cbs_wq[i]);
 		wait_event(barrier_wq,

Signature · 1 line

1.7.8

Signature · 4 lines

Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

From: "Paul E. McKenney" <[email protected]>

The rcu_torture_barrier() function has a copy-and-paste typo in the string passed to rcutorture_shutdown_absorb(), which this commit fixes.

Signed-off-by Paul E. McKenney <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutorture.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

kernel/rcutorture.c +1 -1

index 08d8537..54a3745 100644
@@ -1700,7 +1700,7 @@ static int rcu_torture_barrier(void *arg)
 		schedule_timeout_interruptible(HZ / 10);
 	} while (!kthread_should_stop() && fullstop == FULLSTOP_DONTSTOP);
 	VERBOSE_PRINTK_STRING("rcu_torture_barrier task stopping");
-	rcutorture_shutdown_absorb("rcu_torture_barrier_cbs");
+	rcutorture_shutdown_absorb("rcu_torture_barrier");
 	while (!kthread_should_stop())
 		schedule_timeout_interruptible(1);
 	return 0;

Signature · 1 line

1.7.8

From: "Paul E. McKenney" <[email protected]>

The current rcutorture rcu_barrier() testing never intentionally runs more than one instance of rcu_barrier() at a given time. This fails to test the the shiny new concurrency features of rcu_barrier(). This commit therefore modifies the rcutorture fakewriter kthread to randomly invoke rcu_barrier() rather than the usual synchronize_rcu().

Signed-off-by Paul E. McKenney <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutorture.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

kernel/rcutorture.c +0 -0

index 54a3745..dfb4e20 100644

Quoted 19 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

The current rcutorture rcu_barrier() testing never intentionally runs more than one instance of rcu_barrier() at a given time. This fails to test the the shiny new concurrency features of rcu_barrier(). This commit therefore modifies the rcutorture fakewriter kthread to randomly invoke rcu_barrier() rather than the usual synchronize_rcu().

Signed-off-by Paul E. McKenney <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutorture.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

kernel/rcutorture.c +0 -0

index 54a3745..dfb4e20 100644

kernel/rcutorture.c rcu_torture_fakewriter(void *arg) · line 1029

Quoted 6 lines

kernel/rcutorture.c +2 -1

@@ -1025,7 +1025,11 @@ rcu_torture_fakewriter(void *arg)
 	do {
 		schedule_timeout_uninterruptible(1 + rcu_random(&rand)%10);
 		udelay(rcu_random(&rand) & 0x3ff);
-		cur_ops->sync();
+		if (cur_ops->cb_barrier != NULL &&
+		    rcu_random(&rand) % (NR_CPUS * 8) == 0)

NR_CPUS seems like an odd choice here. I assume you want to control for having many rcu_torture_fakewriter threads, and aim for the same average rate of barrier calls across the whole set of threads regardless of the number of threads. However, NR_CPUS does not accurately reflect either the number of fakewriter threads (which a user can set arbitrarily) or the number of CPUs currently on the system (since NR_CPUS represents the compile-time limit). I'd suggest changing this to use the actual number of fakewriter threads, which rcutorture knows at start time.

Quoted 27 lines · Josh Triplett

Quoted 19 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

The current rcutorture rcu_barrier() testing never intentionally runs more than one instance of rcu_barrier() at a given time. This fails to test the the shiny new concurrency features of rcu_barrier(). This commit therefore modifies the rcutorture fakewriter kthread to randomly invoke rcu_barrier() rather than the usual synchronize_rcu().

Signed-off-by Paul E. McKenney <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutorture.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

kernel/rcutorture.c +0 -0

index 54a3745..dfb4e20 100644

kernel/rcutorture.c rcu_torture_fakewriter(void *arg) · line 1029

Quoted 6 lines

kernel/rcutorture.c +2 -1

@@ -1025,7 +1025,11 @@ rcu_torture_fakewriter(void *arg)
 	do {
 		schedule_timeout_uninterruptible(1 + rcu_random(&rand)%10);
 		udelay(rcu_random(&rand) & 0x3ff);
-		cur_ops->sync();
+		if (cur_ops->cb_barrier != NULL &&
+		    rcu_random(&rand) % (NR_CPUS * 8) == 0)

Quoted 8 lines

NR_CPUS seems like an odd choice here. I assume you want to control for having many rcu_torture_fakewriter threads, and aim for the same average rate of barrier calls across the whole set of threads regardless of the number of threads. However, NR_CPUS does not accurately reflect either the number of fakewriter threads (which a user can set arbitrarily) or the number of CPUs currently on the system (since NR_CPUS represents the compile-time limit). I'd suggest changing this to use the actual number of fakewriter threads, which rcutorture knows at start time.

Indeed, this should use the number of online CPUs. Which should be easy to compute, will fix.

Quoted 37 lines · Paul E. McKenney

Quoted 35 lines · Josh Triplett

Quoted 19 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

The current rcutorture rcu_barrier() testing never intentionally runs more than one instance of rcu_barrier() at a given time. This fails to test the the shiny new concurrency features of rcu_barrier(). This commit therefore modifies the rcutorture fakewriter kthread to randomly invoke rcu_barrier() rather than the usual synchronize_rcu().

Signed-off-by Paul E. McKenney <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutorture.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

kernel/rcutorture.c +0 -0

index 54a3745..dfb4e20 100644

kernel/rcutorture.c rcu_torture_fakewriter(void *arg) · line 1029

Quoted 6 lines

kernel/rcutorture.c +2 -1

@@ -1025,7 +1025,11 @@ rcu_torture_fakewriter(void *arg)
 	do {
 		schedule_timeout_uninterruptible(1 + rcu_random(&rand)%10);
 		udelay(rcu_random(&rand) & 0x3ff);
-		cur_ops->sync();
+		if (cur_ops->cb_barrier != NULL &&
+		    rcu_random(&rand) % (NR_CPUS * 8) == 0)

NR_CPUS seems like an odd choice here. I assume you want to control for having many rcu_torture_fakewriter threads, and aim for the same average rate of barrier calls across the whole set of threads regardless of the number of threads. However, NR_CPUS does not accurately reflect either the number of fakewriter threads (which a user can set arbitrarily) or the number of CPUs currently on the system (since NR_CPUS represents the compile-time limit). I'd suggest changing this to use the actual number of fakewriter threads, which rcutorture knows at start time.

Quoted 2 lines

Indeed, this should use the number of online CPUs. Which should be easy to compute, will fix.

I'd suggest using nfakewriters instead.

Documentation/RCU/rcubarrier.txt Answers to Quick Quizzes · line 267

Quoted 12 lines · Paul E. McKenney

Documentation/RCU/rcubarrier.txt +4 -2

@@ -261,8 +261,10 @@ Answers to Quick Quizzes
 
 Quick Quiz #1: Why is there no srcu_barrier()?
 
-Answer: Since there is no call_srcu(), there can be no outstanding SRCU
-	callbacks. Therefore, there is no need to wait for them.
+Answer: There really is an srcu_barrier() now that there is a call_srcu().
+	Before call_srcu(), there were no SRCU callbacks, and there was
+	therefore no need to wait for them, and therefore there was
+	no srcu_barrier().

I don't think it makes sense to leave this question in its current form and just change the answer. It seems like a trick question now. :)

I'd suggest just dropping the question and renumbering the remaining questions.