delay documentation and checkpatch additions
cover delay documentation and checkpatch additions
This patchset depends on:
commit 22b8f15c2f7130bb0386f548428df2ffd4e81903
Author: Patrick Pannuto <[email protected]> Date: Mon Jul 19 15:09:26 2010 -0700
timer: Added usleep[_range] timer
on tip/master
The series first introduces some documentation explaining the various linux delay mechanisms, and then adds some checkpatch rules to help pick the most appropriate timer.
PATCH 1/4 Documentation: Add timers/delays.txt
This file seeks to explain the nuances in various delays; many driver writers are not necessarily familiar with the various kernel timers, their shortfalls, and quirks. When faced with
ndelay, udelay, mdelay, usleep, usleep_range, msleep, and msleep_interrubtible
the question "How do I just wait 1 ms for my hardware to latch?" has the non-intuitive "best" answer:
usleep_range(1000,2000)
This patch is followed by a series of checkpatch additions that seek to help kernel hackers pick the best delay.
--- Documentation/timers/delays.txt | 97 +++++++++++++++++++++++++++++++++++++++ 1 files changed, 97 insertions(+), 0 deletions(-) create mode 100644 Documentation/timers/delays.txt
Documentation/timers/delays.txt new file +97 -0
@@ -0,0 +1,97 @@ +delays - Information on the various kernel delay / sleep mechanisms +------------------------------------------------------------------- + +This document seeks to answer the common question: "What is the +RightWay (TM) to insert a delay?" + +This question is most often faced by driver writers who have to +deal with hardware delays and who may not be the most initimately +familiar with the inner workings of the Linux Kernel. + + +Inserting Delays +---------------- + +The first, and most important, question you need to ask is "Is my +code in an atomic context?" This should be followed closely by "Does +it really need to delay in atomic context?" If so... + +ATOMIC CONTEXT: + You must use the *delay family of functions. These + functions use the jiffie estimation of clock speed + and will busy wait for enough loop cycles to achieve + the desired delay: + + ndelay(unsigned long nsecs) + udelay(unsigned long usecs) + mdelay(unsgined long msecs) + + udelay is the generally preferred API; ndelay-level + precision may not actually exist on many non-PC devices. + + mdelay is macro wrapper around udelay, to account for + possible overflow when passing large arguments to udelay. + In general, use of mdelay is discouraged. + +NON-ATOMIC CONTEXT: + You should use the *sleep[_range] family of functions. + There are a few more options here, while any of them may + work correctly, using the "right" sleep function will + help the scheduler, power management, and just make your + driver better :) + + -- Backed by busy-wait loop: + udelay(unsigned long usecs) + -- Backed by hrtimers: + usleep(unsigned long usecs) + usleep_range(unsigned long min, unsigned long max) + -- Backed by jiffies / legacy_timers + msleep(unsigned long msecs) + msleep_interruptible(unsigned long msecs) + + Unlike the *delay family, the underlying mechanism + driving each of these calls varies, thus there are + quirks you should be aware of. + + + SLEEPING FOR "A FEW" USECS ( < ~10us? ): + * Use udelay + + - Why not usleep? + On slower systems, (embedded, OR perhaps a speed- + stepped PC!) the overhead of setting up the hrtimers + for usleep *may* not be worth it. Such an evaluation + will obviously depend on your specific situation, but + it is something to be aware of. + + SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms): + * Use usleep_range + + - Why not msleep for (1ms - 20ms)? + Explained originally here: + http://lkml.org/lkml/2007/8/3/250 + msleep(1~20) may not do what the caller intends, and + will often sleep longer (~20 ms actual sleep for any + value given in the 1~20ms range). In many cases this + is not the desired behavior. + + - usleep vs usleep_range: + Since usleep is built on top of high-resolution timers, + you will trigger an interrupt almost *exactly* when your + sleep expires; normally, sleeps (by their nature) do not + need this kind of precision. The *much* friendlier + usleep_range allows the kernel to complete your sleep + any time in the given range, likely when some other + interrupt has already woken up the kernel for some other + reason. + + SLEEPING FOR LARGER MSECS ( 10ms+ ) + * Use msleep or possibly msleep_interruptible + + - What's the difference? + msleep sets the current task to TASK_UNINTERRUPTIBLE + whereas msleep_interruptible sets the current task to + TASK_INTERRUBTIBLE before scheduling the sleep. In + short, the difference is whether the sleep can be ended + early by a signal. In general, just use msleep unless + you know you have a need for the interruptible varient.
Signature · 1 line
1.7.2
PATCH 2/4 Checkpatch: prefer usleep_range over udelay
In reply to Patrick PannutoWhen possible, sleeping is (usually) better than delaying, however, don't bother callers of udelay < 10us, as those cases are not with the switch to usleep_range
--- scripts/checkpatch.pl | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
scripts/checkpatch.pl +8 -0
@@ -2570,6 +2570,14 @@ sub process { } } +# prefer usleep_range over udelay + if ($line =~ /\budelay\s*\((.+)\);/) { + # ignore udelay's < 10, however + if (! (($1 =~ /(\d+)/) && ($1 < 10)) ) { + WARN("usleep_range is preferred over udelay; see Documentation/timers/delays.txt\n" . $line); + } + } + # warn about #ifdefs in C files # if ($line =~ /^.\s*\#\s*if(|n)def/ && ($realfile =~ /\.c$/)) { # print "#ifdef in C files should be avoided\n";
Signature · 1 line
1.7.2
Joe Perches
maintainerRe: [PATCH 2/4] Checkpatch: prefer usleep_range over udelay
scripts/checkpatch.pl sub process { · line 2575
Quoted 6 lines · Patrick Pannuto
When possible, sleeping is (usually) better than delaying, however, don't bother callers of udelay < 10us, as those cases are not with the switch to
usleep_rangescripts/checkpatch.pl +3
-0+# prefer usleep_range over udelay + if ($line =~ /\budelay\s*\((.+)\);/) { + # ignore udelay's < 10, however
This doesn't handle these cases:
udelay(MY_DEFINED_DELAY) udelay( 100 )
Shouldn't this be:
if (($line =~ /\budelay\s*\(\s*(\w+)\s*\)/ {
WARN("usleep_range is preferred over udelay; see Documentation/timers/delays.txt\n" . $line);Maybe these should be converted from WARN to CHK
PATCH 3/4 Checkpatch: prefer usleep_range over usleep
In reply to Patrick PannutoIn (almost) every case, usleep_range is better than usleep, as the precise (ish) wakeup - more accurately the extra interrupt - from usleep is unnecessary.
usleep_range gives a much better chance of coalescing processor wakeups.
--- scripts/checkpatch.pl | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
scripts/checkpatch.pl +5 -0
@@ -2578,6 +2578,11 @@ sub process { } } +# prefer usleep_range over usleep + if ($line =~ /\busleep\s*\(.+\);/) { + WARN("usleep_range is preferred over usleep; see Documentation/timers/delays.txt\n" . $line); + } + # warn about #ifdefs in C files # if ($line =~ /^.\s*\#\s*if(|n)def/ && ($realfile =~ /\.c$/)) { # print "#ifdef in C files should be avoided\n";
Signature · 1 line
1.7.2
PATCH 4/4 Checkpatch: warn about unexpectedly long msleep's
In reply to Patrick PannutoAs explained in Documentation/timers/delays.txt, msleep's of < 20ms may sleep for as long as 20ms. Caller's of msleep(1) or msleep(2), etc are likely not to expect this quirky behavior - warn them.
--- scripts/checkpatch.pl | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
scripts/checkpatch.pl +7 -0
@@ -2583,6 +2583,13 @@ sub process { WARN("usleep_range is preferred over usleep; see Documentation/timers/delays.txt\n" . $line); } +# warn about unexpectedly long msleep's + if ($line =~ /\bmsleep\s*\((\d+)\);/) { + if ($1 < 20) { + WARN("msleep < 20ms can sleep for up to 20ms; see Documentation/timers/delays.txt\n" . $line); + } + } + # warn about #ifdefs in C files # if ($line =~ /^.\s*\#\s*if(|n)def/ && ($realfile =~ /\.c$/)) { # print "#ifdef in C files should be avoided\n";
Signature · 1 line
1.7.2