From: Nicholas Mc Guire <hofrat@osadl.org>
To: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
linux-kernel@vger.kernel.org,
Nicholas Mc Guire <hofrat@osadl.org>
Subject: [PATCH RFC] timer: drop the unnecessary while loop in msleep
Date: Sat, 9 Jan 2016 12:01:04 +0100 [thread overview]
Message-ID: <1452337264-32209-1-git-send-email-hofrat@osadl.org> (raw)
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@osadl.org>
---
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@us.ibm.com> (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
next reply other threads:[~2016-01-09 11:04 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-09 11:01 Nicholas Mc Guire [this message]
2016-01-11 8:15 ` Thomas Gleixner
2016-01-12 12:48 ` Nicholas Mc Guire
2016-01-12 13:53 ` Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1452337264-32209-1-git-send-email-hofrat@osadl.org \
--to=hofrat@osadl.org \
--cc=john.stultz@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®