* [Kernel-janitors] Re: usb/auerswald: reorder set_current_state()
@ 2004-09-29 16:30 Domen Puncer
2004-09-29 17:22 ` Nishanth Aravamudan
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Domen Puncer @ 2004-09-29 16:30 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 487 bytes --]
On 27/09/04 14:46 -0700, Nishanth Aravamudan wrote:
> +++ 2.6.9-rc2/drivers/usb/misc/auerswald.c 2004-09-27 14:42:24.000000000 -0700
> @@ -626,8 +626,8 @@ static int auerchain_start_wait_urb (pau
>
> 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?
Domen
[-- 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] 5+ messages in thread
* [Kernel-janitors] Re: usb/auerswald: reorder set_current_state()
2004-09-29 16:30 [Kernel-janitors] Re: usb/auerswald: reorder set_current_state() Domen Puncer
@ 2004-09-29 17:22 ` Nishanth Aravamudan
2004-09-29 17:42 ` Domen Puncer
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Nishanth Aravamudan @ 2004-09-29 17:22 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 890 bytes --]
On Wed, Sep 29, 2004 at 06:30:14PM +0200, Domen Puncer wrote:
> On 27/09/04 14:46 -0700, Nishanth Aravamudan wrote:
> > +++ 2.6.9-rc2/drivers/usb/misc/auerswald.c 2004-09-27 14:42:24.000000000 -0700
> > @@ -626,8 +626,8 @@ static int auerchain_start_wait_urb (pau
> >
> > 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.
-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] 5+ messages in thread
* [Kernel-janitors] Re: usb/auerswald: reorder set_current_state()
2004-09-29 16:30 [Kernel-janitors] Re: usb/auerswald: reorder set_current_state() Domen Puncer
2004-09-29 17:22 ` Nishanth Aravamudan
@ 2004-09-29 17:42 ` Domen Puncer
2004-09-29 18:00 ` Nishanth Aravamudan
2004-09-29 22:44 ` Nishanth Aravamudan
3 siblings, 0 replies; 5+ messages in thread
From: Domen Puncer @ 2004-09-29 17:42 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]
On 29/09/04 10:22 -0700, Nishanth Aravamudan wrote:
> On Wed, Sep 29, 2004 at 06:30:14PM +0200, Domen Puncer wrote:
> > On 27/09/04 14:46 -0700, Nishanth Aravamudan wrote:
> > > +++ 2.6.9-rc2/drivers/usb/misc/auerswald.c 2004-09-27 14:42:24.000000000 -0700
> > > @@ -626,8 +626,8 @@ static int auerchain_start_wait_urb (pau
> > >
> > > 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.
You're right, missed that.
So... in the first iteration, schedule won't return instantly =>
patch isn't needed?
Domen
[-- 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] 5+ messages in thread
* [Kernel-janitors] Re: usb/auerswald: reorder set_current_state()
2004-09-29 16:30 [Kernel-janitors] Re: usb/auerswald: reorder set_current_state() Domen Puncer
2004-09-29 17:22 ` Nishanth Aravamudan
2004-09-29 17:42 ` Domen Puncer
@ 2004-09-29 18:00 ` Nishanth Aravamudan
2004-09-29 22:44 ` Nishanth Aravamudan
3 siblings, 0 replies; 5+ messages in thread
From: Nishanth Aravamudan @ 2004-09-29 18:00 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]
On Wed, Sep 29, 2004 at 07:42:35PM +0200, Domen Puncer wrote:
> On 29/09/04 10:22 -0700, Nishanth Aravamudan wrote:
> > On Wed, Sep 29, 2004 at 06:30:14PM +0200, Domen Puncer wrote:
> > > On 27/09/04 14:46 -0700, Nishanth Aravamudan wrote:
> > > > +++ 2.6.9-rc2/drivers/usb/misc/auerswald.c 2004-09-27 14:42:24.000000000 -0700
> > > > @@ -626,8 +626,8 @@ static int auerchain_start_wait_urb (pau
> > > >
> > > > 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.
>
> 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.
-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] 5+ messages in thread
* Re: [Kernel-janitors] Re: usb/auerswald: reorder set_current_state()
2004-09-29 16:30 [Kernel-janitors] Re: usb/auerswald: reorder set_current_state() Domen Puncer
` (2 preceding siblings ...)
2004-09-29 18:00 ` Nishanth Aravamudan
@ 2004-09-29 22:44 ` Nishanth Aravamudan
3 siblings, 0 replies; 5+ messages in thread
From: Nishanth Aravamudan @ 2004-09-29 22:44 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]
On Wed, Sep 29, 2004 at 11:00:07AM -0700, Nishanth Aravamudan wrote:
> On Wed, Sep 29, 2004 at 07:42:35PM +0200, Domen Puncer wrote:
> > On 29/09/04 10:22 -0700, Nishanth Aravamudan wrote:
> > > On Wed, Sep 29, 2004 at 06:30:14PM +0200, Domen Puncer wrote:
> > > > On 27/09/04 14:46 -0700, Nishanth Aravamudan wrote:
> > > > > +++ 2.6.9-rc2/drivers/usb/misc/auerswald.c 2004-09-27 14:42:24.000000000 -0700
> > > > > @@ -626,8 +626,8 @@ static int auerchain_start_wait_urb (pau
> > > > >
> > > > > 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.
> >
> > 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.
Didn't say this explicitly, I realized: For now, don't apply this patch.
once I have some more details on the necessity of auerchain_submit_urb()
running in TASK_UNINTERRUPTIBLE, I will resubmit the patch (if
necessary, of course).
-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] 5+ messages in thread
end of thread, other threads:[~2004-09-29 22:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-09-29 16:30 [Kernel-janitors] Re: usb/auerswald: reorder set_current_state() Domen Puncer
2004-09-29 17:22 ` Nishanth Aravamudan
2004-09-29 17:42 ` Domen Puncer
2004-09-29 18:00 ` Nishanth Aravamudan
2004-09-29 22:44 ` Nishanth Aravamudan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.