Messages in this thread | | | Date | Tue, 15 Nov 2011 13:46:15 -0800 | From | Josh Triplett <> | Subject | Re: [PATCH tip/core/rcu 2/9] rcu: Add rcutorture system-shutdown capability |
| |
On Tue, Nov 15, 2011 at 12:27:58PM -0800, Paul E. McKenney wrote: > From: Paul E. McKenney <paul.mckenney@linaro.org> > > Although it is easy to run rcutorture tests under KVM, there is currently > no nice way to run such a test for a fixed time period, collect all of > the rcutorture data, and then shut the system down cleanly. This commit > therefore adds an rcutorture module parameter named "shutdown_secs" that > specified the run duration in seconds, after which rcutorture terminates > the test and powers the system down. The default value for "shutdown_secs" > is zero, which disables shutdown. > > Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
From your recent post on this, I thought you found a solution through the init= parameter, which seems preferable.
> --- a/kernel/rcutorture.c > +++ b/kernel/rcutorture.c > @@ -61,9 +61,10 @@ static int test_no_idle_hz; /* Test RCU's 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 = 0; /* Duration of bursts (us), 0 to disable. */ > -static int fqs_holdoff = 0; /* Hold time within burst (us). */ > +static int fqs_duration; /* Duration of bursts (us), 0 to disable. */ > +static int fqs_holdoff; /* Hold time within burst (us). */
Looks like these lines picked up unrelated whitespace changes in this commit.
> @@ -1305,6 +1313,37 @@ static int rcutorture_booster_init(int cpu) > return 0; > } > > +/* > + * Cause the rcutorture test to "stutter", starting and stopping all > + * threads periodically. > + */
This comment looks like a copy-paste error.
> +static int > +rcu_torture_shutdown(void *arg) > +{ > + VERBOSE_PRINTK_STRING("rcu_torture_shutdown task started"); > + while (ULONG_CMP_LT(jiffies, shutdown_time) && > + !kthread_should_stop()) { > + if (verbose) > + printk(KERN_ALERT "%s" TORTURE_FLAG > + "rcu_torture_shutdown task: %lu " > + "jiffies remaining\n", > + torture_type, shutdown_time - jiffies); > + schedule_timeout_interruptible(HZ); > + }
Any particular reason to wake up once a second here? If !verbose, this could just sleep until shutdown time. (And does the verbose output really help here, given printk timestamps?)
> + if (ULONG_CMP_LT(jiffies, shutdown_time)) { > + VERBOSE_PRINTK_STRING("rcu_torture_shutdown task stopping"); > + return 0; > + } > + > + /* OK, shut down the system. */ > + > + VERBOSE_PRINTK_STRING("rcu_torture_shutdown task shutting down system"); > + shutdown_task = NULL; /* Avoid self-kill deadlock. */
Not that it matters much here, but won't this cause a leak?
> + rcu_torture_cleanup(); /* Get the success/failure message. */ > + kernel_power_off(); /* Shut down the system. */ > + return 0; > +}
Huh. I would have expected kernel_power_off to use noreturn, making the return 0 unnecessary here; however, apparently it doesn't.
- Josh Triplett
| |