[KJ] [PATCH] input/iforce-packets: insert set_current_state()

[KJ] [PATCH] input/iforce-packets: insert set_current_state()

[Nishanth Aravamudan]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

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 <nacc@us.ibm.com>

--- 2.6.9-bk7-vanilla/drivers/input/joystick/iforce/iforce-packets.c	2004-10-22 10:41:36.000000000 -0700
+++ 2.6.9-bk7/drivers/input/joystick/iforce/iforce-packets.c	2004-10-22 14:53:18.000000000 -0700
@@ -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);

[-- 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

Re: [KJ] [PATCH] input/iforce-packets: insert set_current_state()

[Jon Masters]

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

On Fri, 22 Oct 2004 16:28:38 -0700, Nishanth Aravamudan <nacc@us.ibm.com> wrote:

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

> @@ -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);

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

Jon.

[-- 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

Re: [KJ] [PATCH] input/iforce-packets: insert set_current_state()

[Nishanth Aravamudan]

[-- Attachment #1: Type: text/plain, Size: 1226 bytes --]

On Sat, Oct 23, 2004 at 12:33:14AM +0100, Jon Masters wrote:
> On Fri, 22 Oct 2004 16:28:38 -0700, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> 
> > Description: Inserts set_current_state() before schedule_timeout().
> > Without the insertion, schedule_timeout() will return immediately.
> 
> > @@ -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);
> 
> 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

[-- 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

Re: [KJ] [PATCH] input/iforce-packets: insert set_current_state()

[Jon Masters]

[-- Attachment #1: Type: text/plain, Size: 225 bytes --]

On Fri, 22 Oct 2004 16:41:39 -0700, Nishanth Aravamudan <nacc@us.ibm.com> wrote:

> So, if those while-loops iterate more than once, schedule_timeout() will
> return and run again.

Yes, ignore that. Wrong time of day.

Jon.

[-- 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

Re: [KJ] [PATCH] input/iforce-packets: insert set_current_state()

[maximilian attems]

[-- Attachment #1: Type: text/plain, Size: 399 bytes --]

On Fri, 22 Oct 2004, Nishanth Aravamudan wrote:

> 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 <nacc@us.ibm.com>

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

thanks maks


[-- 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