Patchwork sky2: Lock transmit queue while disabling device

login
register
mail settings
Submitter Jarek Poplawski
Date Jan. 1, 2010, 6:31 p.m.
Message ID <20100101183155.GA8519@del.dom.local>
Download mbox | patch
Permalink /patch/41989/
State RFC
Delegated to: David Miller
Headers show

Comments

Jarek Poplawski - Jan. 1, 2010, 6:31 p.m.
On Fri, Jan 01, 2010 at 08:51:23AM +0900, Mike McCormack wrote:
> Another way to solve the problem would be to take the transmit lock in 
> netif_device_detach() to make sure that any in progress transmits have
> completed before returning.

Yes, it seems this lock might be needed around this place, but I'd
like to check another idea: if it's not about awakening too early.
Berck, could you try this patch?

Thanks,
Jarek P.
---

 drivers/net/sky2.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

--
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
Berck Nash - Jan. 4, 2010, 2:44 a.m.
Jarek Poplawski wrote:
> Yes, it seems this lock might be needed around this place, but I'd
> like to check another idea: if it's not about awakening too early.
> Berck, could you try this patch?

Okay, after running with that for several days I have not gotten the
oops.  That doesn't mean that I won't tomorrow, but I've gotten several
of these:

[45621.704025] sky2 eth0: hung mac 124:21 fifo 195 (127:122)
[45621.704027] sky2 eth0: receiver hang detected
[45621.708524] sky2 eth0: disabling interface
[45621.715229] sky2 eth0: enabling interface
[45624.862111] sky2 eth0: Link is up at 1000 Mbps, full duplex, flow
control both
[61024.704036] sky2 eth0: hung mac 124:75 fifo 195 (133:128)
[61024.704039] sky2 eth0: receiver hang detected
[61024.708487] sky2 eth0: disabling interface
[61024.714791] sky2 eth0: enabling interface
[61027.864288] sky2 eth0: Link is up at 1000 Mbps, full duplex, flow
control both

And it hasn't crashed.  The "receiver hang detected" would often (but
not always) be followed by the oops before.

Berck
> ---
> 
>  drivers/net/sky2.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
> index 1c01b96..31f1629 100644
> --- a/drivers/net/sky2.c
> +++ b/drivers/net/sky2.c
> @@ -1844,7 +1844,7 @@ static void sky2_tx_complete(struct sky2_port *sky2, u16 done)
>  	sky2->tx_cons = idx;
>  	smp_mb();
>  
> -	if (tx_avail(sky2) > MAX_SKB_TX_LE + 4)
> +	if (tx_avail(sky2) > MAX_SKB_TX_LE + 4 && netif_device_present(dev))
>  		netif_wake_queue(dev);
>  }
>  
> 

--
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

Patch

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 1c01b96..31f1629 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1844,7 +1844,7 @@  static void sky2_tx_complete(struct sky2_port *sky2, u16 done)
 	sky2->tx_cons = idx;
 	smp_mb();
 
-	if (tx_avail(sky2) > MAX_SKB_TX_LE + 4)
+	if (tx_avail(sky2) > MAX_SKB_TX_LE + 4 && netif_device_present(dev))
 		netif_wake_queue(dev);
 }