[PATCH RFC] timer: drop the unnecessary while loop in msleep

From: Nicholas Mc Guire
Date: Sat Jan 09 2016 - 06:05:31 EST


The while loop in msleep does not seem necessary as
timeout is unsigned long and no larger than MAX_JIFFY_OFFSET (which is
LONG_MAX/2 - 1) so the while-loop condition is always true at the beginning
(msecs_to_jiffies will return >=0 always and with the +1 timeout is >= 1 so
the while condition is always true at the start) and
schedule_timeout_uninterruptible always returns 0, so the while loop always
terminates after the first loop.

So msleep really should be a pure wrapper to
schedule_timeout_uninterruptible(msecs_to_jiffies(timeout));

Signed-off-by: Nicholas Mc Guire <hofrat@xxxxxxxxx>
---

Q: what is the purpose of the + 1 offset to the jiffies here ?

msleep was introduced in 2.6.7 but without the + 1, so with:
unsigned long timeout = msecs_to_jiffies(msecs);
in 2.6.10-rc2 the msecs_to_jiffies(msecs) + 1; is introduced.
Nishanth Aravamudan <nacc@xxxxxxxxxx> (https://lkml.org/lkml/2004/11/19/294)
seems to be the origin while converting msleep to a macro, but no reason
for the + 1 is given there.

The msecs_to_jiffies(msecs) + 1; is not clear to me what purpose it serves
it also pre-dates git... so I was not able to find the rational for the + 1
in the commit logs (and its not documented in
Documentation/timers/timers-howto.txt either). From
http://lkml.org/lkml/2007/8/3/250 it though seems clear that msleep should
not be in use for small values of timeout anyway and msleep(0) is just as
bad as msleep(1) with respect to performance.

>From my understanding the + 1 is not necessary and a call to msleep(0)
would be perfectly legal (__mode_timer will handle it correctly) though not
desirable and the + 1 could also be dropped.

patch was compile tested with: x86_64_defconfig

patch is against linux-next 4.4-rc8 (localversion-next is -next-20160108)

kernel/time/timer.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 2c40347..3b131d3 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1685,8 +1685,7 @@ void msleep(unsigned int msecs)
{
unsigned long timeout = msecs_to_jiffies(msecs) + 1;

- while (timeout)
- timeout = schedule_timeout_uninterruptible(timeout);
+ timeout = schedule_timeout_uninterruptible(timeout);
}

EXPORT_SYMBOL(msleep);
--
2.1.4