netdev
[Top] [All Lists]

Re: [PATCH 1/7] netpoll: shorten carrier detect timeout

To: Matt Mackall <mpm@xxxxxxxxxxx>
Subject: Re: [PATCH 1/7] netpoll: shorten carrier detect timeout
From: Patrick McHardy <kaber@xxxxxxxxx>
Date: Sun, 06 Mar 2005 02:01:01 +0100
Cc: Jeff Garzik <jgarzik@xxxxxxxxx>, netdev@xxxxxxxxxxx, Jeff Moyer <jmoyer@xxxxxxxxxx>
In-reply-to: <20050306002015.GD3120@waste.org>
References: <2.454130102@selenic.com> <422A4A38.4040303@trash.net> <20050306002015.GD3120@waste.org>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.7.5) Gecko/20050106 Debian/1.7.5-1
Matt Mackall wrote:
On Sun, Mar 06, 2005 at 01:09:28AM +0100, Patrick McHardy wrote:

The carrier detection looks partially broken to me. The current logic detects an instantly available carrier as flaky because netif_carrier_ok() takes less than 1/10s. This patch does what I assume is intended, make sure the carrier is stable for 1/10s.


Looks ok, but I've been meaning to change the second loop to something
like msleep().

How about this one ? I also removed oflags, reading it outside of the locked section was racy.

Did you try this with a card that otherwise goes into the wait?

Yes, I tried with sk98lin. I don't have a card here that actually supports netpoll.

Signed-off-by: Patrick McHardy <kaber@xxxxxxxxx>
===== net/core/netpoll.c 1.27 vs edited =====
--- 1.27/net/core/netpoll.c     2005-01-26 06:32:56 +01:00
+++ edited/net/core/netpoll.c   2005-03-06 01:58:48 +01:00
@@ -18,6 +18,7 @@
 #include <linux/interrupt.h>
 #include <linux/netpoll.h>
 #include <linux/sched.h>
+#include <linux/delay.h>
 #include <linux/rcupdate.h>
 #include <net/tcp.h>
 #include <net/udp.h>
@@ -575,16 +576,14 @@
        }
 
        if (!netif_running(ndev)) {
-               unsigned short oflags;
                unsigned long atmost, atleast;
+               long left;
 
                printk(KERN_INFO "%s: device %s not up yet, forcing it\n",
                       np->name, np->dev_name);
 
-               oflags = ndev->flags;
-
                rtnl_shlock();
-               if (dev_change_flags(ndev, oflags | IFF_UP) < 0) {
+               if (dev_change_flags(ndev, ndev->flags | IFF_UP) < 0) {
                        printk(KERN_ERR "%s: failed to open %s\n",
                               np->name, np->dev_name);
                        rtnl_shunlock();
@@ -592,8 +591,7 @@
                }
                rtnl_shunlock();
 
-               atleast = jiffies + HZ/10;
-               atmost = jiffies + 10*HZ;
+               atmost = jiffies + 4*HZ;
                while (!netif_carrier_ok(ndev)) {
                        if (time_after(jiffies, atmost)) {
                                printk(KERN_NOTICE
@@ -604,12 +602,16 @@
                        cond_resched();
                }
 
+               atleast = jiffies + HZ/10;
+               while (netif_carrier_ok(ndev) && time_before(jiffies, atleast))
+                       cond_resched();
                if (time_before(jiffies, atleast)) {
                        printk(KERN_NOTICE "%s: carrier detect appears flaky,"
-                              " waiting 10 seconds\n",
+                              " waiting 4 seconds\n",
                               np->name);
-                       while (time_before(jiffies, atmost))
-                               cond_resched();
+                       left = atmost - jiffies;
+                       if (left > 0)
+                               msleep(jiffies_to_msecs(left));
                }
        }
 
<Prev in Thread] Current Thread [Next in Thread>