On Sat, Oct 23, 2004 at 02:20:00AM +0200, Oliver Neukum wrote:
> Am Samstag, 23. Oktober 2004 00:56 schrieb Nishanth Aravamudan:
> > + while (timeout) {
> > if (signal_pending(current)) {
> > remove_wait_queue(&usblp->wait, &wait);
> > return writecount ? writecount : -EINTR;
> > }
> > - set_current_state(TASK_INTERRUPTIBLE);
> > - if (timeout && !usblp->wcomplete) {
> > - timeout = schedule_timeout(timeout);
> > - } else {
> > - set_current_state(TASK_RUNNING);
> > + if (usblp->wcomplete) {
> > break;
> > }
> > + set_current_state(TASK_INTERRUPTIBLE);
>
> You can miss a wake up here.
>
> > + timeout = schedule_timeout(timeout);
> > }
>
> The order of checking wcomplete and setting the state must be reversed.
OK, thanks for the pointer. Please find the corrected patch below.
-Nish
Description: The while-loop seemed excessively blocked with
conditionals. By reorganizing the code so timeout is the condition for
the loop and changing the checks within the loop, several lines of code
were removed.
Signed-off-by: Nishanth Aravamudan <[EMAIL PROTECTED]>
--- 2.6.10-rc1-vanilla/drivers/usb/class/usblp.c 2004-10-30 15:33:37.000000000
-0700
+++ 2.6.10-rc1/drivers/usb/class/usblp.c 2004-11-02 15:01:28.000000000 -0800
@@ -636,19 +636,16 @@ static ssize_t usblp_write(struct file *
timeout = USBLP_WRITE_TIMEOUT;
add_wait_queue(&usblp->wait, &wait);
- while ( 1==1 ) {
-
+ while (timeout) {
if (signal_pending(current)) {
remove_wait_queue(&usblp->wait, &wait);
return writecount ? writecount : -EINTR;
}
set_current_state(TASK_INTERRUPTIBLE);
- if (timeout && !usblp->wcomplete) {
- timeout = schedule_timeout(timeout);
- } else {
- set_current_state(TASK_RUNNING);
+ if (usblp->wcomplete) {
break;
}
+ timeout = schedule_timeout(timeout);
}
remove_wait_queue(&usblp->wait, &wait);
}
