* [PATCH] e100_shutdown: netif_poll_disable hang
@ 2006-10-20 18:28 Daniel Walker
2006-10-20 20:51 ` Auke Kok
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Walker @ 2006-10-20 18:28 UTC (permalink / raw)
To: Andrew Morton; +Cc: Auke Kok, linux-kernel
My machine annoyingly hangs while rebooting. I tracked it down
to e100-fix-reboot-f-with-netconsole-enabled.patch in 2.6.18-rc2-mm2
I review the changes and it seemed to be calling netif_poll_disable
one too many time. Once in e100_down(), and again in e100_shutdown().
The second one in e100_shutdown() caused the hang. So this patch
removes it.
Signed-Off-By: Daniel Walker <dwalker@mvista.com>
---
drivers/net/e100.c | 1 -
1 files changed, 1 deletion(-)
Index: linux-2.6.18/drivers/net/e100.c
===================================================================
--- linux-2.6.18.orig/drivers/net/e100.c
+++ linux-2.6.18/drivers/net/e100.c
@@ -2709,7 +2709,6 @@ static void e100_shutdown(struct pci_dev
struct net_device *netdev = pci_get_drvdata(pdev);
struct nic *nic = netdev_priv(netdev);
- netif_poll_disable(nic->netdev);
del_timer_sync(&nic->watchdog);
netif_carrier_off(nic->netdev);
--
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] e100_shutdown: netif_poll_disable hang
2006-10-20 18:28 [PATCH] e100_shutdown: netif_poll_disable hang Daniel Walker
@ 2006-10-20 20:51 ` Auke Kok
2006-10-20 21:09 ` Auke Kok
0 siblings, 1 reply; 6+ messages in thread
From: Auke Kok @ 2006-10-20 20:51 UTC (permalink / raw)
To: Daniel Walker, Andrew Morton, Jeff Garzik
Cc: linux-kernel, Jesse Brandeburg, NetDev
Daniel Walker wrote:
> My machine annoyingly hangs while rebooting. I tracked it down
> to e100-fix-reboot-f-with-netconsole-enabled.patch in 2.6.18-rc2-mm2
>
> I review the changes and it seemed to be calling netif_poll_disable
> one too many time. Once in e100_down(), and again in e100_shutdown().
>
> The second one in e100_shutdown() caused the hang. So this patch
> removes it.
>
> Signed-Off-By: Daniel Walker <dwalker@mvista.com>
>
> ---
> drivers/net/e100.c | 1 -
> 1 files changed, 1 deletion(-)
>
> Index: linux-2.6.18/drivers/net/e100.c
> ===================================================================
> --- linux-2.6.18.orig/drivers/net/e100.c
> +++ linux-2.6.18/drivers/net/e100.c
> @@ -2709,7 +2709,6 @@ static void e100_shutdown(struct pci_dev
> struct net_device *netdev = pci_get_drvdata(pdev);
> struct nic *nic = netdev_priv(netdev);
>
> - netif_poll_disable(nic->netdev);
> del_timer_sync(&nic->watchdog);
> netif_carrier_off(nic->netdev);
>
> --
this won't help netconsole shutdown/reboot -f, probably locking it up again!
NAK
Also missing is the NAPI conditional, which I left out. Also missing is the same code
for suspend.
Here's a better approach, allowing both normal shutdown code path and reboot -f to
disable polling.
Cheers,
Auke
---
e100: disable polling only when up during suspend and shutdown
Signed-off-by: Auke Kok <auke-jan.h.kok>
Cc: Daniel Walker <dwalker@mvista.com>
Cc: Andrew Morton <akpm@osdl.org>
---
drivers/net/e100.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
---
diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index d4a2572..815eb29 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -2719,7 +2719,10 @@ static int e100_suspend(struct pci_dev *
struct net_device *netdev = pci_get_drvdata(pdev);
struct nic *nic = netdev_priv(netdev);
- netif_poll_disable(nic->netdev);
+#ifdef CONFIG_E100_NAPI
+ if (netif_running(netdev))
+ netif_poll_disable(nic->netdev);
+#endif
del_timer_sync(&nic->watchdog);
netif_carrier_off(nic->netdev);
@@ -2763,7 +2766,10 @@ static void e100_shutdown(struct pci_dev
struct net_device *netdev = pci_get_drvdata(pdev);
struct nic *nic = netdev_priv(netdev);
- netif_poll_disable(nic->netdev);
+#ifdef CONFIG_E100_NAPI
+ if (netif_running(netdev))
+ netif_poll_disable(nic->netdev);
+#endif
del_timer_sync(&nic->watchdog);
netif_carrier_off(nic->netdev);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] e100_shutdown: netif_poll_disable hang
2006-10-20 20:51 ` Auke Kok
@ 2006-10-20 21:09 ` Auke Kok
2006-10-21 17:41 ` Damien Wyart
0 siblings, 1 reply; 6+ messages in thread
From: Auke Kok @ 2006-10-20 21:09 UTC (permalink / raw)
To: Auke Kok
Cc: Daniel Walker, Andrew Morton, Jeff Garzik, linux-kernel,
Jesse Brandeburg, NetDev
Auke Kok wrote:
> Daniel Walker wrote:
>> My machine annoyingly hangs while rebooting. I tracked it down
>> to e100-fix-reboot-f-with-netconsole-enabled.patch in 2.6.18-rc2-mm2
>>
>> I review the changes and it seemed to be calling netif_poll_disable
>> one too many time. Once in e100_down(), and again in e100_shutdown().
>>
>> The second one in e100_shutdown() caused the hang. So this patch
>> removes it.
it doesn't even do harm to netif_poll_disable() twice as far as I can see, as it merely
calls test_and_set_bit(), which will instantly succeed on the first attempt if the bit
was already set.
did this change actually fix it for you? I'm wondering if the netif_carrier_off might
not be the culprit here...
Auke
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] e100_shutdown: netif_poll_disable hang
2006-10-20 21:09 ` Auke Kok
@ 2006-10-21 17:41 ` Damien Wyart
2006-10-21 17:54 ` Auke Kok
0 siblings, 1 reply; 6+ messages in thread
From: Damien Wyart @ 2006-10-21 17:41 UTC (permalink / raw)
To: Auke Kok
Cc: Daniel Walker, Andrew Morton, Jeff Garzik, linux-kernel,
Jesse Brandeburg, NetDev
> > > My machine annoyingly hangs while rebooting. I tracked it down to
> > > e100-fix-reboot-f-with-netconsole-enabled.patch in 2.6.18-rc2-mm2
> > > I review the changes and it seemed to be calling
> > > netif_poll_disable one too many time. Once in e100_down(), and
> > > again in e100_shutdown().
> > > The second one in e100_shutdown() caused the hang. So this patch
> > > removes it.
* Auke Kok <auke-jan.h.kok@intel.com> [061020 23:09]:
> it doesn't even do harm to netif_poll_disable() twice as far as I can
> see, as it merely calls test_and_set_bit(), which will instantly
> succeed on the first attempt if the bit was already set.
> did this change actually fix it for you? I'm wondering if the
> netif_carrier_off might not be the culprit here...
I can confirm the proposed original change of D. Walker fixed the
problem for me. I did not test the change you proposed as a followup.
--
Damien Wyart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] e100_shutdown: netif_poll_disable hang
2006-10-21 17:41 ` Damien Wyart
@ 2006-10-21 17:54 ` Auke Kok
2006-10-21 20:55 ` Damien Wyart
0 siblings, 1 reply; 6+ messages in thread
From: Auke Kok @ 2006-10-21 17:54 UTC (permalink / raw)
To: Damien Wyart
Cc: Daniel Walker, Andrew Morton, Jeff Garzik, linux-kernel,
Jesse Brandeburg, NetDev
Damien Wyart wrote:
>>>> My machine annoyingly hangs while rebooting. I tracked it down to
>>>> e100-fix-reboot-f-with-netconsole-enabled.patch in 2.6.18-rc2-mm2
>>>> I review the changes and it seemed to be calling
>>>> netif_poll_disable one too many time. Once in e100_down(), and
>>>> again in e100_shutdown().
>>>> The second one in e100_shutdown() caused the hang. So this patch
>>>> removes it.
>
> * Auke Kok <auke-jan.h.kok@intel.com> [061020 23:09]:
>> it doesn't even do harm to netif_poll_disable() twice as far as I can
>> see, as it merely calls test_and_set_bit(), which will instantly
>> succeed on the first attempt if the bit was already set.
>
>> did this change actually fix it for you? I'm wondering if the
>> netif_carrier_off might not be the culprit here...
>
> I can confirm the proposed original change of D. Walker fixed the
> problem for me. I did not test the change you proposed as a followup.
his change breaks something else (a reboot with netconsole, possibly suspend). Please
give the latest version I sent a try. Daniel confirmed me that it works, but it's always
nice to hear it from more people.
Thanks
Auke
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] e100_shutdown: netif_poll_disable hang
2006-10-21 17:54 ` Auke Kok
@ 2006-10-21 20:55 ` Damien Wyart
0 siblings, 0 replies; 6+ messages in thread
From: Damien Wyart @ 2006-10-21 20:55 UTC (permalink / raw)
To: Auke Kok; +Cc: Andrew Morton, Jeff Garzik, linux-kernel, Jesse Brandeburg,
NetDev
* Auke Kok <auke-jan.h.kok@intel.com> [2006-10-21 10:54]: his change
> breaks something else (a reboot with netconsole, possibly suspend).
> Please give the latest version I sent a try. Daniel confirmed me that
> it works, but it's always nice to hear it from more people.
Yes, your version works here too.
--
Damien Wyart
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-10-21 20:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-20 18:28 [PATCH] e100_shutdown: netif_poll_disable hang Daniel Walker
2006-10-20 20:51 ` Auke Kok
2006-10-20 21:09 ` Auke Kok
2006-10-21 17:41 ` Damien Wyart
2006-10-21 17:54 ` Auke Kok
2006-10-21 20:55 ` Damien Wyart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox