From patchwork Sat Nov 19 11:03:01 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Russell King (Oracle)" X-Patchwork-Id: 9438109 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 37B2A60237 for ; Sat, 19 Nov 2016 11:07:13 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2A459297CA for ; Sat, 19 Nov 2016 11:07:13 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1CC80297CC; Sat, 19 Nov 2016 11:07:13 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 0002B297CA for ; Sat, 19 Nov 2016 11:07:11 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1c83RR-0000DT-P3; Sat, 19 Nov 2016 11:04:17 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1c83RN-0000Cx-1b for linux-arm-kernel@lists.infradead.org; Sat, 19 Nov 2016 11:04:14 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=BcK/QHK11z8X9onwQKdleC5LkL7loFrxgi2rWSyYNHo=; b=GxmjVGWltzaTK2ni7oXzOPkGxtiD7YbNOJhbB0t9udBUI+cT6znq8QYC/10W3Ks+kzVCrLA5/UsoOijLS2I/GqWRC3xJaDwDER/BRDledbaiOn8clRDdGORuUCf8Nk7W+32ZcrwgeFYLtfQgo6JbIZkubETD8DTgMchaCr9aXb0=; Received: from n2100.armlinux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:41871) by pandora.armlinux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1c83QK-0002vT-Op; Sat, 19 Nov 2016 11:03:09 +0000 Received: from linux by n2100.armlinux.org.uk with local (Exim 4.76) (envelope-from ) id 1c83QE-0003lP-Bm; Sat, 19 Nov 2016 11:03:02 +0000 Date: Sat, 19 Nov 2016 11:03:01 +0000 From: Russell King - ARM Linux To: Afzal Mohammed , Linus Torvalds Subject: Re: [PATCH] arm: spin one more cycle in timer-based delays Message-ID: <20161119110301.GQ1041@n2100.armlinux.org.uk> References: <582B0F61.6030903@free.fr> <20161118120630.GJ13470@arm.com> <20161118125409.GK1041@n2100.armlinux.org.uk> <582F0DD2.3030805@free.fr> <20161119071702.GA25647@afzalpc> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20161119071702.GA25647@afzalpc> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20161119_030413_723997_54337D08 X-CRM114-Status: GOOD ( 25.55 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Sebastian Frias , Jonathan Austin , Arnd Bergmann , Mason , Peter Zijlstra , Peter De Schrijver , Kevin Hilman , Will Deacon , Douglas Anderson , Steven Rostedt , Thomas Gleixner , Ingo Molnar , Linux ARM Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Linus, please see the comment and patch at the bottom of this mail. Thanks. On Sat, Nov 19, 2016 at 12:47:02PM +0530, Afzal Mohammed wrote: > Hi Mason, > > On Fri, Nov 18, 2016 at 03:18:58PM +0100, Mason wrote: > > On 18/11/2016 13:54, Russell King - ARM Linux wrote: > > > > So, NAK on this change. udelay is not super-accurate. > > > > usleep_range() fixed this issue recently. > > 6c5e9059692567740a4ee51530dffe51a4b9584d > > https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=timers/core&id=6c5e9059692567740a4ee51530dffe51a4b9584d > > But the above "timers: Fix usleep_range() in the context of > wake_up_process()" is to avoid wakeup causing premature return than > about being precise, no ? usleep*() is different from udelay(). usleep*() is not based on looping a certain number of times, and doesn't involve calibration of such a loop. usleep*() is based on the scheduler, which has tighter requirements laid down in POSIX amongst other standards, such as "not timing out before this specified time" (due to things like select(), poll(), etc.) udelay() is purely a kernel thing, unspecified by any standard. > With conflicting opinion on delay/sleep fn's from the players, the one > in gallery would get confused. > > But Linus has mentioned udelay as not meant to be precise, okay ? Exactly - and the reason for that (as I've explained several times in the past) the "standard" software delay loop calibrated against the timer interrupt is _always_ going to be short. I explain why this is in the message to which Linus replied: http://lists.openwall.net/linux-kernel/2011/01/09/56 A consequence of the formula that I give in (2) in that mail is that the higher the HZ value, the more error in the resulting value of loops_per_jiffy, and the shorter udelay(1) than 1us will be, since "timer_interrupt_usec" is a constant but "usec_per_jiffy" reduces. So folk need to put the idea that "udelay(1) will always be at least 1us" out of their minds - that's simply not guaranteed by the kernel. Linus' reply also indicates that we don't care if it's out by 5%, and probably more than that too. If someone can show that our timer-based udelay() produces an error more than 5%, then I'll apply the patch. What I don't want to do is to apply the patch because someone thinks that udelay() should not return early. Applying it in that case has the effect of re-inforcing what is an incorrect assumption, leading to people writing buggy drivers that have delays which are too finely "tuned" - which may work with a timer-based udelay() but cause failures with a loop-based udelay(). This is all about ensuring that driver authors do the right thing. Linus, how about we add something like this to linux/delay.h to document this fact? include/linux/delay.h | 12 +++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/linux/delay.h b/include/linux/delay.h index a6ecb34cf547..2ecb3c46b20a 100644 --- a/include/linux/delay.h +++ b/include/linux/delay.h @@ -5,6 +5,18 @@ * Copyright (C) 1993 Linus Torvalds * * Delay routines, using a pre-computed "loops_per_jiffy" value. + * + * Please note that ndelay(), udelay() and mdelay() may return early for + * several reasons: + * 1. computed loops_per_jiffy too low (due to the time taken to + * execute the timer interrupt.) + * 2. cache behaviour affecting the time it takes to execute the + * loop function. + * 3. CPU clock rate changes. + * As a result, delays should always be over-stated. + * + * Please see this thread: + * http://lists.openwall.net/linux-kernel/2011/01/09/56 */ #include