linux-kernel - Re: [PATCH] torture: use ktime

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]

Date:	Mon, 20 Jun 2016 21:29:56 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	paulmck@...ux.vnet.ibm.com
Cc:	Boqun Feng <boqun.feng@...il.com>,
	Josh Triplett <josh@...htriplett.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] torture: use ktime_t consistently

On Monday, June 20, 2016 11:37:57 AM CEST Paul E. McKenney wrote:
> On Mon, Jun 20, 2016 at 08:29:48PM +0200, Arnd Bergmann wrote:
> > On Monday, June 20, 2016 11:21:05 AM CEST Paul E. McKenney wrote:
> > > On Mon, Jun 20, 2016 at 05:56:40PM +0200, Arnd Bergmann wrote:
> > >  
> > > @@ -446,9 +447,9 @@ EXPORT_SYMBOL_GPL(torture_shuffle_cleanup);
> > >   * Variables for auto-shutdown.  This allows "lights out" torture runs
> > >   * to be fully scripted.
> > >   */
> > > -static int shutdown_secs;		/* desired test duration in seconds. */
> > > +static ktime_t shutdown_ms;		/* desired test duration in seconds. */
> > 
> > the variable name is a bit odd.
> 
> The comment is certainly now wrong, good catch!
> 
> If there was an s_to_ktime(), I would have kept the old name, but I could
> not find one.  Possibly due to me being blind...

I used "ktime_set(ssecs, 0)", which is almost what you want. Given that
the majority of users of ktime_set() actually pass a zero nanoseconds portion,
it would probably be nice to add secs_to_ktime() as well.

> > > @@ -511,10 +513,10 @@ int torture_shutdown_init(int ssecs, void (*cleanup)(void))
> > >  {
> > >  	int ret = 0;
> > >  
> > > -	shutdown_secs = ssecs;
> > >  	torture_shutdown_hook = cleanup;
> > > -	if (shutdown_secs > 0) {
> > > -		shutdown_time = jiffies + shutdown_secs * HZ;
> > > +	if (ssecs > 0) {
> > > +		shutdown_ms = ms_to_ktime(ssecs * 1000ULL);
> > > +		shutdown_time = ktime_add(ktime_get(), shutdown_ms);
> > >  		ret = torture_create_kthread(torture_shutdown, NULL,
> > >  					     shutdown_task);
> > 
> > and I picked ktime_set(ssecs, 0) instead of ms_to_ktime(ssecs * 1000ULL), but
> > both differences are just cosmetic and should end up in exactly the
> > same object code that I suggested. Unless we both made the same mistake,
> > your version should be good too.
> 
> Thank you for looking it over!  My I apply your Acked-by:, Reviewed-by:,
> or some such?

I had not looked over the original changes before, but have done that now,
please add my 

Acked-by: Arnd Bergmann <arnd@...db.de>

I found one small detail that you could change: instead of using
HRTIMER_MODE_REL, you could actually use HRTIME_MODE_ABS and just
pass the end time instead of computing the difference every time.

You still need to take the difference for printing, but you could
do that in place then:

           if (verbose)
                 pr_alert("%s" TORTURE_FLAG
                          "torture_shutdown task: %llu ms remaining\n",
                          torture_type, ktime_ms_delta(shutdown_time, ktime_get()));

	Arnd