* [PATCH] jme: fix panic on load
@ 2010-10-31 15:46 Eric Dumazet
2010-10-31 16:34 ` David Miller
2010-10-31 16:39 ` Guo-Fu Tseng
0 siblings, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2010-10-31 15:46 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Guo-Fu Tseng
Its now illegal to call netif_stop_queue() before register_netdev()
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Guo-Fu Tseng <cooldavid@cooldavid.org>
---
drivers/net/jme.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index d85edf3..c57d9a4 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -2955,11 +2955,7 @@ jme_init_one(struct pci_dev *pdev,
* Tell stack that we are not ready to work until open()
*/
netif_carrier_off(netdev);
- netif_stop_queue(netdev);
- /*
- * Register netdev
- */
rc = register_netdev(netdev);
if (rc) {
pr_err("Cannot register net device\n");
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] jme: fix panic on load
2010-10-31 15:46 [PATCH] jme: fix panic on load Eric Dumazet
@ 2010-10-31 16:34 ` David Miller
2010-10-31 16:39 ` Guo-Fu Tseng
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2010-10-31 16:34 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, cooldavid
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 31 Oct 2010 16:46:18 +0100
> Its now illegal to call netif_stop_queue() before register_netdev()
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] jme: fix panic on load
2010-10-31 15:46 [PATCH] jme: fix panic on load Eric Dumazet
2010-10-31 16:34 ` David Miller
@ 2010-10-31 16:39 ` Guo-Fu Tseng
2010-10-31 16:26 ` David Miller
2010-10-31 16:32 ` Eric Dumazet
1 sibling, 2 replies; 6+ messages in thread
From: Guo-Fu Tseng @ 2010-10-31 16:39 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev
On Sun, 31 Oct 2010 16:46:18 +0100, Eric Dumazet wrote
> Its now illegal to call netif_stop_queue() before register_netdev()
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Guo-Fu Tseng <cooldavid@cooldavid.org>
> ---
> drivers/net/jme.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/net/jme.c b/drivers/net/jme.c
> index d85edf3..c57d9a4 100644
> --- a/drivers/net/jme.c
> +++ b/drivers/net/jme.c
> @@ -2955,11 +2955,7 @@ jme_init_one(struct pci_dev *pdev,
> * Tell stack that we are not ready to work until open()
> */
> netif_carrier_off(netdev);
> - netif_stop_queue(netdev);
>
> - /*
> - * Register netdev
> - */
> rc = register_netdev(netdev);
> if (rc) {
> pr_err("Cannot register net device\n");
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Can this patch be modified to move the netif_stop_queue()
after register_netdev() ?
It seems the __QUEUE_STATE_XOFF is not set after the register_netdev.
The tx_queue was kcalloc() ed without touching state flags.
Guo-Fu Tseng
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] jme: fix panic on load
2010-10-31 16:39 ` Guo-Fu Tseng
@ 2010-10-31 16:26 ` David Miller
2010-11-01 11:22 ` Guo-Fu Tseng
2010-10-31 16:32 ` Eric Dumazet
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2010-10-31 16:26 UTC (permalink / raw)
To: cooldavid; +Cc: eric.dumazet, netdev
From: "Guo-Fu Tseng" <cooldavid@cooldavid.org>
Date: Mon, 1 Nov 2010 00:39:14 +0800
> On Sun, 31 Oct 2010 16:46:18 +0100, Eric Dumazet wrote
> Can this patch be modified to move the netif_stop_queue()
> after register_netdev() ?
> It seems the __QUEUE_STATE_XOFF is not set after the register_netdev.
> The tx_queue was kcalloc() ed without touching state flags.
At the veyr moment that register_netdev() is called, the
->open() method of your driver can be invoked and the
queue state changed.
So no, you can't put it after the register call.
There is zero reason to touch the queue state in your
probe routine, the state of the queue before ->open()
is "don't care".
Eric's patch is going to be applied, it is correct.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] jme: fix panic on load
2010-10-31 16:26 ` David Miller
@ 2010-11-01 11:22 ` Guo-Fu Tseng
0 siblings, 0 replies; 6+ messages in thread
From: Guo-Fu Tseng @ 2010-11-01 11:22 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev
On Sun, 31 Oct 2010 09:26:36 -0700 (PDT), David Miller wrote
> From: "Guo-Fu Tseng" <cooldavid@cooldavid.org>
> Date: Mon, 1 Nov 2010 00:39:14 +0800
>
> > On Sun, 31 Oct 2010 16:46:18 +0100, Eric Dumazet wrote
> > Can this patch be modified to move the netif_stop_queue()
> > after register_netdev() ?
> > It seems the __QUEUE_STATE_XOFF is not set after the register_netdev.
> > The tx_queue was kcalloc() ed without touching state flags.
>
> At the veyr moment that register_netdev() is called, the
> ->open() method of your driver can be invoked and the
> queue state changed.
>
> So no, you can't put it after the register call.
>
> There is zero reason to touch the queue state in your
> probe routine, the state of the queue before ->open()
> is "don't care".
>
> Eric's patch is going to be applied, it is correct.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
I see.
Thanks for the info. :)
Guo-Fu Tseng
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] jme: fix panic on load
2010-10-31 16:39 ` Guo-Fu Tseng
2010-10-31 16:26 ` David Miller
@ 2010-10-31 16:32 ` Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2010-10-31 16:32 UTC (permalink / raw)
To: Guo-Fu Tseng; +Cc: David Miller, netdev
Le lundi 01 novembre 2010 à 00:39 +0800, Guo-Fu Tseng a écrit :
> Can this patch be modified to move the netif_stop_queue()
> after register_netdev() ?
> It seems the __QUEUE_STATE_XOFF is not set after the register_netdev.
> The tx_queue was kcalloc() ed without touching state flags.
>
Real question is : why this netif_stop_queue() was needed at the first
place ?
We believe (David and others), that using before ndo_open() is useless
or a sign of design error.
http://www.spinics.net/lists/netdev/msg145481.html
Refs: commit c117e4a2bb4911
If your driver doesnt work after my change, then another patch is
needed, but not using netif_stop_queue() right after register_netdev()
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-11-01 10:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-31 15:46 [PATCH] jme: fix panic on load Eric Dumazet
2010-10-31 16:34 ` David Miller
2010-10-31 16:39 ` Guo-Fu Tseng
2010-10-31 16:26 ` David Miller
2010-11-01 11:22 ` Guo-Fu Tseng
2010-10-31 16:32 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).