[KJ] [PATCH] net/ewrk3: replace schedule_timeout() with

[KJ] [PATCH] net/ewrk3: replace schedule_timeout() with

[Nishanth Aravamudan]

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

Any comments would be, as always, appreciated.

-Nish

Description: Uses msleep() instead of schedule_timeout() to guarantee
the task delays as expected.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>

--- 2.6.9-bk7-vanilla/drivers/net/ewrk3.c	2004-10-22 10:40:51.000000000 -0700
+++ 2.6.9-bk7/drivers/net/ewrk3.c	2004-10-22 14:56:19.000000000 -0700
@@ -1655,8 +1655,7 @@ static int ewrk3_phys_id(struct net_devi
 
 		/* Wait a little while */
 		spin_unlock_irqrestore(&lp->hw_lock, flags);
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(HZ>>2);
+		msleep(250);
 		spin_lock_irqsave(&lp->hw_lock, flags);
 
 		/* Exit if we got a signal */

[-- 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] net/ewrk3: replace schedule_timeout() with

[Jon Masters]

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

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

> Any comments would be, as always, appreciated.

>                 /* Wait a little while */
>                 spin_unlock_irqrestore(&lp->hw_lock, flags);
> -               set_current_state(TASK_UNINTERRUPTIBLE);
> -               schedule_timeout(HZ>>2);
> +               msleep(250);
>                 spin_lock_irqsave(&lp->hw_lock, flags);

That delay seems a little long.

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] net/ewrk3: replace schedule_timeout() with

[Nishanth Aravamudan]

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

On Sat, Oct 23, 2004 at 12:30:03AM +0100, Jon Masters wrote:
> On Fri, 22 Oct 2004 16:26:43 -0700, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> 
> > Any comments would be, as always, appreciated.
> 
> >                 /* Wait a little while */
> >                 spin_unlock_irqrestore(&lp->hw_lock, flags);
> > -               set_current_state(TASK_UNINTERRUPTIBLE);
> > -               schedule_timeout(HZ>>2);
> > +               msleep(250);
> >                 spin_lock_irqsave(&lp->hw_lock, flags);
> 
> That delay seems a little long.

Do you mean in the original driver or in my conversion?

-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] net/ewrk3: replace schedule_timeout() with

[Jon Masters]

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

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

> Do you mean in the original driver or in my conversion?

Your conversion. Seems an order of magnitude larger than it needs to be.

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] net/ewrk3: replace schedule_timeout() with

[Nishanth Aravamudan]

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

On Sat, Oct 23, 2004 at 12:34:07AM +0100, Jon Masters wrote:
> On Fri, 22 Oct 2004 16:33:43 -0700, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> 
> > Do you mean in the original driver or in my conversion?
> 
> Your conversion. Seems an order of magnitude larger than it needs to be.

The original was:

schedule_timeout(HZ>>2);

Which is, if my math is right,

HZ (the number of jiffies in a second) shifted 2 bits right, or divided
by 4. In other words, the number of jiffies in a quarter-second.

250 is the number of milliseconds in a quarter-second.

Thus, I think, the values are equivalent. Please correct and slap
accordingly...

-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] net/ewrk3: replace schedule_timeout() with

[Jon Masters]

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

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

> The original was:
> 
> schedule_timeout(HZ>>2);

I just wandered whether that was kicking around and assuming some
smaller value of HZ.

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] net/ewrk3: replace schedule_timeout() with

[maximilian attems]

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

On Fri, 22 Oct 2004, Nishanth Aravamudan wrote:

> Any comments would be, as always, appreciated.
> 
> -Nish
> 
> Description: Uses msleep() instead of schedule_timeout() to guarantee
> the task delays as expected.
> 
applied,
didn't check the 2.4 value of that schedule_timeout(),
but old drives may still expect HZ = 100.

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

Re: [KJ] [PATCH] net/ewrk3: replace schedule_timeout() with

[Nishanth Aravamudan]

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

On Sat, Oct 23, 2004 at 01:33:33AM +0100, Jon Masters wrote:
> On Fri, 22 Oct 2004 16:40:26 -0700, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> 
> > The original was:
> > 
> > schedule_timeout(HZ>>2);
> 
> I just wandered whether that was kicking around and assuming some
> smaller value of HZ.

I don't think it should matter, as the intended delay, on any system,
will be one quarter of a second. Now, whether schedule_timeout() really
achieves this for all arch's is a different question :)

-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