Thread (12 messages) 12 messages, 4 authors, 2004-11-24

Re: [PATCH] Add ssleep_interruptible()

From: Horms <horms@verge.net.au>
Date: 2004-11-22 02:48:24
Also in: kernel-janitors, lkml

On Tue, Nov 16, 2004 at 05:30:59PM -0800, Nishanth Aravamudan wrote:
On Mon, Nov 01, 2004 at 12:07:49PM -0800, Nishanth Aravamudan wrote:
quoted
Description: Adds ssleep_interruptible() to allow longer delays to occur
in TASK_INTERRUPTIBLE, similarly to ssleep(). To be consistent with
msleep_interruptible(), ssleep_interruptible() returns the remaining time
left in the delay in terms of seconds. This required dividing the return
value of msleep_interruptible() by 1000, thus a cast to (unsigned long)
to prevent any floating point issues.

Signed-off-by: Nishanth Aravamudan <redacted>
--- 2.6.10-rc1-vanilla/include/linux/delay.h	2004-10-30 
15:34:03.000000000 -0700
+++ 2.6.10-rc1/include/linux/delay.h	2004-11-01 12:06:11.000000000 -0800
@@ -46,4 +46,9 @@ static inline void ssleep(unsigned int s
	msleep(seconds * 1000);
}

+static inline unsigned long ssleep_interruptible(unsigned int seconds)
+{
+	return (unsigned long)(msleep_interruptible(seconds * 1000) / 1000);
+}
+
#endif /* defined(_LINUX_DELAY_H) */
After a discussion on IRC, I believe it is pretty clear that this
function has serious issues. Mainly, that if I request a delay of 1
second, but msleep_interruptible() returns after 1 millisecond, then
ssleep_interruptible() will return 0, claiming the entire delay was
used (due to rounding).

Perhaps we should just be satisfied with milliseconds being the grossest
(in contrast to fine) measure of time, at least in terms of
interruptible delays. ssleep() is unaffected by this problem, of course.

Please revert this patch, if applied, as well as any of the other
patches I sent using ssleep_interruptible() [only a handful].
Would making sure that the time slept was always rounded up to
the nearest second resolve this problem. I believe that rounding
up is a common approach to resolving this type of problem when
changing clock resolution.

I am thinking of something like this.

===== include/linux/delay.h 1.6 vs edited =====
--- 1.6/include/linux/delay.h	2004-09-03 18:08:32 +09:00
+++ edited/include/linux/delay.h	2004-11-22 11:47:03 +09:00
@@ -46,4 +46,10 @@ static inline void ssleep(unsigned int s
 	msleep(seconds * 1000);
 }
 
+static inline unsigned long ssleep_interruptible(unsigned int seconds)
+{
+	return (unsigned long)((msleep_interruptible(seconds * 1000) + 999) / 
+			1000);
+}
+
 #endif /* defined(_LINUX_DELAY_H) */
-- 
Horms
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help