schedule_timeout() issues / questions

PATCH In review Nishanth Aravamudan 1 participant 11 messages Nov 17, 2004 – Nov 22, 2004 2 messages not in the archive High-resolution timers, timer wheel, clockevents

lore

Share & tools

lore

Nishanth Aravamudan

author

2004-11-17 15:32 UTC #0 lore raw

Hi,

After some pretty heavy discussion on IRC, I felt that it may be important / useful to bring the discussion of schedule_timeout() to LKML. There are two issues being considered:

1) msleep_interruptible()

For reference, here is the code for this function:

/**
* msleep_interruptible - sleep waiting for waitqueue interruptions
* @msecs: Time in milliseconds to sleep for
*/
unsigned long msleep_interruptible(unsigned int msecs)

{
	unsigned long timeout = msecs_to_jiffies(msecs);

	while (timeout && !signal_pending(current)) {
		set_current_state(TASK_INTERRUPTIBLE);
		timeout = schedule_timeout(timeout);
	}
	return jiffies_to_msecs(timeout);
}

The first issue deals with whether the while() loop is at all necessary.

Quoted 1 line

From my understanding (primarily from how the code "should" behave, but

also backed up by code itself), I think the following code:

	set_current_state(TASK_INTERRUPTIBLE);
	timeout = schedule_timeout(timeout);

should be interpreted as:

a) I wish to sleep for timeout jiffies; however
b) If a signal occurs before timeout jiffies have gone by, I would also like to wake up.

With this interpretation, though, the while()-conditional becomes questionable. I can see two cases (think inclusive OR not exclusive) for schedule_timeout() returning:

a) A signal was received and thus signal_pending(current) will be true, exiting the loop. In this case, timeout will be some non-negative value (0 is a corner case, I believe, where both the timer fires and a signal is received in the last jiffy).
b) The timer in schedule_timeout() has expired and thus it will return 0. This indicates the function has delayed the requested time (at least) and timeout will be set to 0, again exiting the loop.

Clearly, then, if my interpretion is correct, schedule_timeout() will always return to a state in msleep_interruptible() which causes the loop to only iterate the one time. Does this make sense? Is my interpretation of schedule_timeout()s functioning somehow flawed? If not, we probably can go ahead and change the msleep_interruptible() code, yes?

This is closely tied to a separate issue. I have often seen code check two separate conditions in response to calling schedule_timeout():
whether signals_pending() is true and whether the return value is 0 (in separate if statements). Depending on the correctness of my interpretation again, these are sequentially dependent. Meaning the following code should be a correct and equivalent way to use schedule_timeout():

	set_current_state(TASK_INTERRUPTIBLE);
	timeout = schedule_timeout(timeout);
	if (signals_pending(current))
		/* respond appropriately to signals */
	else
		/* this must mean that timeout == 0, respond accordingly */

I think this should result in smaller, more uniform code and perhaps more well-structured looping (as most schedule_timeout()s occur in some loop). Does this if/else make sense? Since it is contingent on the previous interpretation, maybe you've already thrown it out... But if not, this may result in a good number of standardizing patches (which I think is generally a good thing). This also would make it clear to me how exactly the kernel interprets TASK_INTERRUPTIBLE / TASK_UNINTERRUPTIBLE. <rant> Obviously, one's intuition that TASK_UNINTERRUPTIBLE means don't wake up until the time as gone by, at least, is wrong, or we wouldn't need the similar while-loop in msleep()...

/**
* msleep - sleep safely even with waitqueue interruptions
* @msecs: Time in milliseconds to sleep for
*/
void msleep(unsigned int msecs)

{
	unsigned long timeout = msecs_to_jiffies(msecs);

	while (timeout) {
		set_current_state(TASK_UNINTERRUPTIBLE);
		timeout = schedule_timeout(timeout);
	}
}

</rant>

2) schedule_timeout(1)

A second issue arises when considering the prevalence in the kernel of calling

	set_current_state(TASK_{,UN}INTERRUPTIBLE);
	schedule_timeout(1);

The 1 is rather arbitrary (but most common); any small enough number will do. I think these code segments are now "unintentional" (in the majority, at least; in certain cases, they may certainly be intended). The reason I think this, is that when HZ==100 this is a pretty long delay in human-time (~10ms, assuming no signals...). So maybe code authors actually intended for that length of a delay, but since msleep() and family did not exist, just went with schedule_timeout(). Of course, since 1 jiffy of delay was equivalent to 10ms, it makes the dependency on HZ very hard to see. This is just a theory, though, and I could be completely wrong.

The main point, though, is that this code now introduces excessive overhead. Upon examining the code of schedule_timeout(), one sees the gist of the function to be:

a) Set up a timer to go off after a certain number of jiffies
b) Call schedule()
c) Upon returning, delete the timer and return the number of jiffies left (if positive) or 0

Now, with architectures where HZ==1000 or more, I think the overhead of setting up the timer and then calling schedule() may result in the timer going off in the middle of the schedule() call. This, of course, is not great, because it could mean that another task will start up (for 1 jiffy!), thrash the cache perhaps, and then be preempted by my task again. It would be better, I think to call schedule() directly in these cases where the intention is: give up the CPU. It results in similar behavior (excepting that I won't necessarily be able to run in a jiffy or close to that time), but unless I misunderstand things, that is the same situation as with the timer in schedule_timeout() (i.e., I might be completely wrong, though, if somehow when the timer goes off, my task gets put to the front of it's priority queue).

It just seems to me that we are putting in excessive code to just give up the CPU (the intention of some of the calls, as I see it). I would like to hear other opinions. If I am close to right, though, this will result in a good number of (small) patches which will help clean up the kernel a bit.

Please excuse my lack of brevity. There were a lot of ideas, and I wanted to make myself pretty clear. Any comments would be greatly appreciated.

Thanks,
Nish

Nishanth Aravamudan

author

2004-11-20 00:53 UTC #1 lore raw

PATCH 1/2 kernel/timer.c: remove msleep() and msleep_interruptible()

Quoted 51 lines · Nishanth Aravamudan

Hi,
After some pretty heavy discussion on IRC, I felt that it may be important / useful to bring the discussion of schedule_timeout() to LKML. There are two issues being considered:
1) msleep_interruptible()
For reference, here is the code for this function:
/**
* msleep_interruptible - sleep waiting for waitqueue interruptions
* @msecs: Time in milliseconds to sleep for
*/
unsigned long msleep_interruptible(unsigned int msecs)
{
	unsigned long timeout = msecs_to_jiffies(msecs);

	while (timeout && !signal_pending(current)) {
		set_current_state(TASK_INTERRUPTIBLE);
		timeout = schedule_timeout(timeout);
	}
	return jiffies_to_msecs(timeout);
}
The first issue deals with whether the while() loop is at all necessary. From my understanding (primarily from how the code "should" behave, but also backed up by code itself), I think the following code:
	set_current_state(TASK_INTERRUPTIBLE);
	timeout = schedule_timeout(timeout);
should be interpreted as:
a) I wish to sleep for timeout jiffies; however
b) If a signal occurs before timeout jiffies have gone by, I would also like to wake up.
With this interpretation, though, the while()-conditional becomes questionable. I can see two cases (think inclusive OR not exclusive) for schedule_timeout() returning:
a) A signal was received and thus signal_pending(current) will be true, exiting the loop. In this case, timeout will be some non-negative value (0 is a corner case, I believe, where both the timer fires and a signal is received in the last jiffy).
b) The timer in schedule_timeout() has expired and thus it will return 0. This indicates the function has delayed the requested time (at least) and timeout will be set to 0, again exiting the loop.

Quoted 5 lines

Clearly, then, if my interpretion is correct, schedule_timeout() will always return to a state in msleep_interruptible() which causes the loop to only iterate the one time. Does this make sense? Is my interpretation of schedule_timeout()s functioning somehow flawed? If not, we probably can go ahead and change the msleep_interruptible() code, yes?

Ok, since nobody seems to have objected to my ideas as of yet, I went ahead and implemented them, with much help from Domen and others. I believe there are two options for these changes. One involves using macros instead, the other involves reworking the functions.

I have attached here the version with macros (1/2). I am sure there will be much to correct, so please do so.

Also, I will hopefully submit soon a change to a driver that uses this new msleep*() interface, so as to show how it should work.

-Nish

Description: Removes definitions of msleep() and msleep_interruptible() from kernel/timer.c in preparation for their re-creation as macros in include/linux/delay.h. There are significant issues with the current implementation of msleep_interruptible(). Primarily, the comments given do not reflect the actual results. The function, in fact, ignores wait-queue events and only will respond to signals. The new versions of these functions will correct and clarify this.

kernel/timer.c +0 ~~-33~~

@@ -1609,36 +1609,3 @@ unregister_time_interpolator(struct time
 	spin_unlock(&time_interpolator_lock);
 }
 #endif /* CONFIG_TIME_INTERPOLATION */
-
-/**
- * msleep - sleep safely even with waitqueue interruptions
- * @msecs: Time in milliseconds to sleep for
- */
-void msleep(unsigned int msecs)
-{
-	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
-
-	while (timeout) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		timeout = schedule_timeout(timeout);
-	}
-}
-
-EXPORT_SYMBOL(msleep);
-
-/**
- * msleep_interruptible - sleep waiting for waitqueue interruptions
- * @msecs: Time in milliseconds to sleep for
- */
-unsigned long msleep_interruptible(unsigned int msecs)
-{
-	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
-
-	while (timeout && !signal_pending(current)) {
-		set_current_state(TASK_INTERRUPTIBLE);
-		timeout = schedule_timeout(timeout);
-	}
-	return jiffies_to_msecs(timeout);
-}
-
-EXPORT_SYMBOL(msleep_interruptible);

Nishanth Aravamudan

author

2004-11-20 01:03 UTC #2 lore raw

PATCH 2/2 include/delay.h: replace msleep() and msleep_interruptible() with macros

Quoted 58 lines · Nishanth Aravamudan

Quoted 51 lines · Nishanth Aravamudan
Hi,
After some pretty heavy discussion on IRC, I felt that it may be important / useful to bring the discussion of schedule_timeout() to LKML. There are two issues being considered:
1) msleep_interruptible()
For reference, here is the code for this function:
/**
* msleep_interruptible - sleep waiting for waitqueue interruptions
* @msecs: Time in milliseconds to sleep for
*/
unsigned long msleep_interruptible(unsigned int msecs)
{
	unsigned long timeout = msecs_to_jiffies(msecs);

	while (timeout && !signal_pending(current)) {
		set_current_state(TASK_INTERRUPTIBLE);
		timeout = schedule_timeout(timeout);
	}
	return jiffies_to_msecs(timeout);
}
The first issue deals with whether the while() loop is at all necessary. From my understanding (primarily from how the code "should" behave, but also backed up by code itself), I think the following code:
	set_current_state(TASK_INTERRUPTIBLE);
	timeout = schedule_timeout(timeout);
should be interpreted as:
a) I wish to sleep for timeout jiffies; however
b) If a signal occurs before timeout jiffies have gone by, I would also like to wake up.
With this interpretation, though, the while()-conditional becomes questionable. I can see two cases (think inclusive OR not exclusive) for schedule_timeout() returning:
a) A signal was received and thus signal_pending(current) will be true, exiting the loop. In this case, timeout will be some non-negative value (0 is a corner case, I believe, where both the timer fires and a signal is received in the last jiffy).
b) The timer in schedule_timeout() has expired and thus it will return 0. This indicates the function has delayed the requested time (at least) and timeout will be set to 0, again exiting the loop.
Quoted 5 lines
Clearly, then, if my interpretion is correct, schedule_timeout() will always return to a state in msleep_interruptible() which causes the loop to only iterate the one time. Does this make sense? Is my interpretation of schedule_timeout()s functioning somehow flawed? If not, we probably can go ahead and change the msleep_interruptible() code, yes?

Quoted 4 lines

Ok, since nobody seems to have objected to my ideas as of yet, I went ahead and implemented them, with much help from Domen and others. I believe there are two options for these changes. One involves using macros instead, the other involves reworking the functions.

Here is patch 2/2, which modifies include/linux/delay.h to contain the appropriate macros. Much consideration was given the various names and calling syntax, but corrections/requests/changes are welcome.

-Nish

Description: Remove prototypes of msleep() and msleep_interruptible() to prepare for the macro versions of these functions. Add macros for 4 types of sleeps:
1) Unconditional sleep (msleep)
2) Sleep until signalled (msleep_interruptible)
3) Sleep until wait-queue event (msleep_wq)
4) Sleep until either signalled or wait-queue event (msleep_wq_interruptible)
These 4 cases are all distinct and handled separately by passing appropriate flags to __msleep_sig and __msleep_wq, which should *not* ever be called directly by kernel code.

Signed-off-by Nishanth Aravamudan <[email protected]>

include/linux/delay.h +50 -2

@@ -38,8 +38,56 @@ extern unsigned long loops_per_jiffy;
 #define ndelay(x)	udelay(((x)+999)/1000)
 #endif
 
-void msleep(unsigned int msecs);
-unsigned long msleep_interruptible(unsigned int msecs);
+/*
+ * Sleep in state while condition is true
+ */
+#define __msleep_sig(unsigned int msecs, long state, int condition)	\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		while (timeout && condition) {				\
+ 			set_current_state(state);			\
+			timeout = schedule_timeout(timeout);		\
+		}							\
+									\
+		return jiffies_to_msecs(timeout);			\
+	} while(0)
+
+/*
+ * Sleep in state until schedule_timeout() returns
+ */
+#define __msleep_wq(unsigned int msecs, long state)			\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		set_current_state(condition);				\
+		timeout = schedule_timeout(timeout);			\
+									\
+		return jiffies_to_msecs(timeout);			\
+	} while(0)
+
+/*
+ * Sleep until at least msecs ms have elapsed
+ */ 
+#define msleep(msecs) (void)__msleep_sig(msecs, TASK_UNINTERRUPTIBLE, 1)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a signal is received
+ */
+#define msleep_interruptible(msecs)		\
+	__msleep_sig(msecs, TASK_INTERRUPTIBLE, !signals_pending(current))
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ */
+#define msleep_wq(msecs) __msleep_wq_state(msecs, TASK_UNINTERRUPTIBLE)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ * or a signal is received
+ */
+#define msleep_wq_interruptible(msecs)		\
+	__msleep_wq_state(msecs, TASK_INTERRUPTIBLE);
 
 static inline void ssleep(unsigned int seconds)
 {

Nishanth Aravamudan

author

2004-11-20 01:27 UTC #3 lore raw

PATCH 2/2 include/delay.h: replace msleep() and msleep_interruptible() with macros

Quoted 105 lines · Nishanth Aravamudan

Quoted 58 lines · Nishanth Aravamudan
Quoted 51 lines · Nishanth Aravamudan
Hi,
After some pretty heavy discussion on IRC, I felt that it may be important / useful to bring the discussion of schedule_timeout() to LKML. There are two issues being considered:
1) msleep_interruptible()
For reference, here is the code for this function:
/**
* msleep_interruptible - sleep waiting for waitqueue interruptions
* @msecs: Time in milliseconds to sleep for
*/
unsigned long msleep_interruptible(unsigned int msecs)
{
	unsigned long timeout = msecs_to_jiffies(msecs);

	while (timeout && !signal_pending(current)) {
		set_current_state(TASK_INTERRUPTIBLE);
		timeout = schedule_timeout(timeout);
	}
	return jiffies_to_msecs(timeout);
}
The first issue deals with whether the while() loop is at all necessary. From my understanding (primarily from how the code "should" behave, but also backed up by code itself), I think the following code:
	set_current_state(TASK_INTERRUPTIBLE);
	timeout = schedule_timeout(timeout);
should be interpreted as:
a) I wish to sleep for timeout jiffies; however
b) If a signal occurs before timeout jiffies have gone by, I would also like to wake up.
With this interpretation, though, the while()-conditional becomes questionable. I can see two cases (think inclusive OR not exclusive) for schedule_timeout() returning:
a) A signal was received and thus signal_pending(current) will be true, exiting the loop. In this case, timeout will be some non-negative value (0 is a corner case, I believe, where both the timer fires and a signal is received in the last jiffy).
b) The timer in schedule_timeout() has expired and thus it will return 0. This indicates the function has delayed the requested time (at least) and timeout will be set to 0, again exiting the loop.
Quoted 5 lines
Clearly, then, if my interpretion is correct, schedule_timeout() will always return to a state in msleep_interruptible() which causes the loop to only iterate the one time. Does this make sense? Is my interpretation of schedule_timeout()s functioning somehow flawed? If not, we probably can go ahead and change the msleep_interruptible() code, yes?
Quoted 4 lines
Ok, since nobody seems to have objected to my ideas as of yet, I went ahead and implemented them, with much help from Domen and others. I believe there are two options for these changes. One involves using macros instead, the other involves reworking the functions.
Here is patch 2/2, which modifies include/linux/delay.h to contain the appropriate macros. Much consideration was given the various names and calling syntax, but corrections/requests/changes are welcome.
-Nish
Description: Remove prototypes of msleep() and msleep_interruptible() to prepare for the macro versions of these functions. Add macros for 4 types of sleeps:
1) Unconditional sleep (msleep)
2) Sleep until signalled (msleep_interruptible)
3) Sleep until wait-queue event (msleep_wq)
4) Sleep until either signalled or wait-queue event (msleep_wq_interruptible)
These 4 cases are all distinct and handled separately by passing appropriate flags to __msleep_sig and __msleep_wq, which should *not* ever be called directly by kernel code.
Signed-off-by Nishanth Aravamudan <[email protected]>
include/linux/delay.h +13 -2
@@ -38,8 +38,56 @@ extern unsigned long loops_per_jiffy;
 #define ndelay(x)	udelay(((x)+999)/1000)
 #endif
 
-void msleep(unsigned int msecs);
-unsigned long msleep_interruptible(unsigned int msecs);
+/*
+ * Sleep in state while condition is true
+ */
+#define __msleep_sig(unsigned int msecs, long state, int condition)	\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		while (timeout && condition) {				\
+ 			set_current_state(state);			\
+			timeout = schedule_timeout(timeout);		\
+		}							\
+									\
+		return jiffies_to_msecs(timeout);			\

include/linux/delay.h extern unsigned long loops_per_jiffy; · line 63

Quoted 10 lines

include/linux/delay.h +10 -0

+	} while(0)
+
+/*
+ * Sleep in state until schedule_timeout() returns
+ */
+#define __msleep_wq(unsigned int msecs, long state)			\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		set_current_state(condition);				\

Obvious typo, fixed in patch below...

Signed-off-by Nishanth Aravamudan <[email protected]>

include/linux/delay.h +50 -2

@@ -38,8 +38,56 @@ extern unsigned long loops_per_jiffy;
 #define ndelay(x)	udelay(((x)+999)/1000)
 #endif
 
-void msleep(unsigned int msecs);
-unsigned long msleep_interruptible(unsigned int msecs);
+/*
+ * Sleep in state while condition is true
+ */
+#define __msleep_sig(unsigned int msecs, long state, int condition)	\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		while (timeout && condition) {				\
+ 			set_current_state(state);			\
+			timeout = schedule_timeout(timeout);		\
+		}							\
+									\
+		return jiffies_to_msecs(timeout);			\
+	} while(0)
+
+/*
+ * Sleep in state until schedule_timeout() returns
+ */
+#define __msleep_wq(unsigned int msecs, long state)			\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		set_current_state(state);				\
+		timeout = schedule_timeout(timeout);			\
+									\
+		return jiffies_to_msecs(timeout);			\
+	} while(0)
+
+/*
+ * Sleep until at least msecs ms have elapsed
+ */ 
+#define msleep(msecs) (void)__msleep_sig(msecs, TASK_UNINTERRUPTIBLE, 1)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a signal is received
+ */
+#define msleep_interruptible(msecs)		\
+	__msleep_sig(msecs, TASK_INTERRUPTIBLE, !signals_pending(current))
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ */
+#define msleep_wq(msecs) __msleep_wq_state(msecs, TASK_UNINTERRUPTIBLE)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ * or a signal is received
+ */
+#define msleep_wq_interruptible(msecs)		\
+	__msleep_wq_state(msecs, TASK_INTERRUPTIBLE);
 
 static inline void ssleep(unsigned int seconds)
 {

Nishanth Aravamudan

author

2004-11-20 01:30 UTC #4 lore raw

PATCH 2/2 include/delay.h: replace msleep() and msleep_interruptible() with macros

Quoted 183 lines · Nishanth Aravamudan

Quoted 105 lines · Nishanth Aravamudan
Quoted 58 lines · Nishanth Aravamudan
Quoted 51 lines · Nishanth Aravamudan
Hi,
After some pretty heavy discussion on IRC, I felt that it may be important / useful to bring the discussion of schedule_timeout() to LKML. There are two issues being considered:
1) msleep_interruptible()
For reference, here is the code for this function:
/**
* msleep_interruptible - sleep waiting for waitqueue interruptions
* @msecs: Time in milliseconds to sleep for
*/
unsigned long msleep_interruptible(unsigned int msecs)
{
	unsigned long timeout = msecs_to_jiffies(msecs);

	while (timeout && !signal_pending(current)) {
		set_current_state(TASK_INTERRUPTIBLE);
		timeout = schedule_timeout(timeout);
	}
	return jiffies_to_msecs(timeout);
}
The first issue deals with whether the while() loop is at all necessary. From my understanding (primarily from how the code "should" behave, but also backed up by code itself), I think the following code:
	set_current_state(TASK_INTERRUPTIBLE);
	timeout = schedule_timeout(timeout);
should be interpreted as:
a) I wish to sleep for timeout jiffies; however
b) If a signal occurs before timeout jiffies have gone by, I would also like to wake up.
With this interpretation, though, the while()-conditional becomes questionable. I can see two cases (think inclusive OR not exclusive) for schedule_timeout() returning:
a) A signal was received and thus signal_pending(current) will be true, exiting the loop. In this case, timeout will be some non-negative value (0 is a corner case, I believe, where both the timer fires and a signal is received in the last jiffy).
b) The timer in schedule_timeout() has expired and thus it will return 0. This indicates the function has delayed the requested time (at least) and timeout will be set to 0, again exiting the loop.
Quoted 5 lines
Clearly, then, if my interpretion is correct, schedule_timeout() will always return to a state in msleep_interruptible() which causes the loop to only iterate the one time. Does this make sense? Is my interpretation of schedule_timeout()s functioning somehow flawed? If not, we probably can go ahead and change the msleep_interruptible() code, yes?
Quoted 4 lines
Ok, since nobody seems to have objected to my ideas as of yet, I went ahead and implemented them, with much help from Domen and others. I believe there are two options for these changes. One involves using macros instead, the other involves reworking the functions.
Here is patch 2/2, which modifies include/linux/delay.h to contain the appropriate macros. Much consideration was given the various names and calling syntax, but corrections/requests/changes are welcome.
-Nish
Description: Remove prototypes of msleep() and msleep_interruptible() to prepare for the macro versions of these functions. Add macros for 4 types of sleeps:
1) Unconditional sleep (msleep)
2) Sleep until signalled (msleep_interruptible)
3) Sleep until wait-queue event (msleep_wq)
4) Sleep until either signalled or wait-queue event (msleep_wq_interruptible)
These 4 cases are all distinct and handled separately by passing appropriate flags to __msleep_sig and __msleep_wq, which should *not* ever be called directly by kernel code.
Signed-off-by Nishanth Aravamudan <[email protected]>
include/linux/delay.h +13 -2
@@ -38,8 +38,56 @@ extern unsigned long loops_per_jiffy;
 #define ndelay(x)	udelay(((x)+999)/1000)
 #endif
 
-void msleep(unsigned int msecs);
-unsigned long msleep_interruptible(unsigned int msecs);
+/*
+ * Sleep in state while condition is true
+ */
+#define __msleep_sig(unsigned int msecs, long state, int condition)	\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		while (timeout && condition) {				\
+ 			set_current_state(state);			\
+			timeout = schedule_timeout(timeout);		\
+		}							\
+									\
+		return jiffies_to_msecs(timeout);			\
include/linux/delay.h extern unsigned long loops_per_jiffy; · line 63
Quoted 10 lines
include/linux/delay.h +10 -0
+	} while(0)
+
+/*
+ * Sleep in state until schedule_timeout() returns
+ */
+#define __msleep_wq(unsigned int msecs, long state)			\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		set_current_state(condition);				\
Obvious typo, fixed in patch below...
Description: Remove prototypes of msleep() and msleep_interruptible() to prepare for the macro versions of these functions. Add macros for 4 types of sleeps:
1) Unconditional sleep (msleep)
2) Sleep until signalled (msleep_interruptible)
3) Sleep until wait-queue event (msleep_wq)
4) Sleep until either signalled or wait-queue event (msleep_wq_interruptible)
These 4 cases are all distinct and handled separately by passing appropriate flags to __msleep_sig and __msleep_wq, which should *not* ever be called directly by kernel code.
Signed-off-by Nishanth Aravamudan <[email protected]>
include/linux/delay.h +40 -2
@@ -38,8 +38,56 @@ extern unsigned long loops_per_jiffy;
 #define ndelay(x)	udelay(((x)+999)/1000)
 #endif
 
-void msleep(unsigned int msecs);
-unsigned long msleep_interruptible(unsigned int msecs);
+/*
+ * Sleep in state while condition is true
+ */
+#define __msleep_sig(unsigned int msecs, long state, int condition)	\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		while (timeout && condition) {				\
+ 			set_current_state(state);			\
+			timeout = schedule_timeout(timeout);		\
+		}							\
+									\
+		return jiffies_to_msecs(timeout);			\
+	} while(0)
+
+/*
+ * Sleep in state until schedule_timeout() returns
+ */
+#define __msleep_wq(unsigned int msecs, long state)			\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		set_current_state(state);				\
+		timeout = schedule_timeout(timeout);			\
+									\
+		return jiffies_to_msecs(timeout);			\
+	} while(0)
+
+/*
+ * Sleep until at least msecs ms have elapsed
+ */ 
+#define msleep(msecs) (void)__msleep_sig(msecs, TASK_UNINTERRUPTIBLE, 1)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a signal is received
+ */
+#define msleep_interruptible(msecs)		\
+	__msleep_sig(msecs, TASK_INTERRUPTIBLE, !signals_pending(current))
+
+/*

include/linux/delay.h extern unsigned long loops_per_jiffy; · line 90

Quoted 10 lines

include/linux/delay.h +10 -0

+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ */
+#define msleep_wq(msecs) __msleep_wq_state(msecs, TASK_UNINTERRUPTIBLE)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ * or a signal is received
+ */
+#define msleep_wq_interruptible(msecs)		\
+	__msleep_wq_state(msecs, TASK_INTERRUPTIBLE);

Another stupid typo.

Signed-off-by Nishanth Aravamudan <[email protected]>

include/linux/delay.h +50 -2

@@ -38,8 +38,56 @@ extern unsigned long loops_per_jiffy;
 #define ndelay(x)	udelay(((x)+999)/1000)
 #endif
 
-void msleep(unsigned int msecs);
-unsigned long msleep_interruptible(unsigned int msecs);
+/*
+ * Sleep in state while condition is true
+ */
+#define __msleep_sig(unsigned int msecs, long state, int condition)	\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		while (timeout && condition) {				\
+ 			set_current_state(state);			\
+			timeout = schedule_timeout(timeout);		\
+		}							\
+									\
+		return jiffies_to_msecs(timeout);			\
+	} while(0)
+
+/*
+ * Sleep in state until schedule_timeout() returns
+ */
+#define __msleep_wq(unsigned int msecs, long state)			\
+	do {								\
+		unsigned long timeout = msecs_to_jiffies(msecs) + 1;	\
+									\
+		set_current_state(condition);				\
+		timeout = schedule_timeout(timeout);			\
+									\
+		return jiffies_to_msecs(timeout);			\
+	} while(0)
+
+/*
+ * Sleep until at least msecs ms have elapsed
+ */ 
+#define msleep(msecs) (void)__msleep_sig(msecs, TASK_UNINTERRUPTIBLE, 1)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a signal is received
+ */
+#define msleep_interruptible(msecs)		\
+	__msleep_sig(msecs, TASK_INTERRUPTIBLE, !signals_pending(current))
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ */
+#define msleep_wq(msecs) __msleep_wq_state(msecs, TASK_UNINTERRUPTIBLE)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ * or a signal is received
+ */
+#define msleep_wq_interruptible(msecs)		\
+	__msleep_wq_state(msecs, TASK_INTERRUPTIBLE)
 
 static inline void ssleep(unsigned int seconds)
 {

A message that is not in the archive (sent off-list or not yet received): [email protected]

Nishanth Aravamudan

author

2004-11-22 18:05 UTC #6 lore raw

Re: [PATCH 2/2] include/delay.h: replace msleep() and msleep_interruptible() with macros

Quoted 8 lines · Oliver Neukum

Am Samstag, 20. November 2004 02:23 schrieb Nishanth Aravamudan:
Quoted 3 lines
Description: Remove prototypes of msleep() and msleep_interruptible() to prepare for the macro versions of these functions. Add macros for 4 types of sleeps:
What is the purpose of having macros for delay?
They are on a slow path by definition. You want the smallest possible function call here, nothing fancy.

Just so I'm clear on what you are asking... Do you mean why am I using macros or why the macros are needed at all?

To the former, I am more than happy to change them to functions, and, in fact, I believe I have to for modules (EXPORT_SYMBOL_GPL?) to be able to call the sleep functions.

To the latter, having these 4 functions/macros makes it clear for what reason you are sleeping. It leads to *correct* functionality of the code, which we do not currently have.

-Nish

A message that is not in the archive (sent off-list or not yet received): [email protected]

Nishanth Aravamudan

author

2004-11-22 19:52 UTC #8 lore raw

PATCH 1/1 kernel/timer: correct msleep_interruptible() comment

Quoted 20 lines · Oliver Neukum

Am Montag, 22. November 2004 19:01 schrieb Nishanth Aravamudan:
Quoted 6 lines · Oliver Neukum
Quoted 5 lines
Am Samstag, 20. November 2004 02:23 schrieb Nishanth Aravamudan:
Quoted 3 lines
Description: Remove prototypes of msleep() and msleep_interruptible() to prepare for the macro versions of these functions. Add macros for 4 types of sleeps:
Quoted 6 lines
Quoted 3 lines
What is the purpose of having macros for delay?
They are on a slow path by definition. You want the smallest possible function call here, nothing fancy.
Just so I'm clear on what you are asking... Do you mean why am I using macros or why the macros are needed at all?
Yes, they should be functions.
Quoted 4 lines
To the former, I am more than happy to change them to functions, and, in fact, I believe I have to for modules (EXPORT_SYMBOL_GPL?) to be able to call the sleep functions.

Quoted 6 lines

Quoted 3 lines
To the latter, having these 4 functions/macros makes it clear for what reason you are sleeping. It leads to *correct* functionality of the code, which we do not currently have.
But two of them are redundant. You are reinventing the wait_event_* family.

Fair enough. I was not aware of the various wait_event_*timeout() functions. Thanks for beating me over the head with this. Kind of shoots down my fancy patch though :) Here's one that at least corrects the comment that made all of this happen...

-Nish

Description: The comment for msleep_interruptible() incorrectly asserts that the function takes into consideration wait-queue interruptions. The wait_event_*() family of functions should be used in those cases. msleep_interruptible() is only to be used when sleeping waiting for signals.

Signed-off-by Nishanth Aravamudan <[email protected]>

kernel/timer.c +1 -1

@@ -1627,7 +1627,7 @@ void msleep(unsigned int msecs)
 EXPORT_SYMBOL(msleep);
 
 /**
- * msleep_interruptible - sleep waiting for waitqueue interruptions
+ * msleep_interruptible - sleep waiting for signals
  * @msecs: Time in milliseconds to sleep for
  */
 unsigned long msleep_interruptible(unsigned int msecs)

Nishanth Aravamudan

author

2004-11-20 01:01 UTC #9 lore raw

PATCH 1/2 kernel/timer.c: remove msleep() and msleep_interruptible()

In reply to Nishanth Aravamudan

Quoted 51 lines · Nishanth Aravamudan

Hi,
After some pretty heavy discussion on IRC, I felt that it may be important / useful to bring the discussion of schedule_timeout() to LKML. There are two issues being considered:
1) msleep_interruptible()
For reference, here is the code for this function:
/**
* msleep_interruptible - sleep waiting for waitqueue interruptions
* @msecs: Time in milliseconds to sleep for
*/
unsigned long msleep_interruptible(unsigned int msecs)
{
	unsigned long timeout = msecs_to_jiffies(msecs);

	while (timeout && !signal_pending(current)) {
		set_current_state(TASK_INTERRUPTIBLE);
		timeout = schedule_timeout(timeout);
	}
	return jiffies_to_msecs(timeout);
}
The first issue deals with whether the while() loop is at all necessary. From my understanding (primarily from how the code "should" behave, but also backed up by code itself), I think the following code:
	set_current_state(TASK_INTERRUPTIBLE);
	timeout = schedule_timeout(timeout);
should be interpreted as:
a) I wish to sleep for timeout jiffies; however
b) If a signal occurs before timeout jiffies have gone by, I would also like to wake up.
With this interpretation, though, the while()-conditional becomes questionable. I can see two cases (think inclusive OR not exclusive) for schedule_timeout() returning:
a) A signal was received and thus signal_pending(current) will be true, exiting the loop. In this case, timeout will be some non-negative value (0 is a corner case, I believe, where both the timer fires and a signal is received in the last jiffy).
b) The timer in schedule_timeout() has expired and thus it will return 0. This indicates the function has delayed the requested time (at least) and timeout will be set to 0, again exiting the loop.

Quoted 5 lines

Clearly, then, if my interpretion is correct, schedule_timeout() will always return to a state in msleep_interruptible() which causes the loop to only iterate the one time. Does this make sense? Is my interpretation of schedule_timeout()s functioning somehow flawed? If not, we probably can go ahead and change the msleep_interruptible() code, yes?

Forgot signed-off-by line...

I have attached here the version with macros (1/2). I am sure there will be much to correct, so please do so.

Also, I will hopefully submit soon a change to a driver that uses this new msleep*() interface, so as to show how it should work.

-Nish

Signed-off-by Nishanth Aravamudan <[email protected]>

kernel/timer.c +0 ~~-33~~

@@ -1609,36 +1609,3 @@ unregister_time_interpolator(struct time
 	spin_unlock(&time_interpolator_lock);
 }
 #endif /* CONFIG_TIME_INTERPOLATION */
-
-/**
- * msleep - sleep safely even with waitqueue interruptions
- * @msecs: Time in milliseconds to sleep for
- */
-void msleep(unsigned int msecs)
-{
-	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
-
-	while (timeout) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		timeout = schedule_timeout(timeout);
-	}
-}
-
-EXPORT_SYMBOL(msleep);
-
-/**
- * msleep_interruptible - sleep waiting for waitqueue interruptions
- * @msecs: Time in milliseconds to sleep for
- */
-unsigned long msleep_interruptible(unsigned int msecs)
-{
-	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
-
-	while (timeout && !signal_pending(current)) {
-		set_current_state(TASK_INTERRUPTIBLE);
-		timeout = schedule_timeout(timeout);
-	}
-	return jiffies_to_msecs(timeout);
-}
-
-EXPORT_SYMBOL(msleep_interruptible);

Nishanth Aravamudan

author

2004-11-20 01:21 UTC #10 lore raw

PATCH 1/2 kernel/timer: remove msleep{,_interruptible}(); add __msleep_{sig,wq}()

In reply to Nishanth Aravamudan

Quoted 51 lines · Nishanth Aravamudan

Hi,
After some pretty heavy discussion on IRC, I felt that it may be important / useful to bring the discussion of schedule_timeout() to LKML. There are two issues being considered:
1) msleep_interruptible()
For reference, here is the code for this function:
/**
* msleep_interruptible - sleep waiting for waitqueue interruptions
* @msecs: Time in milliseconds to sleep for
*/
unsigned long msleep_interruptible(unsigned int msecs)
{
	unsigned long timeout = msecs_to_jiffies(msecs);

	while (timeout && !signal_pending(current)) {
		set_current_state(TASK_INTERRUPTIBLE);
		timeout = schedule_timeout(timeout);
	}
	return jiffies_to_msecs(timeout);
}
The first issue deals with whether the while() loop is at all necessary. From my understanding (primarily from how the code "should" behave, but also backed up by code itself), I think the following code:
	set_current_state(TASK_INTERRUPTIBLE);
	timeout = schedule_timeout(timeout);
should be interpreted as:
a) I wish to sleep for timeout jiffies; however
b) If a signal occurs before timeout jiffies have gone by, I would also like to wake up.
With this interpretation, though, the while()-conditional becomes questionable. I can see two cases (think inclusive OR not exclusive) for schedule_timeout() returning:
a) A signal was received and thus signal_pending(current) will be true, exiting the loop. In this case, timeout will be some non-negative value (0 is a corner case, I believe, where both the timer fires and a signal is received in the last jiffy).
b) The timer in schedule_timeout() has expired and thus it will return 0. This indicates the function has delayed the requested time (at least) and timeout will be set to 0, again exiting the loop.

Quoted 5 lines

Clearly, then, if my interpretion is correct, schedule_timeout() will always return to a state in msleep_interruptible() which causes the loop to only iterate the one time. Does this make sense? Is my interpretation of schedule_timeout()s functioning somehow flawed? If not, we probably can go ahead and change the msleep_interruptible() code, yes?

Here is the function variant of the previous patches.

Description: Remove msleep() and msleep_interruptible(). Add function definitions for __msleep_sig() and __msleep_wq(). These functions are the "hidden" backends to the new implementations of the msleep() class of functions. They should avoid many of the problems with the existing system by being clearer on what they are sleeping for.

Signed-off-by Nishanth Aravamudan <[email protected]>

kernel/timer.c +15 ~~-20~~

@@ -1610,35 +1610,30 @@ unregister_time_interpolator(struct time
 }
 #endif /* CONFIG_TIME_INTERPOLATION */
 
-/**
- * msleep - sleep safely even with waitqueue interruptions
- * @msecs: Time in milliseconds to sleep for
+/*
+ * Sleep in state and, if check sigs, until signal received
  */
-void msleep(unsigned int msecs)
+unsigned int __msleep_sig(unsigned int msecs, long state, int check_sigs)
 {
 	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
-
-	while (timeout) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
+	
+	while (timeout && (check_sigs ? !signals_pending(current) : 1)) {
+		set_current_state(state);
 		timeout = schedule_timeout(timeout);
 	}
-}
 
-EXPORT_SYMBOL(msleep);
+	return jiffies_to_msecs(timeout);
+}
 
-/**
- * msleep_interruptible - sleep waiting for waitqueue interruptions
- * @msecs: Time in milliseconds to sleep for
+/*
+ * Sleep in state until schedule_timeout() returns
  */
-unsigned long msleep_interruptible(unsigned int msecs)
+unsigned int __msleep_wq(unsigned int msecs, long state)
 {
 	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
-
-	while (timeout && !signal_pending(current)) {
-		set_current_state(TASK_INTERRUPTIBLE);
-		timeout = schedule_timeout(timeout);
-	}
+	
+	set_current_state(state);
+	timeout = schedule_timeout(timeout);
+	
 	return jiffies_to_msecs(timeout);
 }
-
-EXPORT_SYMBOL(msleep_interruptible);

Nishanth Aravamudan

author

2004-11-20 01:25 UTC #11 lore raw

PATCH 2/2 include/delay: replace msleep{,_interruptible}() with macros

Quoted 52 lines · Nishanth Aravamudan

Quoted 51 lines · Nishanth Aravamudan
Hi,
After some pretty heavy discussion on IRC, I felt that it may be important / useful to bring the discussion of schedule_timeout() to LKML. There are two issues being considered:
1) msleep_interruptible()
For reference, here is the code for this function:
/**
* msleep_interruptible - sleep waiting for waitqueue interruptions
* @msecs: Time in milliseconds to sleep for
*/
unsigned long msleep_interruptible(unsigned int msecs)
{
	unsigned long timeout = msecs_to_jiffies(msecs);

	while (timeout && !signal_pending(current)) {
		set_current_state(TASK_INTERRUPTIBLE);
		timeout = schedule_timeout(timeout);
	}
	return jiffies_to_msecs(timeout);
}
The first issue deals with whether the while() loop is at all necessary. From my understanding (primarily from how the code "should" behave, but also backed up by code itself), I think the following code:
	set_current_state(TASK_INTERRUPTIBLE);
	timeout = schedule_timeout(timeout);
should be interpreted as:
a) I wish to sleep for timeout jiffies; however
b) If a signal occurs before timeout jiffies have gone by, I would also like to wake up.
With this interpretation, though, the while()-conditional becomes questionable. I can see two cases (think inclusive OR not exclusive) for schedule_timeout() returning:
a) A signal was received and thus signal_pending(current) will be true, exiting the loop. In this case, timeout will be some non-negative value (0 is a corner case, I believe, where both the timer fires and a signal is received in the last jiffy).
b) The timer in schedule_timeout() has expired and thus it will return 0. This indicates the function has delayed the requested time (at least) and timeout will be set to 0, again exiting the loop.

Quoted 7 lines

Quoted 5 lines
Clearly, then, if my interpretion is correct, schedule_timeout() will always return to a state in msleep_interruptible() which causes the loop to only iterate the one time. Does this make sense? Is my interpretation of schedule_timeout()s functioning somehow flawed? If not, we probably can go ahead and change the msleep_interruptible() code, yes?
Here is the function variant of the previous patches.

Here is the corresponding change to delay.h. The major difference from the macro version is that a flag is now passed to __msleep_sig() to determine whether an extra condition is necessary in the while() loop.

-Nish

Description: Add macros for various sleep functions, one for each case of:

Description: Remove prototypes of msleep() and msleep_interruptible() to prepare for the macro versions of these functions. Add prototypes for __msleep_wq() and __msleep_sig(). Add macros for 4 types of sleeps:
1) Unconditional sleep (msleep)
2) Sleep until signalled (msleep_interruptible)
3) Sleep until wait-queue event (msleep_wq)
4) Sleep until either signalled or wait-queue event (msleep_wq_interruptible)
These 4 cases are all distinct and handled separately by passing appropriate flags to __msleep_sig and __msleep_wq, which should *not* ever be called directly by kernel code.

Signed-off-by Nishanth Aravamudan <[email protected]>

include/linux/delay.h +25 -2

@@ -38,8 +38,31 @@ extern unsigned long loops_per_jiffy;
 #define ndelay(x)	udelay(((x)+999)/1000)
 #endif
 
-void msleep(unsigned int msecs);
-unsigned long msleep_interruptible(unsigned int msecs);
+unsigned int __msleep_sig(unsigned int msecs, long state, int check_sigs);
+unsigned int __msleep_wq(unsigned int msecs, long state);
+
+/*
+ * Sleep until at least msecs ms have elapsed
+ */ 
+#define msleep(msecs) (void)__msleep_sig(msecs, TASK_UNINTERRUPTIBLE, 0)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a signal is received
+ */
+#define msleep_interruptible(msecs)		\
+	__msleep_sig(msecs, TASK_INTERRUPTIBLE, 1))
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ */
+#define msleep_wq(msecs) __msleep_wq_state(msecs, TASK_UNINTERRUPTIBLE)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ * or a signal is received
+ */
+#define msleep_wq_interruptible(msecs)		\
+	__msleep_wq_state(msecs, TASK_INTERRUPTIBLE);
 
 static inline void ssleep(unsigned int seconds)
 {

Nishanth Aravamudan

author

2004-11-20 01:30 UTC #12 lore raw

PATCH 2/2 include/delay: replace msleep{,_interruptible}() with macros

Quoted 109 lines · Nishanth Aravamudan

Quoted 52 lines · Nishanth Aravamudan
Quoted 51 lines · Nishanth Aravamudan
Hi,
After some pretty heavy discussion on IRC, I felt that it may be important / useful to bring the discussion of schedule_timeout() to LKML. There are two issues being considered:
1) msleep_interruptible()
For reference, here is the code for this function:
/**
* msleep_interruptible - sleep waiting for waitqueue interruptions
* @msecs: Time in milliseconds to sleep for
*/
unsigned long msleep_interruptible(unsigned int msecs)
{
	unsigned long timeout = msecs_to_jiffies(msecs);

	while (timeout && !signal_pending(current)) {
		set_current_state(TASK_INTERRUPTIBLE);
		timeout = schedule_timeout(timeout);
	}
	return jiffies_to_msecs(timeout);
}
The first issue deals with whether the while() loop is at all necessary. From my understanding (primarily from how the code "should" behave, but also backed up by code itself), I think the following code:
	set_current_state(TASK_INTERRUPTIBLE);
	timeout = schedule_timeout(timeout);
should be interpreted as:
a) I wish to sleep for timeout jiffies; however
b) If a signal occurs before timeout jiffies have gone by, I would also like to wake up.
With this interpretation, though, the while()-conditional becomes questionable. I can see two cases (think inclusive OR not exclusive) for schedule_timeout() returning:
a) A signal was received and thus signal_pending(current) will be true, exiting the loop. In this case, timeout will be some non-negative value (0 is a corner case, I believe, where both the timer fires and a signal is received in the last jiffy).
b) The timer in schedule_timeout() has expired and thus it will return 0. This indicates the function has delayed the requested time (at least) and timeout will be set to 0, again exiting the loop.
Quoted 7 lines
Quoted 5 lines
Clearly, then, if my interpretion is correct, schedule_timeout() will always return to a state in msleep_interruptible() which causes the loop to only iterate the one time. Does this make sense? Is my interpretation of schedule_timeout()s functioning somehow flawed? If not, we probably can go ahead and change the msleep_interruptible() code, yes?
Here is the function variant of the previous patches.
Here is the corresponding change to delay.h. The major difference from the macro version is that a flag is now passed to __msleep_sig() to determine whether an extra condition is necessary in the while() loop.
-Nish
Description: Add macros for various sleep functions, one for each case of:
Description: Remove prototypes of msleep() and msleep_interruptible() to prepare for the macro versions of these functions. Add prototypes for __msleep_wq() and __msleep_sig(). Add macros for 4 types of sleeps:
1) Unconditional sleep (msleep)
2) Sleep until signalled (msleep_interruptible)
3) Sleep until wait-queue event (msleep_wq)
4) Sleep until either signalled or wait-queue event (msleep_wq_interruptible)
These 4 cases are all distinct and handled separately by passing appropriate flags to __msleep_sig and __msleep_wq, which should *not* ever be called directly by kernel code.
Signed-off-by Nishanth Aravamudan <[email protected]>
include/linux/delay.h +15 -2
@@ -38,8 +38,31 @@ extern unsigned long loops_per_jiffy;
 #define ndelay(x)	udelay(((x)+999)/1000)
 #endif
 
-void msleep(unsigned int msecs);
-unsigned long msleep_interruptible(unsigned int msecs);
+unsigned int __msleep_sig(unsigned int msecs, long state, int check_sigs);
+unsigned int __msleep_wq(unsigned int msecs, long state);
+
+/*
+ * Sleep until at least msecs ms have elapsed
+ */ 
+#define msleep(msecs) (void)__msleep_sig(msecs, TASK_UNINTERRUPTIBLE, 0)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a signal is received
+ */
+#define msleep_interruptible(msecs)		\
+	__msleep_sig(msecs, TASK_INTERRUPTIBLE, 1))
+
+/*

include/linux/delay.h extern unsigned long loops_per_jiffy; · line 65

Quoted 10 lines

include/linux/delay.h +10 -0

+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ */
+#define msleep_wq(msecs) __msleep_wq_state(msecs, TASK_UNINTERRUPTIBLE)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ * or a signal is received
+ */
+#define msleep_wq_interruptible(msecs)		\
+	__msleep_wq_state(msecs, TASK_INTERRUPTIBLE);

A stupid typo...

Signed-off-by Nishanth Aravamudan <[email protected]>

include/linux/delay.h +25 -2

@@ -38,8 +38,31 @@ extern unsigned long loops_per_jiffy;
 #define ndelay(x)	udelay(((x)+999)/1000)
 #endif
 
-void msleep(unsigned int msecs);
-unsigned long msleep_interruptible(unsigned int msecs);
+unsigned int __msleep_sig(unsigned int msecs, long state, int check_sigs);
+unsigned int __msleep_wq(unsigned int msecs, long state);
+
+/*
+ * Sleep until at least msecs ms have elapsed
+ */ 
+#define msleep(msecs) (void)__msleep_sig(msecs, TASK_UNINTERRUPTIBLE, 0)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a signal is received
+ */
+#define msleep_interruptible(msecs)		\
+	__msleep_sig(msecs, TASK_INTERRUPTIBLE, 1))
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ */
+#define msleep_wq(msecs) __msleep_wq_state(msecs, TASK_UNINTERRUPTIBLE)
+
+/*
+ * Sleep until at least msecs ms have elapsed or a wait-queue event occurs
+ * or a signal is received
+ */
+#define msleep_wq_interruptible(msecs)		\
+	__msleep_wq_state(msecs, TASK_INTERRUPTIBLE)
 
 static inline void ssleep(unsigned int seconds)
 {