Skip to contentSkip to conversation

input/iforce-packets: insert set_current_state()

PATCH Applied Nishanth Aravamudan 3 participants 5 messages Oct 22, 2004 – Oct 24, 2004

Subsystems (2)
Kernel janitors Input drivers
lore
Share & tools
lore

Nishanth Aravamudan

author
#0 lore raw

PATCH 1/1 input/iforce-packets: insert set_current_state()

Any comments would be, as always, appreciated.

-Nish

Description: Inserts set_current_state() before schedule_timeout(). Without the insertion, schedule_timeout() will return immediately.

Signed-off-by Nishanth Aravamudan <[email protected]>
drivers/input/joystick/iforce/iforce-packets.c +6 -2
@@ -269,8 +269,10 @@ int iforce_get_id_packet(struct iforce *
 			return -1;
 		}
 
-		while (timeout && iforce->ctrl->status == -EINPROGRESS)
+		while (timeout && iforce->ctrl->status == -EINPROGRESS) {
+			set_current_state(TASK_INTERRUPTIBLE);
 			timeout = schedule_timeout(timeout);
+		}
 
 		set_current_state(TASK_RUNNING);
 		remove_wait_queue(&iforce->wait, &wait);

@@ -293,8 +295,10 @@ int iforce_get_id_packet(struct iforce *
 		set_current_state(TASK_INTERRUPTIBLE);
 		add_wait_queue(&iforce->wait, &wait);
 
-		while (timeout && iforce->expect_packet)
+		while (timeout && iforce->expect_packet) {
+			set_current_state(TASK_INTERRUPTIBLE);
 			timeout = schedule_timeout(timeout);
+		}
 
 		set_current_state(TASK_RUNNING);
 		remove_wait_queue(&iforce->wait, &wait);

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#1 lore raw
Quoted 5 lines · Nishanth Aravamudan

Description: Inserts set_current_state() before schedule_timeout(). Without the insertion, schedule_timeout() will return immediately.

drivers/input/joystick/iforce/iforce-packets.c +0 -0
@@ -293,8 +295,10 @@ int iforce_get_id_packet(struct iforce *
                set_current_state(TASK_INTERRUPTIBLE);

drivers/input/joystick/iforce/iforce-packets.c int iforce_get_id_packet(struct iforce * · line 304

Quoted 10 lines
drivers/input/joystick/iforce/iforce-packets.c +3 -1
                add_wait_queue(&iforce->wait, &wait);
 
-               while (timeout && iforce->expect_packet)
+               while (timeout && iforce->expect_packet) {
+                       set_current_state(TASK_INTERRUPTIBLE);
                        timeout = schedule_timeout(timeout);
+               }
 
                set_current_state(TASK_RUNNING);
                remove_wait_queue(&iforce->wait, &wait);

What did I miss there? Why's this second one necessary?

Jon.

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

author
#2 lore raw
In reply to Nishanth Aravamudan
Quoted 15 lines · Jon Masters
Quoted 13 lines · Nishanth Aravamudan

Description: Inserts set_current_state() before schedule_timeout(). Without the insertion, schedule_timeout() will return immediately.

drivers/input/joystick/iforce/iforce-packets.c +3 -1
@@ -293,8 +295,10 @@ int iforce_get_id_packet(struct iforce *
                set_current_state(TASK_INTERRUPTIBLE);
                add_wait_queue(&iforce->wait, &wait);
 
-               while (timeout && iforce->expect_packet)
+               while (timeout && iforce->expect_packet) {
+                       set_current_state(TASK_INTERRUPTIBLE);
                        timeout = schedule_timeout(timeout);
+               }
Quoted 4 lines

Review int iforce_get_id_packet(struct iforce * · line 304

Quoted 2 lines
(quoted diff) +0 -0
                set_current_state(TASK_RUNNING);
                remove_wait_queue(&iforce->wait, &wait);

What did I miss there? Why's this second one necessary?

So, if those while-loops iterate more than once, schedule_timeout() will return and run again. The way schedule_timeout() is designed, though, it will always return in TASK_RUNNING. Thus, the next call to schedule_timeout() returns immediately and we have a busy-loop. Not good!

-Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Applied
#4 lore raw
In reply to Nishanth Aravamudan
Quoted 8 lines · Nishanth Aravamudan

Any comments would be, as always, appreciated.

-Nish

Description: Inserts set_current_state() before schedule_timeout(). Without the insertion, schedule_timeout() will return immediately.

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

applied to kjt,
to allow further testing and to be shure it gets upstream.

thanks maks

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors