usb/auerswald: reorder set_current_state()

Quoted 3 lines · Nishanth Aravamudan

+++ 2.6.9-rc2/drivers/usb/misc/auerswald.c 2004-09-27 14:42:24.000000000 -0700

(quoted diff) +0 -0

@@ -626,8 +626,8 @@ static int auerchain_start_wait_urb (pau
 

Review static int auerchain_start_wait_urb (pau · line 633

Quoted 8 lines

(quoted diff) +1 -1

 	while (timeout && !chs.done)
 	{
-		timeout = schedule_timeout (timeout);
 		set_current_state(TASK_UNINTERRUPTIBLE);
+		timeout = schedule_timeout (timeout);
 		rmb();
 	}
 

Now "set_current_state (TASK_RUNNING);" a line under this isn't needed anymore, right?

Quoted 4 lines · Domen Puncer

Quoted 3 lines · Nishanth Aravamudan

+++ 2.6.9-rc2/drivers/usb/misc/auerswald.c 2004-09-27 14:42:24.000000000 -0700

(quoted diff) +0 -0

@@ -626,8 +626,8 @@ static int auerchain_start_wait_urb (pau
 

Quoted 10 lines

Review static int auerchain_start_wait_urb (pau · line 633

Quoted 8 lines

(quoted diff) +1 -1

 	while (timeout && !chs.done)
 	{
-		timeout = schedule_timeout (timeout);
 		set_current_state(TASK_UNINTERRUPTIBLE);
+		timeout = schedule_timeout (timeout);
 		rmb();
 	}
 

Now "set_current_state (TASK_RUNNING);" a line under this isn't needed anymore, right?

I think it is still needed, because the state is initially set just before add_wait_queue() [before the if] to TASK_UNINTERRUPTIBLE. So, in the off chance that the while-loop's condition is false on the first iteration, the state needs to be set back to TASK_RUNNING, since schedule_timeout() won't be run at all.

Quoted 16 lines · Nishanth Aravamudan

Quoted 4 lines · Domen Puncer

Quoted 3 lines · Nishanth Aravamudan

+++ 2.6.9-rc2/drivers/usb/misc/auerswald.c 2004-09-27 14:42:24.000000000 -0700

(quoted diff) +0 -0

@@ -626,8 +626,8 @@ static int auerchain_start_wait_urb (pau
 

Quoted 10 lines

Review static int auerchain_start_wait_urb (pau · line 633

Quoted 8 lines

(quoted diff) +1 -1

 	while (timeout && !chs.done)
 	{
-		timeout = schedule_timeout (timeout);
 		set_current_state(TASK_UNINTERRUPTIBLE);
+		timeout = schedule_timeout (timeout);
 		rmb();
 	}
 

Now "set_current_state (TASK_RUNNING);" a line under this isn't needed anymore, right?

Quoted 5 lines

I think it is still needed, because the state is initially set just before add_wait_queue() [before the if] to TASK_UNINTERRUPTIBLE. So, in the off chance that the while-loop's condition is false on the first iteration, the state needs to be set back to TASK_RUNNING, since schedule_timeout() won't be run at all.

You're right, missed that.
So... in the first iteration, schedule won't return instantly => patch isn't needed?

Quoted 23 lines · Domen Puncer

Quoted 16 lines · Nishanth Aravamudan

Quoted 4 lines · Domen Puncer

Quoted 3 lines · Nishanth Aravamudan

+++ 2.6.9-rc2/drivers/usb/misc/auerswald.c 2004-09-27 14:42:24.000000000 -0700

(quoted diff) +0 -0

@@ -626,8 +626,8 @@ static int auerchain_start_wait_urb (pau
 

Quoted 10 lines

Review static int auerchain_start_wait_urb (pau · line 633

Quoted 8 lines

(quoted diff) +1 -1

 	while (timeout && !chs.done)
 	{
-		timeout = schedule_timeout (timeout);
 		set_current_state(TASK_UNINTERRUPTIBLE);
+		timeout = schedule_timeout (timeout);
 		rmb();
 	}
 

Now "set_current_state (TASK_RUNNING);" a line under this isn't needed anymore, right?

Quoted 5 lines

I think it is still needed, because the state is initially set just before add_wait_queue() [before the if] to TASK_UNINTERRUPTIBLE. So, in the off chance that the while-loop's condition is false on the first iteration, the state needs to be set back to TASK_RUNNING, since schedule_timeout() won't be run at all.

Quoted 3 lines

You're right, missed that.
So... in the first iteration, schedule won't return instantly => patch isn't needed?

You're right, actually. The one thing I'm not sure about is if the state is meant to be set before the add_wait_queue(). I'm not sure if this driver needs to be in TASK_UNINTERRUPTIBLE for the assignment and auerchain_submit_urb(). If anyone has any input on that, it would be great.