| Message ID | 1339786674-25265-6-git-send-email-paulmck@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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);