Re: Add msleep_interruptible() function to kernel/timer.c

Re: Add msleep_interruptible() function to kernel/timer.c

[Alan Cox]

On Sul, 2004-08-15 at 13:18, maximilian attems wrote:
> + * msleep_interruptible - sleep waiting for waitqueue interruptions
> + * @msecs: Time in milliseconds to sleep for
> + */
> +void msleep_interruptible(unsigned int msecs)
> +{
> +	unsigned long timeout = msecs_to_jiffies(msecs);
> +
> +	while (timeout) {

You want to have while(timeout && !signal_pending(current))

A signal will wake the timeout which will then loop. It might also
be good to add

> +		set_current_state(TASK_INTERRUPTIBLE);
> +		timeout = schedule_timeout(timeout);
> +	}

return timeout;

so that the caller knows more about how long the timer ran for before
the interrupt and if it was interrupted.

Add msleep_interruptible() function to kernel/timer.c

[maximilian attems]

while doing kernel-janitor msleep conversion of drivers own (incorrect),
functions, some places would need msleep_interruptible().

this function is equivalent to:
        current->state = TASK_INTERRUPTIBLE;
	schedule_timeout(timeout);

idea from Ingo Molnar:
well, aboves is not 100% equivalent because msleep() is uninterruptible so
stoppage of the md thread (upon shutdown) will occur with only a 250 msec
delay. Someone should add a msleep_interruptible() function to kernel/timer.c.

Signed-off-by: Maximilian Attems <janitor@sternwelten.at>


---

 linux-2.6.8-max/kernel/timer.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+)

diff -puN kernel/timer.c~add-msleep_interruptible kernel/timer.c
--- linux-2.6.8/kernel/timer.c~add-msleep_interruptible	2004-08-14 23:41:46.000000000 +0200
+++ linux-2.6.8-max/kernel/timer.c	2004-08-14 23:41:46.000000000 +0200
@@ -1503,3 +1503,18 @@ void msleep(unsigned int msecs)
 
 EXPORT_SYMBOL(msleep);
 
+/**
+ * msleep_interruptible - sleep waiting for waitqueue interruptions
+ * @msecs: Time in milliseconds to sleep for
+ */
+void msleep_interruptible(unsigned int msecs)
+{
+	unsigned long timeout = msecs_to_jiffies(msecs);
+
+	while (timeout) {
+		set_current_state(TASK_INTERRUPTIBLE);
+		timeout = schedule_timeout(timeout);
+	}
+}
+
+EXPORT_SYMBOL(msleep_interruptible);

_

Re: Add msleep_interruptible() function to kernel/timer.c

[Oliver Neukum]

Am Sonntag, 15. August 2004 14:18 schrieb maximilian attems:
> while doing kernel-janitor msleep conversion of drivers own (incorrect),
> functions, some places would need msleep_interruptible().
> 
> this function is equivalent to:
>         current->state = TASK_INTERRUPTIBLE;
> 	schedule_timeout(timeout);
> 
> idea from Ingo Molnar:
> well, aboves is not 100% equivalent because msleep() is uninterruptible so
> stoppage of the md thread (upon shutdown) will occur with only a 250 msec
> delay. Someone should add a msleep_interruptible() function to kernel/timer.c.

[..] 
> +/**
> + * msleep_interruptible - sleep waiting for waitqueue interruptions
> + * @msecs: Time in milliseconds to sleep for
> + */
> +void msleep_interruptible(unsigned int msecs)
> +{
> +	unsigned long timeout = msecs_to_jiffies(msecs);
> +
> +	while (timeout) {
> +		set_current_state(TASK_INTERRUPTIBLE);
> +		timeout = schedule_timeout(timeout);
> +	}

This is not really interruptible sleep because it is missing a check
for pending signals.

	Regards
		Oliver

Re: Add msleep_interruptible() function to kernel/timer.c

[maximilian attems]

On Sun, 15 Aug 2004, Alan Cox wrote:

> On Sul, 2004-08-15 at 13:18, maximilian attems wrote:
> > + * msleep_interruptible - sleep waiting for waitqueue interruptions
> > + * @msecs: Time in milliseconds to sleep for
> > + */
> > +void msleep_interruptible(unsigned int msecs)
> > +{
> > +	unsigned long timeout = msecs_to_jiffies(msecs);
> > +
> > +	while (timeout) {
> 
> You want to have while(timeout && !signal_pending(current))
> 
> A signal will wake the timeout which will then loop. It might also
> be good to add
> 
> > +		set_current_state(TASK_INTERRUPTIBLE);
> > +		timeout = schedule_timeout(timeout);
> > +	}
> 
> return timeout;
> 
> so that the caller knows more about how long the timer ran for before
> the interrupt and if it was interrupted.

belows patches returns timeout in msecs 
as the function is also called with that unit, 
added definition in include/linux/delay.h



this function is equivalent to:
	current->state = TASK_INTERRUPTIBLE;
	schedule_timeout(timeout);

idea from Ingo Molnar:
well, aboves is not 100% equivalent because msleep() is uninterruptible so
stoppage of the md thread (upon shutdown) will occur with only a 250 msec
delay. Someone should add a msleep_interruptible() function to kernel/timer.c.

Signed-off-by: Maximilian Attems <janitor@sternwelten.at>



---

 linux-2.6.8-max/kernel/timer.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+)


this function is equivalent to:
	current->state = TASK_INTERRUPTIBLE;
	schedule_timeout(timeout);

idea from Ingo Molnar:
well, aboves is not 100% equivalent because msleep() is uninterruptible so
stoppage of the md thread (upon shutdown) will occur with only a 250 msec
delay. Someone should add a msleep_interruptible() function to kernel/timer.c.

Signed-off-by: Maximilian Attems <janitor@sternwelten.at>



---

 linux-2.6.8-max/include/linux/delay.h |    1 +
 linux-2.6.8-max/kernel/timer.c        |   16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff -puN kernel/timer.c~add-msleep_interruptible kernel/timer.c
--- linux-2.6.8/kernel/timer.c~add-msleep_interruptible	2004-08-15 15:15:01.000000000 +0200
+++ linux-2.6.8-max/kernel/timer.c	2004-08-15 15:18:56.000000000 +0200
@@ -1503,3 +1503,19 @@ void msleep(unsigned int msecs)
 
 EXPORT_SYMBOL(msleep);
 
+/**
+ * msleep_interruptible - sleep waiting for waitqueue interruptions
+ * @msecs: Time in milliseconds to sleep for
+ */
+unsigned int 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);
+}
+
+EXPORT_SYMBOL(msleep_interruptible);
diff -puN include/linux/delay.h~add-msleep_interruptible include/linux/delay.h
--- linux-2.6.8/include/linux/delay.h~add-msleep_interruptible	2004-08-15 15:15:01.000000000 +0200
+++ linux-2.6.8-max/include/linux/delay.h	2004-08-15 15:17:56.000000000 +0200
@@ -39,5 +39,6 @@ extern unsigned long loops_per_jiffy;
 #endif
 
 void msleep(unsigned int msecs);
+unsigned int msleep_interruptible(unsigned int msecs);
 
 #endif /* defined(_LINUX_DELAY_H) */

_

Re: Add msleep_interruptible() function to kernel/timer.c

[Ingo Molnar]

* maximilian attems <janitor@sternwelten.at> wrote:

> +/**
> + * msleep_interruptible - sleep waiting for waitqueue interruptions
> + * @msecs: Time in milliseconds to sleep for
> + */
> +unsigned int 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);
> +}

looks good to me.

Signed-off-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: [Kernel-janitors] Re: Add msleep_interruptible() function to kernel/timer.c

[Nishanth Aravamudan]

On Sun, Aug 15, 2004 at 03:25:48PM +0200, maximilian attems wrote:
> On Sun, 15 Aug 2004, Alan Cox wrote:
> 
> > On Sul, 2004-08-15 at 13:18, maximilian attems wrote:
> > > + * msleep_interruptible - sleep waiting for waitqueue interruptions
> > > + * @msecs: Time in milliseconds to sleep for
> > > + */
> > > +void msleep_interruptible(unsigned int msecs)
> > > +{
> > > +	unsigned long timeout = msecs_to_jiffies(msecs);
> > > +
> > > +	while (timeout) {
> > 
> > You want to have while(timeout && !signal_pending(current))
> > 
> > A signal will wake the timeout which will then loop. It might also
> > be good to add
> > 
> > > +		set_current_state(TASK_INTERRUPTIBLE);
> > > +		timeout = schedule_timeout(timeout);
> > > +	}
> > 
> > return timeout;
> > 
> > so that the caller knows more about how long the timer ran for before
> > the interrupt and if it was interrupted.
> 
> belows patches returns timeout in msecs 
> as the function is also called with that unit, 
> added definition in include/linux/delay.h

<snip>

An entry could be added to the TODO to do what I had done but with
long delays in TASK_INTERRUPTIBLE (which there probably should not be
many of). It would also probably be useful to verify that any drivers 
sleeping for times measurable in msecs while INTERRUPTIBLE actually intend
to do so (as I tried to do in some cases).

-Nish