[PATCH] Force rcutorture tasks to spread over CPUs

[PATCH] Force rcutorture tasks to spread over CPUs

[Paul E. McKenney]

Of late, the scheduler seems to have decided to make things too easy for
RCU -- on some configurations, all of the rcutorture tasks end up on the
same CPU, which doesn't do a very good job of torturing RCU.  This patch
helps the scheduler spread these tasks out by forcing a 20-millisecond
burst of CPU-bound execution on each of rcutorture's tasks, which seems
to work reasonably well in practice.

My challenge for those working on the scheduler is to make this patch
unnecessary.  ;-)

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---

 rcutorture.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff -urpNa -X dontdiff linux-2.6.21.4-rt13/kernel/rcutorture.c linux-2.6.21.4-rt13-rcutorturespread/kernel/rcutorture.c
--- linux-2.6.21.4-rt13/kernel/rcutorture.c	2007-06-12 09:19:02.000000000 -0700
+++ linux-2.6.21.4-rt13-rcutorturespread/kernel/rcutorture.c	2007-06-12 21:05:05.000000000 -0700
@@ -102,6 +102,8 @@ struct rcu_torture {
 };
 
 static int fullstop = 0;	/* stop generating callbacks at test end. */
+static int startwriters;	/* force load-balancing of writers. */
+static int startreaders;	/* force load-balancing of readers. */
 static LIST_HEAD(rcu_torture_freelist);
 static struct rcu_torture *rcu_torture_current = NULL;
 static long rcu_torture_current_version = 0;
@@ -525,6 +527,8 @@ rcu_torture_writer(void *arg)
 	static DEFINE_RCU_RANDOM(rand);
 
 	VERBOSE_PRINTK_STRING("rcu_torture_writer task started");
+	while (!startwriters)
+		barrier();	/* Force scheduler to spread over CPUs. */
 	set_user_nice(current, 19);
 	current->flags |= PF_NOFREEZE;
 
@@ -565,6 +569,8 @@ rcu_torture_fakewriter(void *arg)
 	DEFINE_RCU_RANDOM(rand);
 
 	VERBOSE_PRINTK_STRING("rcu_torture_fakewriter task started");
+	while (!startwriters)
+		barrier();	/* Force scheduler to spread over CPUs. */
 	set_user_nice(current, 19);
 	current->flags |= PF_NOFREEZE;
 
@@ -596,6 +602,8 @@ rcu_torture_reader(void *arg)
 	int pipe_count;
 
 	VERBOSE_PRINTK_STRING("rcu_torture_reader task started");
+	while (!startreaders)
+		barrier();	/* Force scheduler to spread over CPUs. */
 	set_user_nice(current, 19);
 	current->flags |= PF_NOFREEZE;
 
@@ -929,6 +937,8 @@ rcu_torture_init(void)
 
 	/* Start up the kthreads. */
 
+	startwriters = 0;
+	barrier();
 	VERBOSE_PRINTK_STRING("Creating rcu_torture_writer task");
 	writer_task = kthread_run(rcu_torture_writer, NULL,
 				  "rcu_torture_writer");
@@ -956,6 +966,12 @@ rcu_torture_init(void)
 			goto unwind;
 		}
 	}
+	barrier();
+	startwriters = 1;
+	schedule_timeout_interruptible(round_jiffies_relative(HZ/50));
+
+	startreaders = 0;
+	barrier();
 	reader_tasks = kzalloc(nrealreaders * sizeof(reader_tasks[0]),
 			       GFP_KERNEL);
 	if (reader_tasks == NULL) {
@@ -974,6 +990,10 @@ rcu_torture_init(void)
 			goto unwind;
 		}
 	}
+	schedule_timeout_interruptible(round_jiffies_relative(HZ/50));
+	barrier();
+	startreaders = 1;
+
 	if (stat_interval > 0) {
 		VERBOSE_PRINTK_STRING("Creating rcu_torture_stats task");
 		stats_task = kthread_run(rcu_torture_stats, NULL,

Re: [PATCH] Force rcutorture tasks to spread over CPUs

[Josh Triplett]

Paul E. McKenney wrote:
> Of late, the scheduler seems to have decided to make things too easy for
> RCU -- on some configurations, all of the rcutorture tasks end up on the
> same CPU, which doesn't do a very good job of torturing RCU.  This patch
> helps the scheduler spread these tasks out by forcing a 20-millisecond
> burst of CPU-bound execution on each of rcutorture's tasks, which seems
> to work reasonably well in practice.

Given the scheduler behavior you observed, this seems reasonable to me, and it
seems reasonable to do this balancing by default.  You might consider adding a
module param to set the timeout, and disable the barrier code entirely when 0,
so people can still test the previous behavior.

Independently from this change, you might also consider adding a way to set
CPU affinity for rcutorture threads.  This would allow explicitly spreading
readers and writers across all CPUs, and possibly also allow clumping these
threads together on CPUs in bug-revealing ways.

> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Acked-by: Josh Triplett <josh@kernel.org>

- Josh Triplett

Re: [PATCH] Force rcutorture tasks to spread over CPUs

[Andrew Morton]

On Tue, 12 Jun 2007 21:28:04 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> +	while (!startwriters)
> +		barrier();	/* Force scheduler to spread over CPUs. */

one wonders whether a cpu_relax() would be a bit nicer here.  That implicitly
does a barrier().

This patch doesn't make much sense for non-SMP builds?

Re: [PATCH] Force rcutorture tasks to spread over CPUs

[Ingo Molnar]

* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 12 Jun 2007 21:28:04 -0700
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> 
> > +	while (!startwriters)
> > +		barrier();	/* Force scheduler to spread over CPUs. */
> 
> one wonders whether a cpu_relax() would be a bit nicer here.  That 
> implicitly does a barrier().
> 
> This patch doesn't make much sense for non-SMP builds?

i think this patch should be unnecessary because we found the real SMP 
balancing bug in the upstream scheduler causing this rcu problem, see:

 commit 92c4ca5c3a5e180e9762438db235f41d192cb955
 Author: Christoph Lameter <clameter@sgi.com>
 Date:   Sat Jun 23 17:16:33 2007 -0700

     sched: fix next_interval determination in idle_balance()

	Ingo

Re: [PATCH] Force rcutorture tasks to spread over CPUs

[Paul E. McKenney]

On Wed, Jun 27, 2007 at 08:30:55AM +0200, Ingo Molnar wrote:
> 
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Tue, 12 Jun 2007 21:28:04 -0700
> > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > 
> > > +	while (!startwriters)
> > > +		barrier();	/* Force scheduler to spread over CPUs. */
> > 
> > one wonders whether a cpu_relax() would be a bit nicer here.  That 
> > implicitly does a barrier().
> > 
> > This patch doesn't make much sense for non-SMP builds?
> 
> i think this patch should be unnecessary because we found the real SMP 
> balancing bug in the upstream scheduler causing this rcu problem, see:
> 
>  commit 92c4ca5c3a5e180e9762438db235f41d192cb955
>  Author: Christoph Lameter <clameter@sgi.com>
>  Date:   Sat Jun 23 17:16:33 2007 -0700
> 
>      sched: fix next_interval determination in idle_balance()

Ingo is correct -- applying the above patch caused the scheduler to
correctly balance the rcutorture tasks, so that my patch to rcutorture
is no longer needed.  Which is a very good thing!  ;-)

							Thanx, Paul