checkpatch: Add warnings for use of mdelay()
in [Kernel]
|
Prev: [PATCH 5/9] vmap: add flag to allow lazy unmap to be disabled at runtime
Next: [PATCH] Xen-SWIOTLB v0.8.6 used for Xen PCI pass through for PV guests.
From: Joe Perches on 27 Jul 2010 13:30 On Tue, 2010-07-27 at 10:11 -0700, Israel Schlesinger wrote: > + if ($line =~ /\bmdelay\(/) { That should be: if ($line =~ /\bmdelay\s*\(/) { -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jonathan Corbet on 27 Jul 2010 13:40 On Tue, 27 Jul 2010 10:11:11 -0700 Israel Schlesinger <israels(a)codeaurora.org> wrote: > mdelay is a busy-wait loop which is wasteful. If at all possible, > callers should use msleep instead of mdelay. > > The only time mdelay is really appropriate is in atomic context, > however, delays of 1ms+ in atomic context are rather expensive, so > a warning for this case is probably appropriate as well to encourage > people to move such expensive delays outside of atomic context Once upon a time, msleep(1) would sleep for 20ms, while mdelay(1) gave a 1ms delay. My patch to fix msleep() at that time didn't get in due to concerns about the cost of using hrtimers. Perhaps msleep() has gotten better, but, if not, actually getting a 1ms delay remains a valid reason for using mdelay() instead IMO. It made a difference of a few seconds at open time for a driver I was doing at the time. jon -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Patrick Pannuto on 27 Jul 2010 13:40 On 07/27/2010 10:31 AM, Jonathan Corbet wrote: > On Tue, 27 Jul 2010 10:11:11 -0700 > Israel Schlesinger <israels(a)codeaurora.org> wrote: > >> mdelay is a busy-wait loop which is wasteful. If at all possible, >> callers should use msleep instead of mdelay. >> >> The only time mdelay is really appropriate is in atomic context, >> however, delays of 1ms+ in atomic context are rather expensive, so >> a warning for this case is probably appropriate as well to encourage >> people to move such expensive delays outside of atomic context > > Once upon a time, msleep(1) would sleep for 20ms, while mdelay(1) gave > a 1ms delay. My patch to fix msleep() at that time didn't get in due > to concerns about the cost of using hrtimers. Perhaps msleep() has > gotten better, but, if not, actually getting a 1ms delay remains a > valid reason for using mdelay() instead IMO. It made a difference of a > few seconds at open time for a driver I was doing at the time. > > jon > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo(a)vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ Check out the recently added usleep in -tip, and the checkpatch patch pending in my queue that fixes that case (I'll send in a few hours ;) ) -pat -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on 27 Jul 2010 15:20 On Tue, 27 Jul 2010 10:32:54 -0700 Patrick Pannuto <ppannuto(a)codeaurora.org> wrote: > On 07/27/2010 10:31 AM, Jonathan Corbet wrote: > > On Tue, 27 Jul 2010 10:11:11 -0700 > > Israel Schlesinger <israels(a)codeaurora.org> wrote: > > > >> mdelay is a busy-wait loop which is wasteful. If at all possible, > >> callers should use msleep instead of mdelay. > >> > >> The only time mdelay is really appropriate is in atomic context, > >> however, delays of 1ms+ in atomic context are rather expensive, so > >> a warning for this case is probably appropriate as well to encourage > >> people to move such expensive delays outside of atomic context > > > > Once upon a time, msleep(1) would sleep for 20ms, while mdelay(1) gave > > a 1ms delay. My patch to fix msleep() at that time didn't get in due > > to concerns about the cost of using hrtimers. Perhaps msleep() has > > gotten better, but, if not, actually getting a 1ms delay remains a > > valid reason for using mdelay() instead IMO. It made a difference of a > > few seconds at open time for a driver I was doing at the time. > > > > jon > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo(a)vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > Check out the recently added usleep in -tip, and the checkpatch patch > pending in my queue that fixes that case (I'll send in a few hours ;) ) > The message should point people at usleep_range(), I'd suggest. It's a more power-friendly way of sleeping. That assumes that the below patch gets merged - the people who handle timer-related things are presently, err, asleep. From: Patrick Pannuto <ppannuto(a)codeaurora.org> 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 <ppannuto(a)codeaurora.org> Acked-by: Arjan van de Ven <arjan(a)linux.intel.com> Cc: Ingo Molnar <mingo(a)elte.hu> Cc: Thomas Gleixner <tglx(a)linutronix.de> Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org> --- include/linux/delay.h | 6 ++++++ kernel/timer.c | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff -puN include/linux/delay.h~timers-add-usleep-timer include/linux/delay.h --- a/include/linux/delay.h~timers-add-usleep-timer +++ a/include/linux/delay.h @@ -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) { diff -puN kernel/timer.c~timers-add-usleep-timer kernel/timer.c --- a/kernel/timer.c~timers-add-usleep-timer +++ a/kernel/timer.c @@ -1763,3 +1763,25 @@ unsigned long msleep_interruptible(unsig } 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); _ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Américo Wang on 27 Jul 2010 23:20
On Tue, Jul 27, 2010 at 10:27:10AM -0700, Israel Schlesinger wrote: >mdelay is a busy-wait loop which is wasteful. If at all possible, >callers should use msleep instead of mdelay. > >The only time mdelay is really appropriate is in atomic context, >however, delays of 1ms+ in atomic context are rather expensive, so >a warning for this case is probably appropriate as well to encourage >people to move such expensive delays outside of atomic context > Then udelay() is also preferred... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |