From patchwork Fri May 9 21:05:43 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Pitre X-Patchwork-Id: 4145171 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 958CCBFF02 for ; Fri, 9 May 2014 21:09:05 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B51422034F for ; Fri, 9 May 2014 21:09:04 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C8ECE202EC for ; Fri, 9 May 2014 21:09:03 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Wirzf-0003Xc-TE; Fri, 09 May 2014 21:06:11 +0000 Received: from mail-qg0-f51.google.com ([209.85.192.51]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Wirzd-0003VD-Cx for linux-arm-kernel@lists.infradead.org; Fri, 09 May 2014 21:06:10 +0000 Received: by mail-qg0-f51.google.com with SMTP id q107so5147197qgd.10 for ; Fri, 09 May 2014 14:05:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version:content-type; bh=8PjEm8UDGEDf1DYUqzoenWSZjUEBvmqtrzNGb4e1Sb8=; b=jKN5HOiU87mRodAZqcIEPnyZBUPnNYv2O/ObpIHulOCOiViIB2UrkpVGIUZ4x6cLqU oxHXS4BGzyKBv5Ufm/CkELOK2wZoOlZT+qgrZ54g1j2rWSQ9LcTYDmjchJorcsk4mbAZ fsjU+mPzx25Q/I33WLDCt6raDK7kpcgjNIxHvgjrw0piS+f0zV+IJhQB+U3Qgexl7YB+ naFNuoLBbheGIao2N0qNrT89Ub4wc2HSrgUT6bsxdA0ONQqDrctvmDf4hKp1jTckiWqc 90v9GOBzySO5VjWtBGjLHTdr8XGmuHaJFT9YjLW4ADgqwO6LuRmLINribz+vqktoqGjy hW5A== X-Gm-Message-State: ALoCoQlCfD4X/+QRUZkFDphGPztgnfBTIarVZi7iVgR5aOf39oH4slMHX1bCf9ubrqZbCyKSEBvy X-Received: by 10.140.88.83 with SMTP id s77mr17228564qgd.113.1399669545945; Fri, 09 May 2014 14:05:45 -0700 (PDT) Received: from xanadu.home (modemcable177.143-130-66.mc.videotron.ca. [66.130.143.177]) by mx.google.com with ESMTPSA id j7sm8312606qab.27.2014.05.09.14.05.44 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 09 May 2014 14:05:45 -0700 (PDT) Date: Fri, 9 May 2014 17:05:43 -0400 (EDT) From: Nicolas Pitre To: Russell King - ARM Linux Subject: Re: [PATCH] ARM: Don't ever downscale loops_per_jiffy in SMP systems# In-Reply-To: <20140509182245.GM3693@n2100.arm.linux.org.uk> Message-ID: References: <20140508192209.GH3693@n2100.arm.linux.org.uk> <20140508205223.GI3693@n2100.arm.linux.org.uk> <20140509091824.GL3693@n2100.arm.linux.org.uk> <20140509182245.GM3693@n2100.arm.linux.org.uk> User-Agent: Alpine 2.11 (LFD 23 2013-08-11) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140509_140609_518531_9460D025 X-CRM114-Status: GOOD ( 26.23 ) X-Spam-Score: -0.0 (/) Cc: Stephen Boyd , David Riley , Stephen Warren , Marc Zyngier , Viresh Kumar , "Rafael J. Wysocki" , Doug Anderson , Will Deacon , "linux-kernel@vger.kernel.org" , Paul Gortmaker , John Stultz , "olof@lixom.net" , Santosh Shilimkar , Shawn Guo , Sonny Rao , Richard Zhao , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, 9 May 2014, Russell King - ARM Linux wrote: > I'd much prefer just printing a warning at kernel boot time to report > that the kernel is running with features which would make udelay() less > than accurate. What if there is simply no timer to rely upon, as in those cases where interrupts are needed for time keeping to make progress? We should do better than simply saying "sorry your kernel should irradicate every udelay() usage to be reliable". And I mean "reliable" which is not exactly the same as "accurate". Reliable means "never *significantly* shorter". > Remember, it should be usable for _short_ delays on slow machines as > well as other stuff, and if we're going to start throwing stuff like > the above at it, it's going to become very inefficient. You said that udelay can be much longer than expected due to various reasons. You also said that the IRQ handler overhead during udelay calibration makes actual delays slightli shorter than expected. I'm suggesting the addition of a slight overhead that is much smaller than the IRQ handler here. That shouldn't impact things masurably. I'd certainly like Doug to run his udelay timing test with the following patch to see if it solves the problem. diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h index dff714d886..74fb571a55 100644 --- a/arch/arm/include/asm/delay.h +++ b/arch/arm/include/asm/delay.h @@ -57,11 +57,6 @@ extern void __bad_udelay(void); __const_udelay((n) * UDELAY_MULT)) : \ __udelay(n)) -/* Loop-based definitions for assembly code. */ -extern void __loop_delay(unsigned long loops); -extern void __loop_udelay(unsigned long usecs); -extern void __loop_const_udelay(unsigned long); - /* Delay-loop timer registration. */ #define ARCH_HAS_READ_CURRENT_TIMER extern void register_current_timer_delay(const struct delay_timer *timer); diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c index 5306de3501..9150d31c2d 100644 --- a/arch/arm/lib/delay.c +++ b/arch/arm/lib/delay.c @@ -25,6 +25,11 @@ #include #include +/* Loop-based definitions for assembly code. */ +extern void __loop_delay(unsigned long loops); +extern void __loop_udelay(unsigned long usecs); +extern void __loop_const_udelay(unsigned long); + /* * Default to the loop-based delay implementation. */ @@ -34,6 +39,85 @@ struct arm_delay_ops arm_delay_ops = { .udelay = __loop_udelay, }; +#if defined(CONFIG_CPU_FREQ) && (defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)) + +#include + +/* + * Another CPU/thread might increase the CPU clock in the middle of + * the loop based delay routine and the newly scaled LPJ value won't be + * accounted for, resulting in a possibly significantly shorter delay than + * expected. Let's make sure this occurrence is trapped and compensated. + */ + +static int __loop_seq; +static unsigned int __loop_security_factor; + +#define __safe_loop_(type) \ +static void __safe_loop_##type(unsigned long val) \ +{ \ + int seq_count = __loop_seq; \ + __loop_##type(val); \ + if (seq_count != __loop_seq) \ + __loop_##type(val * __loop_security_factor); \ +} + +__safe_loop_(delay) +__safe_loop_(const_udelay) +__safe_loop_(udelay) + +static int cpufreq_callback(struct notifier_block *nb, + unsigned long val, void *data) +{ + struct cpufreq_freqs *freq = data; + unsigned int f; + + if ((freq->flags & CPUFREQ_CONST_LOOPS) || + freq->old >= freq->new) + return NOTIFY_OK; + + switch (val) { + case CPUFREQ_PRECHANGE: + /* Remember the largest security factor ever needed */ + f = DIV_ROUND_UP(freq->new, freq->old) - 1; + if (__loop_security_factor < f) + __loop_security_factor = f; + /* fallthrough */ + case CPUFREQ_POSTCHANGE: + __loop_seq++; + } + return NOTIFY_OK; +} + +static struct notifier_block cpufreq_notifier = { + .notifier_call = cpufreq_callback, +}; + +static int __init init_safe_loop_delays(void) +{ + int err; + + /* + * Bail out if the default loop based implementation has + * already been replaced by something better. + */ + if (arm_delay_ops.udelay != __loop_udelay) + return 0; + + __loop_security_factor = 1; + err = cpufreq_register_notifier(&cpufreq_notifier, + CPUFREQ_TRANSITION_NOTIFIER); + if (!err) { + arm_delay_ops.delay = __safe_loop_delay; + arm_delay_ops.const_udelay = __safe_loop_const_udelay; + arm_delay_ops.udelay = __safe_loop_udelay; + } + return err; +} +core_initcall(init_safe_loop_delays); + +#endif + static const struct delay_timer *delay_timer; static bool delay_calibrated;