mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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

             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®