RE: [KJ] RE: [PATCH 14/21] net/s2io: replace schedule_timeout() with

["Ravinandan Arakali" <ravinandan.arakali@neterion.com>]

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

Domen,
Thanks for pointing that out. As you mention, the intention is
indeed to wait for Ctrl+C.
Nishanth, are you planning to modify it to msleep_interruptible().
If so, pls note that I submitted a Multicast fix after your below
patch. Pls apply multicast patch before changing to msleep_interruptible().

Ravi

-----Original Message-----
From: Domen Puncer [mailto:domen@coderock.org]
Sent: Wednesday, February 16, 2005 5:26 AM
To: Ravinandan Arakali
Cc: 'Raghavendra Koushik'; 'Nishanth Aravamudan';
kernel-janitors@lists.osdl.org; leonid.grossman@neterion.com
Subject: Re: [KJ] RE: [PATCH 14/21] net/s2io: replace schedule_timeout()
with msleep()


On 09/02/05 10:37 -0800, Ravinandan Arakali wrote:
> Nishanth,
> We did some basic testing after applying your patch and it seems okay.
...
>
> In the function s2io_ethtool_idnic(), call to schedule_timeout() has not
> been replaced with msleep(). I've given below the code change I did before
> testing.
>
> @@ -3284,11 +3277,10 @@
>                 sp->id_timer.data = (unsigned long) sp;
>         }
>         mod_timer(&sp->id_timer, jiffies);
> -       set_current_state(TASK_INTERRUPTIBLE);
>         if (data)
> -               schedule_timeout(data * HZ);
> +               msleep(data*1000);
>         else
> -               schedule_timeout(MAX_SCHEDULE_TIMEOUT);
> +               msleep(0xFFFFFFFF);

Was this code path tested too? It looks weird, as msleep _will_ sleep
for almost a day.
I don't know ethtool, but might this 'idnic' with undefined sleep time
just wait for ctrl+c for example? If that is so, msleep_interruptible
is what you want.

>         del_timer_sync(&sp->id_timer);
>
>         if (CARDS_WITH_FAULTY_LINK_INDICATORS(subid)) {
>


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