linux-kernel - Re: [RFC] Improving udelay/ndelay on platforms where that is possible

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] [day] [month] [year] [list]

Message-ID: <20171207124326.GC24547@atrey.karlin.mff.cuni.cz>
Date:   Thu, 7 Dec 2017 13:43:26 +0100
From:   Pavel Machek <pavel@....cz>
To:     Russell King - ARM Linux <linux@...linux.org.uk>
Cc:     Marc Gonzalez <marc_gonzalez@...madesigns.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Alan Cox <gnomes@...rguk.ukuu.org.uk>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        John Stultz <john.stultz@...aro.org>,
        Douglas Anderson <dianders@...omium.org>,
        Nicolas Pitre <nico@...aro.org>,
        Mark Rutland <mark.rutland@....com>,
        Will Deacon <will.deacon@....com>,
        Jonathan Austin <jonathan.austin@....com>,
        Arnd Bergmann <arnd@...db.de>,
        Kevin Hilman <khilman@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...eaurora.org>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>,
        Thibaud Cornic <thibaud_cornic@...madesigns.com>,
        Mason <slash.tmp@...e.fr>
Subject: Re: [RFC] Improving udelay/ndelay on platforms where that is possible

> On Wed, Nov 15, 2017 at 01:51:54PM +0100, Marc Gonzalez wrote:
> > On 01/11/2017 20:38, Marc Gonzalez wrote:
> > 
> > > OK, I'll just send my patch, and then crawl back under my rock.
> > 
> > Linus,
> > 
> > As promised, the patch is provided below. And as promised, I will
> > no longer bring this up on LKML.
> > 
> > FWIW, I have checked that the computed value matches the expected
> > value for all HZ and delay_us, and for a few clock frequencies,
> > using the following program:
> > 
> > $ cat delays.c
> > #include <stdio.h>
> > #define MEGA 1000000u
> > typedef unsigned int uint;
> > typedef unsigned long long u64;
> > #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
> > 
> > static const uint HZ_tab[] = { 100, 250, 300, 1000 };
> > 
> > static void check_cycle_count(uint freq, uint HZ, uint delay_us)
> > {
> > 	uint UDELAY_MULT = (2147 * HZ) + (483648 * HZ / MEGA);
> > 	uint lpj = DIV_ROUND_UP(freq, HZ);
> > 	uint computed = ((u64)lpj * delay_us * UDELAY_MULT >> 31) + 1;
> > 	uint expected = DIV_ROUND_UP((u64)delay_us * freq, MEGA);
> > 
> > 	if (computed != expected)
> > 		printf("freq=%u HZ=%u delay_us=%u comp=%u exp=%u\n", freq, HZ, delay_us, computed, expected);
> > }
> > 
> > int main(void)
> > {
> > 	uint idx, delay_us, freq;
> > 
> > 	for (freq = 3*MEGA; freq <= 100*MEGA; freq += 3*MEGA)
> > 		for (idx = 0; idx < sizeof HZ_tab / sizeof *HZ_tab; ++idx)
> > 			for (delay_us = 1; delay_us <= 2000; ++delay_us)
> > 				check_cycle_count(freq, HZ_tab[idx], delay_us);
> > 
> > 	return 0;
> > }
> > 
> > 
> > 
> > -- >8 --
> > Subject: [PATCH] ARM: Tweak clock-based udelay implementation
> > 
> > In 9f8197980d87a ("delay: Add explanation of udelay() inaccuracy")
> > Russell pointed out that loop-based delays may return early.
> > 
> > On the arm platform, delays may be either loop-based or clock-based.
> > 
> > This patch tweaks the clock-based implementation so that udelay(N)
> > is guaranteed to spin at least N microseconds.
> 
> As I've already said, I don't want this, because it encourages people
> to use too-small delays in driver code, and if we merge it then you
> will look at your data sheet, decide it says "you need to wait 10us"
> and write in your driver "udelay(10)" which will break on the loops
> based delay.
> 
> udelay() needs to offer a consistent interface so that drivers know
> what to expect no matter what the implementation is.  Making one
> implementation conform to your ideas while leaving the other
> implementations with other expectations is a recipe for bugs.

udelay() needs to be consistent across platforms, and yes, udelay(10)
is expected to delay at least 10usec.

If that is not true on your platform, _fix your platform_. But it is
not valid to reject patches fixing other platforms, just because your
platform is broken.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html