* Re: [Kernel-janitors] RE: [PATCH] cciss: replace schedule_timeout()
2004-07-23 22:45 [Kernel-janitors] RE: [PATCH] cciss: replace schedule_timeout() Miller, Mike (OS Dev)
@ 2004-07-23 23:26 ` Nish Aravamudan
2004-07-28 22:51 ` Miller, Mike (OS Dev)
1 sibling, 0 replies; 3+ messages in thread
From: Nish Aravamudan @ 2004-07-23 23:26 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1755 bytes --]
uOn Fri, 23 Jul 2004 17:45:55 -0500, Miller, Mike (OS Dev)
<mike.miller@hp.com> wrote:
> You'll have to explain why msleep will guarantee the timeout period whereas
> schedule_timeout may not. Maybe I'm missing something.
Well, there are two things to consider here:
As a reference, the way the code was written before:
- set_current_state(TASK_INTERRUPTIBLE);
- schedule_timeout(HZ / 10); /* wait 100ms */
1) Since the state is set to TASK_INTERRUPTIBLE, although a 100ms
delay is requested, if a signal is received, then the delay ends
prematurely and the task will resume after the schedule_timeout()
call. Now, if this was the desired effect, then the the patch should
not be applied.
2) However, if a true delay was desired, then TASK_INTERRUPTIBLE
should not have been the set state, but TASK_UNINTERRUPTIBLE should
have been. This would have made the code:
set_current_state(TASK_UNINTERRUPTIBLE);
schedule_timeout(HZ / 10);
msleep() achieves this exact purpose, but also:
a) allows the parameter time to be in msecs, which is far clearer IMO; and
b) guarantees that the task will not continue execution until after
the timeout has expired. Confusingly (at least to me), there are
certain conditions -- the details of which I will have to defer to
Greg Kroah-Hartman -- in which TASK_UNINTERRUPTIBLE'd
schedule_timeout()s may still return before the timeout. The desired
delay is achieved by wrapping the schedule_timeout(timeout) with a
while(timeout) loop. Thus, as long as timeout is non-zero, the task is
deferred. (see msleep()s definition for more details).
Does that help some? Please mail again if you have more questions, or
if something I wrote was unclear.
-Nish
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 3+ messages in thread* RE: [Kernel-janitors] RE: [PATCH] cciss: replace schedule_timeout()
2004-07-23 22:45 [Kernel-janitors] RE: [PATCH] cciss: replace schedule_timeout() Miller, Mike (OS Dev)
2004-07-23 23:26 ` Nish Aravamudan
@ 2004-07-28 22:51 ` Miller, Mike (OS Dev)
1 sibling, 0 replies; 3+ messages in thread
From: Miller, Mike (OS Dev) @ 2004-07-28 22:51 UTC (permalink / raw)
To: kernel-janitors
Nish,
Sorry I've taken so long to respond. I just figured out yesterday that my Linux mail was not going out :(
I agree about TASK_INTERRUPTIBLE, but in all the 2.6.x code I've looked at we are using TASK_UNINTERRUPTIBLE.
If Greg Kroah-Hartman would care to elaborate on the problems using that I am open to new ideas & suggestions.
Thanks,
mikem
-----Original Message-----
From: Nish Aravamudan [mailto:nish.aravamudan@gmail.com]
Sent: Friday, July 23, 2004 6:27 PM
To: Miller, Mike (OS Dev)
Cc: kernel-janitors@lists.osdl.org
Subject: Re: [Kernel-janitors] RE: [PATCH] cciss: replace
schedule_timeout() with msleep()
uOn Fri, 23 Jul 2004 17:45:55 -0500, Miller, Mike (OS Dev)
<mike.miller@hp.com> wrote:
> You'll have to explain why msleep will guarantee the timeout period whereas
> schedule_timeout may not. Maybe I'm missing something.
Well, there are two things to consider here:
As a reference, the way the code was written before:
- set_current_state(TASK_INTERRUPTIBLE);
- schedule_timeout(HZ / 10); /* wait 100ms */
1) Since the state is set to TASK_INTERRUPTIBLE, although a 100ms
delay is requested, if a signal is received, then the delay ends
prematurely and the task will resume after the schedule_timeout()
call. Now, if this was the desired effect, then the the patch should
not be applied.
2) However, if a true delay was desired, then TASK_INTERRUPTIBLE
should not have been the set state, but TASK_UNINTERRUPTIBLE should
have been. This would have made the code:
set_current_state(TASK_UNINTERRUPTIBLE);
schedule_timeout(HZ / 10);
msleep() achieves this exact purpose, but also:
a) allows the parameter time to be in msecs, which is far clearer IMO; and
b) guarantees that the task will not continue execution until after
the timeout has expired. Confusingly (at least to me), there are
certain conditions -- the details of which I will have to defer to
Greg Kroah-Hartman -- in which TASK_UNINTERRUPTIBLE'd
schedule_timeout()s may still return before the timeout. The desired
delay is achieved by wrapping the schedule_timeout(timeout) with a
while(timeout) loop. Thus, as long as timeout is non-zero, the task is
deferred. (see msleep()s definition for more details).
Does that help some? Please mail again if you have more questions, or
if something I wrote was unclear.
-Nish
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 3+ messages in thread