[tip/core/rcu,6/6] rcu: Make rcutorture fakewriters invoke rcu_barrier()

Message ID 1339786674-25265-6-git-send-email-paulmck@linux.vnet.ibm.com
State New
Headers show

Commit Message

Paul E. McKenney June 15, 2012, 6:57 p.m.
From: "Paul E. McKenney" <paul.mckenney@linaro.org>

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 <paul.mckenney@linaro.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutorture.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

Comments

Josh Triplett June 15, 2012, 8:37 p.m. | #1
On Fri, Jun 15, 2012 at 11:57:54AM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paul.mckenney@linaro.org>
> 
> 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 <paul.mckenney@linaro.org>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>  kernel/rcutorture.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> index 54a3745..dfb4e20 100644
> --- a/kernel/rcutorture.c
> +++ b/kernel/rcutorture.c
> @@ -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.

- Josh Triplett
Paul E. McKenney June 15, 2012, 9:19 p.m. | #2
On Fri, Jun 15, 2012 at 01:37:05PM -0700, Josh Triplett wrote:
> On Fri, Jun 15, 2012 at 11:57:54AM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paul.mckenney@linaro.org>
> > 
> > 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 <paul.mckenney@linaro.org>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> >  kernel/rcutorture.c |    6 +++++-
> >  1 files changed, 5 insertions(+), 1 deletions(-)
> > 
> > diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> > index 54a3745..dfb4e20 100644
> > --- a/kernel/rcutorture.c
> > +++ b/kernel/rcutorture.c
> > @@ -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.

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

Also will fix the commit message as suggested, and apply your reviewed-bys
as specified.

							Thanx, Paul
Josh Triplett June 15, 2012, 9:52 p.m. | #3
On Fri, Jun 15, 2012 at 02:19:02PM -0700, Paul E. McKenney wrote:
> On Fri, Jun 15, 2012 at 01:37:05PM -0700, Josh Triplett wrote:
> > On Fri, Jun 15, 2012 at 11:57:54AM -0700, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <paul.mckenney@linaro.org>
> > > 
> > > 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 <paul.mckenney@linaro.org>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > ---
> > >  kernel/rcutorture.c |    6 +++++-
> > >  1 files changed, 5 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> > > index 54a3745..dfb4e20 100644
> > > --- a/kernel/rcutorture.c
> > > +++ b/kernel/rcutorture.c
> > > @@ -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.
> 
> Indeed, this should use the number of online CPUs.  Which should be
> easy to compute, will fix.

I'd suggest using nfakewriters instead.

- Josh Triplett
Paul E. McKenney June 15, 2012, 11:48 p.m. | #4
On Fri, Jun 15, 2012 at 02:52:48PM -0700, Josh Triplett wrote:
> On Fri, Jun 15, 2012 at 02:19:02PM -0700, Paul E. McKenney wrote:
> > On Fri, Jun 15, 2012 at 01:37:05PM -0700, Josh Triplett wrote:
> > > On Fri, Jun 15, 2012 at 11:57:54AM -0700, Paul E. McKenney wrote:
> > > > From: "Paul E. McKenney" <paul.mckenney@linaro.org>
> > > > 
> > > > 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 <paul.mckenney@linaro.org>
> > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > ---
> > > >  kernel/rcutorture.c |    6 +++++-
> > > >  1 files changed, 5 insertions(+), 1 deletions(-)
> > > > 
> > > > diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> > > > index 54a3745..dfb4e20 100644
> > > > --- a/kernel/rcutorture.c
> > > > +++ b/kernel/rcutorture.c
> > > > @@ -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.
> > 
> > Indeed, this should use the number of online CPUs.  Which should be
> > easy to compute, will fix.
> 
> I'd suggest using nfakewriters instead.

Right...  Fixed, thank you!!!

							Thanx, Paul

Patch

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 54a3745..dfb4e20 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -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)
+			cur_ops->cb_barrier();
+		else
+			cur_ops->sync();
 		rcu_stutter_wait("rcu_torture_fakewriter");
 	} while (!kthread_should_stop() && fullstop == FULLSTOP_DONTSTOP);