linux-kernel - Re: [PATCH tip/core/rcu 2/9] rcu: Add rcutorture system-shutdown capability

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111115214615.GD31473@leaf>
Date:	Tue, 15 Nov 2011 13:46:15 -0800
From:	Josh Triplett <josh@...htriplett.org>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...e.hu, laijs@...fujitsu.com,
	dipankar@...ibm.com, akpm@...ux-foundation.org,
	mathieu.desnoyers@...ymtl.ca, niv@...ibm.com, tglx@...utronix.de,
	peterz@...radead.org, rostedt@...dmis.org, Valdis.Kletnieks@...edu,
	dhowells@...hat.com, eric.dumazet@...il.com, darren@...art.com,
	patches@...aro.org, "Paul E. McKenney" <paul.mckenney@...aro.org>
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@...aro.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@...aro.org>
> Signed-off-by: Paul E. McKenney <paulmck@...ux.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/