[net-next PATCH 0/3] qlge: EEH fixes.

[net-next PATCH 0/3] qlge: EEH fixes.

[Ron Mercer]

1) Move asic reset from io_resume() to io_reset.
2) Fix case where close() can hang if eeh did not recover.
3) Fix stall in eeh recovery process.

[net-next PATCH 1/3] qlge: Move reset from eeh io_resume to slot_reset.

[Ron Mercer]

Issue asic reset and verify functionality before continuing to the
resume call.  This allows proper error code to be returned in the case
the asic does not recover.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 5be3ae2..87ec720 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4717,6 +4717,12 @@ static pci_ers_result_t qlge_io_slot_reset(struct pci_dev *pdev)
 		return PCI_ERS_RESULT_DISCONNECT;
 	}
 	pci_set_master(pdev);
+
+	if (ql_adapter_reset(qdev)) {
+		QPRINTK(qdev, DRV, ERR, "reset FAILED!\n");
+		return PCI_ERS_RESULT_DISCONNECT;
+	}
+
 	return PCI_ERS_RESULT_RECOVERED;
 }
 
@@ -4726,8 +4732,6 @@ static void qlge_io_resume(struct pci_dev *pdev)
 	struct ql_adapter *qdev = netdev_priv(ndev);
 	int err = 0;
 
-	if (ql_adapter_reset(qdev))
-		QPRINTK(qdev, DRV, ERR, "reset FAILED!\n");
 	if (netif_running(ndev)) {
 		err = qlge_open(ndev);
 		if (err) {
-- 
1.6.0.2

Re: [net-next PATCH 1/3] qlge: Move reset from eeh io_resume to slot_reset.

[David Miller]

From: Ron Mercer <ron.mercer@qlogic.com>
Date: Wed,  3 Feb 2010 09:24:11 -0800

> Issue asic reset and verify functionality before continuing to the
> resume call.  This allows proper error code to be returned in the case
> the asic does not recover.
> 
> Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>

Applied.

[net-next PATCH 2/3] qlge: Add check for eeh failure when closing device.

[Ron Mercer]

Fix crash where resources are freed twice on an eeh recovery failure.
If eeh recovery fails we set a flag to indicate to close() that
resources have been freed.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h      |    1 +
 drivers/net/qlge/qlge_main.c |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 9169c4c..780a387 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -2005,6 +2005,7 @@ enum {
 	QL_SELFTEST = 9,
 	QL_LB_LINK_UP = 10,
 	QL_FRC_COREDUMP = 11,
+	QL_EEH_FATAL = 12,
 };
 
 /* link_status bit definitions */
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 87ec720..7e00029 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -3929,6 +3929,16 @@ static int qlge_close(struct net_device *ndev)
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
 
+	/* If we hit pci_channel_io_perm_failure
+	 * failure condition, then we already
+	 * brought the adapter down.
+	 */
+	if (test_bit(QL_EEH_FATAL, &qdev->flags)) {
+		QPRINTK(qdev, DRV, ERR, "EEH fatal did unload.\n");
+		clear_bit(QL_EEH_FATAL, &qdev->flags);
+		return 0;
+	}
+
 	/*
 	 * Wait for device to recover from a reset.
 	 * (Rarely happens, but possible.)
@@ -4677,6 +4687,7 @@ static pci_ers_result_t qlge_io_error_detected(struct pci_dev *pdev,
 					       enum pci_channel_state state)
 {
 	struct net_device *ndev = pci_get_drvdata(pdev);
+	struct ql_adapter *qdev = netdev_priv(ndev);
 
 	switch (state) {
 	case pci_channel_io_normal:
@@ -4690,6 +4701,8 @@ static pci_ers_result_t qlge_io_error_detected(struct pci_dev *pdev,
 	case pci_channel_io_perm_failure:
 		dev_err(&pdev->dev,
 			"%s: pci_channel_io_perm_failure.\n", __func__);
+		ql_eeh_close(ndev);
+		set_bit(QL_EEH_FATAL, &qdev->flags);
 		return PCI_ERS_RESULT_DISCONNECT;
 	}
 
@@ -4720,6 +4733,7 @@ static pci_ers_result_t qlge_io_slot_reset(struct pci_dev *pdev)
 
 	if (ql_adapter_reset(qdev)) {
 		QPRINTK(qdev, DRV, ERR, "reset FAILED!\n");
+		set_bit(QL_EEH_FATAL, &qdev->flags);
 		return PCI_ERS_RESULT_DISCONNECT;
 	}
 
-- 
1.6.0.2

Re: [net-next PATCH 2/3] qlge: Add check for eeh failure when closing device.

[David Miller]

From: Ron Mercer <ron.mercer@qlogic.com>
Date: Wed,  3 Feb 2010 09:24:12 -0800

> Fix crash where resources are freed twice on an eeh recovery failure.
> If eeh recovery fails we set a flag to indicate to close() that
> resources have been freed.
> 
> Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>

Applied.

[net-next PATCH 3/3] qlge: Add watchdog timer.

[Ron Mercer]

Add periodic heartbeat register read to trigger the eeh
recovery process.
We see cases where an eeh error was injected and the slot was
suspended.  An asic access attempt is required to flush the recovery process,
but without interrupts the process can stall.
Adding this periodic register read causes the recovery process to begin.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h      |    1 +
 drivers/net/qlge/qlge_main.c |   26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 780a387..ebfd177 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -2145,6 +2145,7 @@ struct ql_adapter {
 	struct completion ide_completion;
 	struct nic_operations *nic_ops;
 	u16 device_id;
+	struct timer_list timer;
 	atomic_t lb_count;
 };
 
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 7e00029..8f2a5ae 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4574,6 +4574,20 @@ static const struct net_device_ops qlge_netdev_ops = {
 	.ndo_vlan_rx_kill_vid	= qlge_vlan_rx_kill_vid,
 };
 
+static void ql_timer(unsigned long data)
+{
+	struct ql_adapter *qdev = (struct ql_adapter *)data;
+	u32 var = 0;
+
+	var = ql_read32(qdev, STS);
+	if (pci_channel_offline(qdev->pdev)) {
+		QPRINTK(qdev, IFUP, ERR, "EEH STS = 0x%.08x.\n", var);
+		return;
+	}
+
+	mod_timer(&qdev->timer, jiffies + HZ);
+}
+
 static int __devinit qlge_probe(struct pci_dev *pdev,
 				const struct pci_device_id *pci_entry)
 {
@@ -4625,6 +4639,13 @@ static int __devinit qlge_probe(struct pci_dev *pdev,
 		pci_disable_device(pdev);
 		return err;
 	}
+	/* Start up the timer to trigger EEH if
+	 * the bus goes dead
+	 */
+	init_timer(&qdev->timer);
+	qdev->timer.data = (unsigned long)qdev;
+	qdev->timer.function = ql_timer;
+	mod_timer(&qdev->timer, jiffies + HZ);
 	ql_link_off(qdev);
 	ql_display_dev_info(ndev);
 	atomic_set(&qdev->lb_count, 0);
@@ -4645,6 +4666,8 @@ int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget)
 static void __devexit qlge_remove(struct pci_dev *pdev)
 {
 	struct net_device *ndev = pci_get_drvdata(pdev);
+	struct ql_adapter *qdev = netdev_priv(ndev);
+	del_timer_sync(&qdev->timer);
 	unregister_netdev(ndev);
 	ql_release_all(pdev);
 	pci_disable_device(pdev);
@@ -4757,6 +4780,7 @@ static void qlge_io_resume(struct pci_dev *pdev)
 		QPRINTK(qdev, IFUP, ERR,
 			"Device was not running prior to EEH.\n");
 	}
+	mod_timer(&qdev->timer, jiffies + HZ);
 	netif_device_attach(ndev);
 }
 
@@ -4773,6 +4797,7 @@ static int qlge_suspend(struct pci_dev *pdev, pm_message_t state)
 	int err;
 
 	netif_device_detach(ndev);
+	del_timer_sync(&qdev->timer);
 
 	if (netif_running(ndev)) {
 		err = ql_adapter_down(qdev);
@@ -4817,6 +4842,7 @@ static int qlge_resume(struct pci_dev *pdev)
 			return err;
 	}
 
+	mod_timer(&qdev->timer, jiffies + HZ);
 	netif_device_attach(ndev);
 
 	return 0;
-- 
1.6.0.2

Re: [net-next PATCH 3/3] qlge: Add watchdog timer.

[David Miller]

From: Ron Mercer <ron.mercer@qlogic.com>
Date: Wed,  3 Feb 2010 09:24:13 -0800

> Add periodic heartbeat register read to trigger the eeh
> recovery process.
> We see cases where an eeh error was injected and the slot was
> suspended.  An asic access attempt is required to flush the recovery process,
> but without interrupts the process can stall.
> Adding this periodic register read causes the recovery process to begin.
> 
> Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>

This does a PIO to the chip every single second?

How about something like 5 or 10 seconds, and use the
deferrable interfaces so that it's more friendly to
power management on an otherwise idle system?

Thanks.

Re: [net-next PATCH 3/3] qlge: Add watchdog timer.

[Ron Mercer]

> This does a PIO to the chip every single second?
> 
> How about something like 5 or 10 seconds, and use the
> deferrable interfaces so that it's more friendly to
> power management on an otherwise idle system?
>

You are correct that we don't need to check every second.
I will increase the duration and test it out a deferrable timer.

Thanks

Re: [net-next PATCH 3/3] qlge: Add watchdog timer.

[Ron Mercer]

>From 81cc7ea1ce2b906a29ad250daeb120101b70f4f1 Mon Sep 17 00:00:00 2001
From: Ron Mercer <ron.mercer@qlogic.com>
Date: Thu, 4 Feb 2010 11:54:37 -0800
Subject: [net-next PATCH 1/1] qlge: Add watchdog timer.

Add deferred timer for 5-second periodic heartbeat register read to
trigger the eeh recovery process.
We see cases where an eeh error was injected and the slot was
suspended.  An asic access attempt is required to flush the recovery process,
but without interrupts the process can stall.
Adding this periodic register read causes the recovery process to begin.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h      |    1 +
 drivers/net/qlge/qlge_main.c |   27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 780a387..ebfd177 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -2145,6 +2145,7 @@ struct ql_adapter {
 	struct completion ide_completion;
 	struct nic_operations *nic_ops;
 	u16 device_id;
+	struct timer_list timer;
 	atomic_t lb_count;
 };
 
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 7e00029..f1fc3a8 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4574,6 +4574,20 @@ static const struct net_device_ops qlge_netdev_ops = {
 	.ndo_vlan_rx_kill_vid	= qlge_vlan_rx_kill_vid,
 };
 
+static void ql_timer(unsigned long data)
+{
+	struct ql_adapter *qdev = (struct ql_adapter *)data;
+	u32 var = 0;
+
+	var = ql_read32(qdev, STS);
+	if (pci_channel_offline(qdev->pdev)) {
+		QPRINTK(qdev, IFUP, ERR, "EEH STS = 0x%.08x.\n", var);
+		return;
+	}
+
+	add_timer(&qdev->timer);
+}
+
 static int __devinit qlge_probe(struct pci_dev *pdev,
 				const struct pci_device_id *pci_entry)
 {
@@ -4625,6 +4639,14 @@ static int __devinit qlge_probe(struct pci_dev *pdev,
 		pci_disable_device(pdev);
 		return err;
 	}
+	/* Start up the timer to trigger EEH if
+	 * the bus goes dead
+	 */
+	init_timer_deferrable(&qdev->timer);
+	qdev->timer.data = (unsigned long)qdev;
+	qdev->timer.function = ql_timer;
+	qdev->timer.expires = jiffies + (5*HZ);
+	add_timer(&qdev->timer);
 	ql_link_off(qdev);
 	ql_display_dev_info(ndev);
 	atomic_set(&qdev->lb_count, 0);
@@ -4645,6 +4667,8 @@ int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget)
 static void __devexit qlge_remove(struct pci_dev *pdev)
 {
 	struct net_device *ndev = pci_get_drvdata(pdev);
+	struct ql_adapter *qdev = netdev_priv(ndev);
+	del_timer_sync(&qdev->timer);
 	unregister_netdev(ndev);
 	ql_release_all(pdev);
 	pci_disable_device(pdev);
@@ -4757,6 +4781,7 @@ static void qlge_io_resume(struct pci_dev *pdev)
 		QPRINTK(qdev, IFUP, ERR,
 			"Device was not running prior to EEH.\n");
 	}
+	add_timer(&qdev->timer);
 	netif_device_attach(ndev);
 }
 
@@ -4773,6 +4798,7 @@ static int qlge_suspend(struct pci_dev *pdev, pm_message_t state)
 	int err;
 
 	netif_device_detach(ndev);
+	del_timer_sync(&qdev->timer);
 
 	if (netif_running(ndev)) {
 		err = ql_adapter_down(qdev);
@@ -4817,6 +4843,7 @@ static int qlge_resume(struct pci_dev *pdev)
 			return err;
 	}
 
+	add_timer(&qdev->timer);
 	netif_device_attach(ndev);
 
 	return 0;
-- 
1.6.0.2

Re: [net-next PATCH 3/3] qlge: Add watchdog timer.

[David Miller]

From: Ron Mercer <ron.mercer@qlogic.com>
Date: Thu, 4 Feb 2010 12:06:03 -0800

> +static void ql_timer(unsigned long data)
> +{
> +	struct ql_adapter *qdev = (struct ql_adapter *)data;
> +	u32 var = 0;
> +
> +	var = ql_read32(qdev, STS);
> +	if (pci_channel_offline(qdev->pdev)) {
> +		QPRINTK(qdev, IFUP, ERR, "EEH STS = 0x%.08x.\n", var);
> +		return;
> +	}
> +
> +	add_timer(&qdev->timer);
> +}
> +

Did the timer every fire more than once in your testing?

Don't you need to re-setup the expiration period before
adding it again?

Same goes for all of those sequences where you go:

	del_timer_sync(&qdev->timer);

	...

	add_timer(&qdev->timer);

in the patch.

Re: [net-next PATCH 3/3] qlge: Add watchdog timer.

[Ron Mercer]

> Did the timer every fire more than once in your testing?
> 
> Don't you need to re-setup the expiration period before
> adding it again?

It does fire repeatedly but I should have put in the new expiration
period.  I added some prints to the timer and without resetting the
expiration the timer would pop faster, but still less than 5 seconds.
I've added the expiration here and see very even pops on 5 second
intervals.

>From 4637fe181eda8440282b6a3acc0bc2c5aefbd7ea Mon Sep 17 00:00:00 2001
From: Ron Mercer <ron.mercer@qlogic.com>
Date: Thu, 4 Feb 2010 13:11:43 -0800
Subject: [net-next PATCH 1/1] qlge: Add watchdog timer.

Add periodic heartbeat register read to trigger the eeh
recovery process.
We see cases where an eeh error was injected and the slot was
suspended.  An asic access attempt is required to flush the recovery process,
but without interrupts the process can stall.
Adding this periodic register read causes the recovery process to begin.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h      |    1 +
 drivers/net/qlge/qlge_main.c |   30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 780a387..ebfd177 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -2145,6 +2145,7 @@ struct ql_adapter {
 	struct completion ide_completion;
 	struct nic_operations *nic_ops;
 	u16 device_id;
+	struct timer_list timer;
 	atomic_t lb_count;
 };
 
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 7e00029..87a40d1 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4574,6 +4574,21 @@ static const struct net_device_ops qlge_netdev_ops = {
 	.ndo_vlan_rx_kill_vid	= qlge_vlan_rx_kill_vid,
 };
 
+static void ql_timer(unsigned long data)
+{
+	struct ql_adapter *qdev = (struct ql_adapter *)data;
+	u32 var = 0;
+
+	var = ql_read32(qdev, STS);
+	if (pci_channel_offline(qdev->pdev)) {
+		QPRINTK(qdev, IFUP, ERR, "EEH STS = 0x%.08x.\n", var);
+		return;
+	}
+
+	qdev->timer.expires = jiffies + (5*HZ);
+	add_timer(&qdev->timer);
+}
+
 static int __devinit qlge_probe(struct pci_dev *pdev,
 				const struct pci_device_id *pci_entry)
 {
@@ -4625,6 +4640,14 @@ static int __devinit qlge_probe(struct pci_dev *pdev,
 		pci_disable_device(pdev);
 		return err;
 	}
+	/* Start up the timer to trigger EEH if
+	 * the bus goes dead
+	 */
+	init_timer_deferrable(&qdev->timer);
+	qdev->timer.data = (unsigned long)qdev;
+	qdev->timer.function = ql_timer;
+	qdev->timer.expires = jiffies + (5*HZ);
+	add_timer(&qdev->timer);
 	ql_link_off(qdev);
 	ql_display_dev_info(ndev);
 	atomic_set(&qdev->lb_count, 0);
@@ -4645,6 +4668,8 @@ int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget)
 static void __devexit qlge_remove(struct pci_dev *pdev)
 {
 	struct net_device *ndev = pci_get_drvdata(pdev);
+	struct ql_adapter *qdev = netdev_priv(ndev);
+	del_timer_sync(&qdev->timer);
 	unregister_netdev(ndev);
 	ql_release_all(pdev);
 	pci_disable_device(pdev);
@@ -4757,6 +4782,8 @@ static void qlge_io_resume(struct pci_dev *pdev)
 		QPRINTK(qdev, IFUP, ERR,
 			"Device was not running prior to EEH.\n");
 	}
+	qdev->timer.expires = jiffies + (5*HZ);
+	add_timer(&qdev->timer);
 	netif_device_attach(ndev);
 }
 
@@ -4773,6 +4800,7 @@ static int qlge_suspend(struct pci_dev *pdev, pm_message_t state)
 	int err;
 
 	netif_device_detach(ndev);
+	del_timer_sync(&qdev->timer);
 
 	if (netif_running(ndev)) {
 		err = ql_adapter_down(qdev);
@@ -4817,6 +4845,8 @@ static int qlge_resume(struct pci_dev *pdev)
 			return err;
 	}
 
+	qdev->timer.expires = jiffies + (5*HZ);
+	add_timer(&qdev->timer);
 	netif_device_attach(ndev);
 
 	return 0;
-- 
1.6.0.2

Re: [net-next PATCH 3/3] qlge: Add watchdog timer.

[David Miller]

From: Ron Mercer <ron.mercer@qlogic.com>
Date: Thu, 4 Feb 2010 13:19:29 -0800

>> Did the timer every fire more than once in your testing?
>> 
>> Don't you need to re-setup the expiration period before
>> adding it again?
> 
> It does fire repeatedly but I should have put in the new expiration
> period.  I added some prints to the timer and without resetting the
> expiration the timer would pop faster, but still less than 5 seconds.
> I've added the expiration here and see very even pops on 5 second
> intervals.

Of course it pops faster, if you don't reset the expiration
period it is "right now". :-)

I'll apply this new version, thanks.