Skip to contentSkip to conversation

net/ewrk3: replace schedule_timeout() with

PATCH Applied Nishanth Aravamudan 3 participants 8 messages Oct 22, 2004 – Oct 25, 2004

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

Nishanth Aravamudan

author
#0 lore raw

PATCH 1/1 net/ewrk3: replace schedule_timeout() with

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 <[email protected]>
drivers/net/ewrk3.c +1 -2
@@ -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 */

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

#1 lore raw

drivers/net/ewrk3.c static int ewrk3_phys_id(struct net_devi · line 1659

Quoted 8 lines · Nishanth Aravamudan

Any comments would be, as always, appreciated.

                /* Wait a little while */
                spin_unlock_irqrestore(&lp->hw_lock, flags);
drivers/net/ewrk3.c +1 -2
-               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.

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

author
#2 lore raw
In reply to Nishanth Aravamudan
Quoted 4 lines · Jon Masters
Quoted 1 line · Nishanth Aravamudan

Any comments would be, as always, appreciated.

Quoted 8 lines

drivers/net/ewrk3.c static int ewrk3_phys_id(struct net_devi · line 1659

Quoted 6 lines
                /* Wait a little while */
                spin_unlock_irqrestore(&lp->hw_lock, flags);
drivers/net/ewrk3.c +1 -2
-               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

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

author
#4 lore raw
In reply to Nishanth Aravamudan
Quoted 5 lines · Jon Masters
Quoted 1 line · Nishanth Aravamudan

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

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

Applied
#6 lore raw
In reply to Nishanth Aravamudan
Quoted 7 lines · Nishanth Aravamudan

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

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

author
#7 lore raw
In reply to Nishanth Aravamudan
Quoted 8 lines · Jon Masters
Quoted 3 lines · Nishanth Aravamudan

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

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