[PATCH] Fix deadlock in netconsole with no carrier

[PATCH] Fix deadlock in netconsole with no carrier

[Andi Kleen]

I got a deadlock at boot with netconsole when the netword card
did not have a cable connected. This patch fixes this by limiting
the number of retries. 

Also when we run into the device spinlock dont poll all the time,
just spin.

Signed-off-by: Andi Kleen <ak@suse.de>


diff -u net/core/netpoll.c-o net/core/netpoll.c
--- net/core/netpoll.c-o	2005-03-02 08:38:32.000000000 +0100
+++ net/core/netpoll.c	2005-04-19 15:50:42.959142733 +0200
@@ -190,9 +190,10 @@
 static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
 {
 	int status;
+	int try = 5; 
 
 repeat:
-	if(!np || !np->dev || !netif_running(np->dev)) {
+	if(try-- == 0 || !np || !np->dev || !netif_running(np->dev)) {
 		__kfree_skb(skb);
 		return;
 	}
@@ -218,6 +219,10 @@
 
 	/* transmit busy */
 	if(status) {
+		if (status == NETDEV_TX_LOCKED) {
+			try++;
+			goto repeat;
+		}
 		netpoll_poll(np);
 		goto repeat;
 	}

Re: [PATCH] Fix deadlock in netconsole with no carrier

[Matt Mackall]

On Tue, Apr 19, 2005 at 03:53:50PM +0200, Andi Kleen wrote:
> 
> I got a deadlock at boot with netconsole when the netword card
> did not have a cable connected. This patch fixes this by limiting
> the number of retries. 

It should be waiting for carrier detect before proceeding. What NIC is that?
I'm sure five retries is not enough.

> Also when we run into the device spinlock dont poll all the time,
> just spin.

Two patches? Again, I don't think we should give up so easily.

> 
> Signed-off-by: Andi Kleen <ak@suse.de>
> 
> 
> diff -u net/core/netpoll.c-o net/core/netpoll.c
> --- net/core/netpoll.c-o	2005-03-02 08:38:32.000000000 +0100
> +++ net/core/netpoll.c	2005-04-19 15:50:42.959142733 +0200
> @@ -190,9 +190,10 @@
>  static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
>  {
>  	int status;
> +	int try = 5; 
>  
>  repeat:
> -	if(!np || !np->dev || !netif_running(np->dev)) {
> +	if(try-- == 0 || !np || !np->dev || !netif_running(np->dev)) {
>  		__kfree_skb(skb);
>  		return;
>  	}
> @@ -218,6 +219,10 @@
>  
>  	/* transmit busy */
>  	if(status) {
> +		if (status == NETDEV_TX_LOCKED) {
> +			try++;
> +			goto repeat;
> +		}
>  		netpoll_poll(np);
>  		goto repeat;
>  	}

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] Fix deadlock in netconsole with no carrier

[Andi Kleen]

Matt Mackall <mpm@selenic.com> writes:

[sorry for the late answer, but you dont seem to have cced the
answer to me so I lost it until now]

> On Tue, Apr 19, 2005 at 03:53:50PM +0200, Andi Kleen wrote:
>> 
>> I got a deadlock at boot with netconsole when the netword card
>> did not have a cable connected. This patch fixes this by limiting
>> the number of retries. 
>
> It should be waiting for carrier detect before proceeding. What NIC is that?


e1000

> I'm sure five retries is not enough.

Well, infinite is definitely too many. And the early netconsole
code already waits for carrier up, so waiting even longer in the
actual write does not make much sense to me. The problem with
spinning longer here is that when you boot on a system with no
carrier but netconsole configured it will waste a lot of time
uselessly spinning/polling here all the time. It is better 
to end this early. In theory you could do a more clever backoff
scheme and note when a device is always down, but I think 
the short retry combined with the long wait at early netconsole
init is nearly equivalent.

Without this patch my setup doesnt even boot so I would
appreciate if the patch could be applied.

>
>> Also when we run into the device spinlock dont poll all the time,
>> just spin.
>
> Two patches? Again, I don't think we should give up so easily.

For the device spinlock polling is useless because the
NIC is not actually out of resources, all you need to do 
is to spin. Polling too is a waste of CPU time.
In case polling is really needed (in case of a race) it will
be retried once the spinlock is free.

-Andi