From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753890AbcAILEx (ORCPT ); Sat, 9 Jan 2016 06:04:53 -0500 Received: from www.osadl.org ([62.245.132.105]:56765 "EHLO www.osadl.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752395AbcAILEw (ORCPT ); Sat, 9 Jan 2016 06:04:52 -0500 From: Nicholas Mc Guire To: John Stultz Cc: Thomas Gleixner , linux-kernel@vger.kernel.org, Nicholas Mc Guire Subject: [PATCH RFC] timer: drop the unnecessary while loop in msleep Date: Sat, 9 Jan 2016 12:01:04 +0100 Message-Id: <1452337264-32209-1-git-send-email-hofrat@osadl.org> X-Mailer: git-send-email 2.1.4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- 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 (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