timer: Added usleep[_range][_interruptible] timer
PATCH 1/1 timer: Added usleep[_range][_interruptible] timer
*** INTRO ***
As discussed here ( http://lkml.org/lkml/2007/8/3/250 ), msleep(1) is not precise enough for many drivers (yes, sleep precision is an unfair notion, but consistently sleeping for ~an order of magnitude greater than requested is worth fixing). This patch adds a usleep API so that udelay does not have to be used. Obviously not every udelay can be replaced (those in atomic contexts or being used for simple bitbanging come to mind), but there are many, many examples of
mydriver_write(...)
/* Wait for hardware to latch */
udelay(100)
in various drivers where a busy-wait loop is neither beneficial nor necessary, but msleep simply does not provide enough precision and people are using a busy-wait loop instead.
*** CONCERNS FROM THE RFC ***
Why is udelay a problem / necessary? Most callers of udelay are in device/ driver initialization code, which is serial...
As I see it, there is only benefit to sleeping over a delay; the notion of "refactoring" areas that use udelay was presented, but I see usleep as the refactoring. Consider i2c, if the bus is busy, you need to wait a bit (say 100us) before trying again, your current options are:
* udelay(100)
* msleep(1) <-- As noted above, actually as high as ~20ms on some platforms, so not really an option
* Manually set up an hrtimer to try again in 100us (which is what usleep does anyway...)
People choose the udelay route because it is EASY; we need to provide a better easy route.
Device / driver / boot code is *currently* serial, but every few months someone makes noise about parallelizing boot, and IMHO, a little forward-thinking now is one less thing to worry about if/when that ever happens
udelay's could be preempted
Sure, but if udelay plans on looping 1000 times, and it gets preempted on loop 200, whenever it's scheduled again, it is going to do the next 800 loops.
Is the interruptible case needed?
Probably not, but I see usleep as a very logical parallel to msleep, so it made sense to include the "full" API. Processors are getting faster (albeit not as quickly as they are becoming more parallel), so if someone wanted to be interruptible for a few usecs, why not let them? If this is a contentious point, I'm happy to remove it.
*** OTHER THOUGHTS ***
I believe there is also value in exposing the usleep_range option; it gives the scheduler a lot more flexibility and allows the programmer to express his intent much more clearly; it's something I would hope future driver writers will take advantage of.
To get the results in the NUMBERS section below, I literally s/udelay/usleep the kernel tree; I had to go in and undo the changes to the USB drivers, but everything else booted successfully; I find that extremely telling in and of itself -- many people are using a delay API where a sleep will suit them just fine.
*** SOME ATTEMPTS AT NUMBERS ***
It turns out that calculating quantifiable benefit on this is challenging, so instead I will simply present the current state of things, and I hope this to be sufficient:
How many udelay calls are there in 2.6.35-rc5?
udealy(ARG) >= | COUNT 1000 | 319 500 | 414 100 | 1146 20 | 1832
I am working on Android, so that is my focus for this. The following table is a modified usleep that simply printk's the amount of time requested to sleep; these tests were run on a kernel with udelay >= 20 --> usleep
"boot" is power-on to lock screen
"power collapse" is when the power button is pushed and the device suspends "resume" is when the power button is pushed and the lock screen is displayed
(no touchscreen events or anything, just turning on the display)
"use device" is from the unlock swipe to clicking around a bit; there is no
sd card in this phone, so fail loading music, video, camera ACTION | TOTAL NUMBER OF USLEEP CALLS | NET TIME (us) boot | 22 | 1250 power-collapse | 9 | 1200 resume | 5 | 500 use device | 59 | 7700
The most interesting category to me is the "use device" field; 7700us of busy-wait time that could be put towards better responsiveness, or at the least less power usage.
*** SUMMARY ***
I believe usleep to be a useful and logical extension to the current API, and would like to submit this patch for linux-next
-Pat
Quoted 1 line
From
26193064936016e3f679c911b4e988a3de97c531Mon Sep 17 00:00:00 2001
From: Patrick Pannuto <[email protected]>
Date: Tue, 22 Jun 2010 10:08:08 -0700
Subject: [PATCH] timer: Added usleep[_range][_interruptible] timer
usleep[_range][_interruptible] are finer precision implementations of msleep[_interruptible] and are designed to be drop-in replacements for udelay where a precise sleep / busy-wait is unnecessary. They also allow an easy interface to specify slack when a precise (ish) wakeup is unnecessary to help minimize wakeups
--- include/linux/delay.h | 12 ++++++++++++ kernel/timer.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 0 deletions(-)
include/linux/delay.h +12 -0
@@ -45,6 +45,18 @@ extern unsigned long lpj_fine;void calibrate_delay (void );void msleep (unsigned int msecs);unsigned long msleep_interruptible (unsigned int msecs); +void usleep_range (unsigned long min,unsigned long max); +unsigned long usleep_range_interruptible (unsigned long min,unsigned long max); + +static inline void usleep (unsigned long usecs) +{ +usleep_range (usecs, usecs); +} + +static inline unsigned long usleep_interruptible (unsigned long usecs) +{ +return usleep_range_interruptible (usecs, usecs); +}static inline void ssleep (unsigned int seconds) {
kernel/timer.c +44 -0
47 lines not on this page · raw message
-- 1.7.1
Quoted 2 lines · Patrick Pannuto
*** INTRO ***
I like the general idea, but I kinda doubt the "interruptible" version makes sense for this...
do you have any users for that in mind? if not... can we leave the interruptible version out... makes it simpler by a lot.
PATCH 1/1 timer: Added usleep[_range] timer
Quoted 6 lines
I like the general idea, but I kinda doubt the "interruptible" version makes sense for this...
do you have any users for that in mind? if not... can we leave the interruptible version out... makes it simpler by a lot.
Respun without interruptible...
Quoted 1 line
From
08b717cf4f76d7578cb20c1b1444d89a9331ce02Mon Sep 17 00:00:00 2001
From: Patrick Pannuto <[email protected]>
Date: Mon, 19 Jul 2010 15:09:26 -0700
Subject: [PATCH] timer: Added usleep[_range] timer
usleep[_range] are finer precision implementations of msleep and are designed to be drop-in replacements for udelay where a precise sleep / busy-wait is unnecessary. They also allow an easy interface to specify slack when a precise (ish) wakeup is unnecessary to help minimize wakeups
--- include/linux/delay.h | 6 ++++++ kernel/timer.c | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 0 deletions(-)
include/linux/delay.h +6 -0
@@ -45,6 +45,12 @@ extern unsigned long lpj_fine;void calibrate_delay (void );void msleep (unsigned int msecs);unsigned long msleep_interruptible (unsigned int msecs); +void usleep_range (unsigned long min,unsigned long max); + +static inline void usleep (unsigned long usecs) +{ +usleep_range (usecs, usecs); +}static inline void ssleep (unsigned int seconds) {
kernel/timer.c +22 -0
@@ -1750,3 +1750,25 @@ unsigned long msleep_interruptible(unsigned int msecs) }EXPORT_SYMBOL (msleep_interruptible); + +static int __sched do_usleep_range(unsigned long min, unsigned long max) +{ +ktime_t kmin; +unsigned long delta; + + kmin =ktime_set (0 , min *NSEC_PER_USEC ); + delta = max - min; +return schedule_hrtimeout_range (&kmin, delta,HRTIMER_MODE_REL ); +} + +/** +* usleep_range - Drop in replacement for udelay where wakeup is flexible +* @min: Minimum time in usecs to sleep +* @max: Maximum time in usecs to sleep +*/ +void usleep_range (unsigned long min,unsigned long max) +{ +__set_current_state (TASK_UNINTERRUPTIBLE ); +do_usleep_range (min, max); +} +EXPORT_SYMBOL (usleep_range);
Signature · 1 line
1.7.1.1
Re: [PATCH] timer: Added usleep[_range] timer
Quoted 15 lines · Patrick Pannuto
Quoted 7 lines
I like the general idea, but I kinda doubt the "interruptible" version makes sense for this...
do you have any users for that in mind? if not... can we leave the interruptible version out... makes it simpler by a lot.Respun without interruptible...
From
08b717cf4f76d7578cb20c1b1444d89a9331ce02Mon Sep 17 00:00:00 2001
From: Patrick Pannuto<[email protected]>
Date: Mon, 19 Jul 2010 15:09:26 -0700
Subject: [PATCH] timer: Added usleep[_range] timer
Quoted 7 lines
usleep[_range] are finer precision implementations of msleep and are designed to be drop-in replacements for udelay where a precise sleep / busy-wait is unnecessary. They also allow an easy interface to specify slack when a precise (ish) wakeup is unnecessary to help minimize wakeups
excellent
Re: [PATCH] timer: Added usleep[_range] timer
In reply to Patrick PannutoQuoted 2 lines · On Mon, 19 Jul 2010 15:12:34 -0700 Patrick Pannuto <[email protected]>…
Respun without interruptible...
And without all the nice changelog info. Oh well.
Quoted 49 lines
usleep[_range] are finer precision implementations of msleep and are designed to be drop-in replacements for udelay where a precise sleep / busy-wait is unnecessary. They also allow an easy interface to specify slack when a precise (ish) wakeup is unnecessary to help minimize wakeups
Signed-off-by Patrick Pannuto <[email protected]>--- include/linux/delay.h | 6 ++++++ kernel/timer.c | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 0 deletions(-)include/linux/delay.h +6
-0@@ -45,6 +45,12 @@ extern unsigned long lpj_fine;void calibrate_delay (void );void msleep (unsigned int msecs);unsigned long msleep_interruptible (unsigned int msecs); +void usleep_range (unsigned long min,unsigned long max); + +static inline void usleep (unsigned long usecs) +{ +usleep_range (usecs, usecs); +}static inline void ssleep (unsigned int seconds) {kernel/timer.c +12
-0@@ -1750,3 +1750,25 @@ unsigned long msleep_interruptible(unsigned int msecs) }EXPORT_SYMBOL (msleep_interruptible); + +static int __sched do_usleep_range(unsigned long min, unsigned long max) +{ +ktime_t kmin; +unsigned long delta; + + kmin =ktime_set (0 , min *NSEC_PER_USEC ); + delta = max - min; +return schedule_hrtimeout_range (&kmin, delta,HRTIMER_MODE_REL ); +} + +/**
kernel/timer.c unsigned long msleep_interruptible(unsigned int msecs) · line 1774
Quoted 10 lines
kernel/timer.c +10
-0+* usleep_range - Drop in replacement for udelay where wakeup is flexible +* @min: Minimum time in usecs to sleep +* @max: Maximum time in usecs to sleep +*/ +void usleep_range (unsigned long min,unsigned long max) +{ +__set_current_state (TASK_UNINTERRUPTIBLE ); +do_usleep_range (min, max); +} +EXPORT_SYMBOL (usleep_range);
Fair enough, I guess. People can go over and look at the schedule_hrtimeout_range() documentation to work out why on earth they should specify a "range".
Of course, nobody will actually think to _use_ this thing. Someone owes me a "usleep_range is preferred over usleep" checkpatch rule!
[tip:timers/core] timer: Added usleep[_range] timer
In reply to Patrick PannutoCommit-ID: 22b8f15c2f7130bb0386f548428df2ffd4e81903 Gitweb: http://git.kernel.org/tip/22b8f15c2f7130bb0386f548428df2ffd4e81903 Author: Patrick Pannuto <[email protected]> AuthorDate: Mon, 19 Jul 2010 15:09:26 -0700 Committer: Thomas Gleixner <[email protected]> CommitDate: Fri, 23 Jul 2010 15:08:12 +0200
timer: Added usleep[_range] timer
usleep[_range] are finer precision implementations of msleep and are designed to be drop-in replacements for udelay where a precise sleep / busy-wait is unnecessary. They also allow an easy interface to specify slack when a precise (ish) wakeup is unnecessary to help minimize wakeups
LKML-Reference: <[email protected]>
--- include/linux/delay.h | 6 ++++++ kernel/timer.c | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 0 deletions(-)
include/linux/delay.h +6 -0
@@ -45,6 +45,12 @@ extern unsigned long lpj_fine;void calibrate_delay (void );void msleep (unsigned int msecs);unsigned long msleep_interruptible (unsigned int msecs); +void usleep_range (unsigned long min,unsigned long max); + +static inline void usleep (unsigned long usecs) +{ +usleep_range (usecs, usecs); +}static inline void ssleep (unsigned int seconds) {
kernel/timer.c +22 -0
@@ -1755,3 +1755,25 @@ unsigned long msleep_interruptible(unsigned int msecs) }EXPORT_SYMBOL (msleep_interruptible); + +static int __sched do_usleep_range(unsigned long min, unsigned long max) +{ +ktime_t kmin; +unsigned long delta; + + kmin =ktime_set (0 , min *NSEC_PER_USEC ); + delta = max - min; +return schedule_hrtimeout_range (&kmin, delta,HRTIMER_MODE_REL ); +} + +/** +* usleep_range - Drop in replacement for udelay where wakeup is flexible +* @min: Minimum time in usecs to sleep +* @max: Maximum time in usecs to sleep +*/ +void usleep_range (unsigned long min,unsigned long max) +{ +__set_current_state (TASK_UNINTERRUPTIBLE ); +do_usleep_range (min, max); +} +EXPORT_SYMBOL (usleep_range);