From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753124AbcFUSjw (ORCPT ); Tue, 21 Jun 2016 14:39:52 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:43167 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753099AbcFUSju (ORCPT ); Tue, 21 Jun 2016 14:39:50 -0400 X-IBM-Helo: d01dlp02.pok.ibm.com X-IBM-MailFrom: paulmck@linux.vnet.ibm.com Date: Tue, 21 Jun 2016 11:39:46 -0700 From: "Paul E. McKenney" To: Arnd Bergmann Cc: Boqun Feng , Josh Triplett , linux-kernel@vger.kernel.org Subject: Re: [PATCH] torture: use ktime_t consistently Reply-To: paulmck@linux.vnet.ibm.com References: <20160620155651.2497676-1-arnd@arndb.de> <7277827.7A0WjZ9kZU@wuerfel> <20160620183757.GX3923@linux.vnet.ibm.com> <17304047.6zmVOzdKjg@wuerfel> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <17304047.6zmVOzdKjg@wuerfel> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16062118-0044-0000-0000-0000007634AF X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16062118-0045-0000-0000-0000048B442D Message-Id: <20160621183946.GT3923@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-06-21_10:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1606210208 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 20, 2016 at 09:29:56PM +0200, Arnd Bergmann wrote: > 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. And there are quite a few that pass in zero for the seconds portion, and many that pass in zero for both. For the moment, I switched to ktime_set(), as you suggested. > > > > @@ -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 Thank you! > 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())); Good point! I did this, but used ktime_snap instead of ktime_get(). I have to capture ktime_snap anyway for the loop termination condition. Updated commit below. Thanx, Paul ------------------------------------------------------------------------ commit 0a387a5b8718feace7aadd847937fdc336567a36 Author: Paul E. McKenney Date: Sat Jun 18 07:45:43 2016 -0700 torture: Convert torture_shutdown() to hrtimer Upcoming changes to the timer wheel introduce significant inaccuracy and possibly also an ultimate limit on timeout duration. This is a problem for the current implementation of torture_shutdown() because (1) shutdown times are user-specified, and can therefore be quite long, and (2) the torture scripting will kill a test instance that runs for more than a few minutes longer than scheduled. This commit therefore converts the torture_shutdown() timed waits to an hrtimer. Signed-off-by: Paul E. McKenney Acked-by: Arnd Bergmann diff --git a/kernel/torture.c b/kernel/torture.c index 75961b3decfe..08f45621394a 100644 --- a/kernel/torture.c +++ b/kernel/torture.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -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_secs; /* desired test duration in seconds. */ static struct task_struct *shutdown_task; -static unsigned long shutdown_time; /* jiffies to system shutdown. */ +static ktime_t shutdown_time; /* jiffies to system shutdown. */ static void (*torture_shutdown_hook)(void); /* @@ -471,20 +472,20 @@ EXPORT_SYMBOL_GPL(torture_shutdown_absorb); */ static int torture_shutdown(void *arg) { - long delta; - unsigned long jiffies_snap; + ktime_t ktime_snap; VERBOSE_TOROUT_STRING("torture_shutdown task started"); - jiffies_snap = jiffies; - while (ULONG_CMP_LT(jiffies_snap, shutdown_time) && + ktime_snap = ktime_get(); + while (ktime_before(ktime_snap, shutdown_time) && !torture_must_stop()) { - delta = shutdown_time - jiffies_snap; if (verbose) pr_alert("%s" TORTURE_FLAG - "torture_shutdown task: %lu jiffies remaining\n", - torture_type, delta); - schedule_timeout_interruptible(delta); - jiffies_snap = jiffies; + "torture_shutdown task: %llu ms remaining\n", + torture_type, + ktime_ms_delta(shutdown_time, ktime_snap)); + set_current_state(TASK_INTERRUPTIBLE); + schedule_hrtimeout(&shutdown_time, HRTIMER_MODE_ABS); + ktime_snap = ktime_get(); } if (torture_must_stop()) { torture_kthread_stopping("torture_shutdown"); @@ -511,10 +512,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_secs = ktime_set(ssecs, 0); + shutdown_time = ktime_add(ktime_get(), shutdown_secs); ret = torture_create_kthread(torture_shutdown, NULL, shutdown_task); }