Re: My vote against eepro* removal

[Evgeniy Polyakov]

On Fri, Jan 20, 2006 at 10:37:43AM +0100, kus Kusche Klaus (kus@xxxxxxxx) wrote:
> > From: John Ronciak
> > During the watchdog the e100 driver reads all of the status registers
> > from the actual hardware.  There are 26 (worst case) register reads. 
> > There is also a spin lock for another check in the watchdog.  It would
> > still surprise me that all of this would take 500 usec.  If you are
> > seeing this delay, you can comment out the scheduling of the watchdog
> > to see if this goes away.  We'll need to narrow down exactly what in
> > the watchdog is causing the delay
> 
> Retested it.

...

> Effect of the e100 driver:
> * There is no measurable effect when my test program is running
>   at prio 2 - 99.
> * For prio 1, I get an interval of 500-650 us every 2 seconds,
>   which indicates a scheduling latency of 380-530 us.
> Hence, some piece of code is running for ~500 us at rt prio 1.
> 
> Analysis of e100:
> * If I comment out the whole body of e100_watchdog except for the
>   timer re-registration, the delays are gone (so it is really the
>   body of e100_watchdog). However, this makes eth0 non-functional.
> * Commenting out parts of it, I found out that most of the time
>   goes into its first half: The code from mii_ethtool_gset to
>   mii_check_link (including) makes the big difference, as far as
>   I can tell especially mii_ethtool_gset.

Each MDIO read can take upto 2 msecs (!) and at least 20 usecs in e100,
and this runs in timer handler.
Concider attaching (only compile tested) patch which moves e100 watchdog
into workqueue.

Signed-off-by: Evgeniy Polyakov <johnpol@xxxxxxxxxxx>

diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index 22cd045..e884578 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -549,7 +549,7 @@ struct nic {
 	enum phy phy;
 	struct params params;
 	struct net_device_stats net_stats;
-	struct timer_list watchdog;
+	struct work_struct watchdog;
 	struct timer_list blink_timer;
 	struct mii_if_info mii;
 	struct work_struct tx_timeout_task;
@@ -1495,7 +1495,7 @@ static void e100_adjust_adaptive_ifs(str
 	}
 }
 
-static void e100_watchdog(unsigned long data)
+static void e100_watchdog(void *data)
 {
 	struct nic *nic = (struct nic *)data;
 	struct ethtool_cmd cmd;
@@ -1539,7 +1539,7 @@ static void e100_watchdog(unsigned long 
 	else
 		nic->flags &= ~ich_10h_workaround;
 
-	mod_timer(&nic->watchdog, jiffies + E100_WATCHDOG_PERIOD);
+	schedule_delayed_work(&nic->watchdog, E100_WATCHDOG_PERIOD);
 }
 
 static inline void e100_xmit_prepare(struct nic *nic, struct cb *cb,
@@ -2004,7 +2004,7 @@ static int e100_up(struct nic *nic)
 		goto err_clean_cbs;
 	e100_set_multicast_list(nic->netdev);
 	e100_start_receiver(nic, NULL);
-	mod_timer(&nic->watchdog, jiffies);
+	schedule_work(&nic->watchdog);
 	if((err = request_irq(nic->pdev->irq, e100_intr, SA_SHIRQ,
 		nic->netdev->name, nic->netdev)))
 		goto err_no_irq;
@@ -2016,7 +2016,7 @@ static int e100_up(struct nic *nic)
 	return 0;
 
 err_no_irq:
-	del_timer_sync(&nic->watchdog);
+	cancel_rearming_delayed_work(&nic->watchdog);
 err_clean_cbs:
 	e100_clean_cbs(nic);
 err_rx_clean_list:
@@ -2031,7 +2031,7 @@ static void e100_down(struct nic *nic)
 	netif_stop_queue(nic->netdev);
 	e100_hw_reset(nic);
 	free_irq(nic->pdev->irq, nic->netdev);
-	del_timer_sync(&nic->watchdog);
+	flush_scheduled_work();
 	netif_carrier_off(nic->netdev);
 	e100_clean_cbs(nic);
 	e100_rx_clean_list(nic);
@@ -2570,9 +2570,7 @@ static int __devinit e100_probe(struct p
 
 	pci_set_master(pdev);
 
-	init_timer(&nic->watchdog);
-	nic->watchdog.function = e100_watchdog;
-	nic->watchdog.data = (unsigned long)nic;
+	INIT_WORK(&nic->watchdog, e100_watchdog, nic);
 	init_timer(&nic->blink_timer);
 	nic->blink_timer.function = e100_blink_led;
 	nic->blink_timer.data = (unsigned long)nic;

> -- 
> Klaus Kusche                 (Software Development - Control Systems)
> KEBA AG             Gewerbepark Urfahr, A-4041 Linz, Austria (Europe)
> Tel: +43 / 732 / 7090-3120                 Fax: +43 / 732 / 7090-6301
> E-Mail: kus@xxxxxxxx                                WWW: www.keba.com

-- 
	Evgeniy Polyakov
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/