acenic lockup

acenic lockup

[Anton Blanchard]

Hi,

Ive got a bucketload of acenic adapters in a ppc64 box. I get random
tx timeouts, I suspect there is a missing memory barrier (power4 is
good at catching those). Still looking.

I did manage to lock a card up in ace_start_xmit:

restart:
...
        if (tx_ring_full(ap, ap->tx_ret_csm, idx))
	                goto overflow;
...
overflow:
        /*
         * This race condition is unavoidable with lock-free drivers.
         * We wake up the queue _before_ tx_prd is advanced, so that we
         * can
         * enter hard_start_xmit too early, while tx ring still looks
         * closed.
         * This happens ~1-4 times per 100000 packets, so that we can
         * allow
         * to loop syncing to other CPU. Probably, we need an additional
         * wmb() in ace_tx_intr as well.
         *
         * Note that this race is relieved by reserving one more entry
         * in tx ring than it is necessary (see original non-SG driver).
         * However, with SG we need to reserve 2*MAX_SKB_FRAGS+1, which
         * is already overkill.
         *
         * Alternative is to return with 1 not throttling queue. In this
         * case loop becomes longer, no more useful effects.
         */
        barrier();
        goto restart;

Its stuck there and never coming out. Alexey: I have a feeling you
wrote this code, is that correct? :)

Anton

Re: acenic lockup

[David S. Miller]

   From: Anton Blanchard <anton@samba.org>
   Date: Wed, 7 May 2003 17:06:57 +1000
   
   Its stuck there and never coming out. Alexey: I have a feeling you
   wrote this code, is that correct? :)

It is well understood failure of this driver, actually.

If Acenic hangs in any way at all, driver hangs in this loop.

Re: acenic lockup

[Alexey Kuznetsov]

Hello!

>    Its stuck there and never coming out. Alexey: I have a feeling you
>    wrote this code, is that correct? :)

Yes, I did. I am not sure about barriers, I stopped attempts
to understand this long ago. Anyway, intel does not have anything
to put here, mb() at side updating pointer looks enough.
 

There is a real ugly BUG in the driver, ace_watchdog():

 	} else {
		printk(KERN_DEBUG "%s: BUG... transmitter died. Kicking it.\n",
		       dev->name);
#if 0
		netif_wake_queue(dev);
#endif
	}

(#if is from my local tree). I remember we discussed this with Dave
and he was very angry about this flaw shared by most of drivers and, seems,
did not eat this fix.

So, if you see the message, it is surely this bug.


Alexey

Re: acenic lockup

[David S. Miller]

   From: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
   Date: Wed, 7 May 2003 21:06:32 +0400 (MSD)
   
   I remember we discussed this with Dave
   and he was very angry about this flaw shared by most of drivers
   and, seems, did not eat this fix.
   
Yes, this netif_wake_queue() in acenic's watchdog routine
is bogus.  It is guarenteed lockup.

Ok, I applied this.

However, what does kick device back into working state?

Do we make shamans dance when this message hits the logs
and pray for the best? :-)

Re: acenic lockup

[Alexey Kuznetsov]

Hello!

> However, what does kick device back into working state?

Usually it is a link problem and device will recover after
link is restored. This happens here.

If it is some PCI failure or something went wrong in hardware,
the device will stop forever, I guess. And I guess this happens
with the same frequency as memory parity errors i.e. not so much. :-)


> Do we make shamans dance when this message hits the logs
> and pray for the best? :-) 

Sort of. I was about to dance for a while when saw creepy
"ethX: BUG, tx ring is full" from tulip, which has the same bogus
netif_wake_queue(). :-)

Well, full reset is difficult thing with lock-free acenic.
Seems, it has to throttle card, wake up something at process context,
to disable irq there and to reset nic like it happens at ifconfig down
(or even module unload in face of hard hardware failure?)

Alexey

Re: acenic lockup

[David S. Miller]

   From: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
   Date: Thu, 8 May 2003 02:21:11 +0400 (MSD)

   > However, what does kick device back into working state?
   
   Usually it is a link problem and device will recover after
   link is restored. This happens here.
   
I fully recognize this.

   If it is some PCI failure or something went wrong in hardware,
   the device will stop forever, I guess. And I guess this happens
   with the same frequency as memory parity errors i.e. not so much. :-)
   
Sometimes it is not cosmic bit-flip which causes this, but rather
bit-flip caused by programmer of some unrelated area of kernel :-)))
   
I am happy that hard-hang part is probably gone now.  But I also
desire real resiliency in this area of drivers.

   > Do we make shamans dance when this message hits the logs
   > and pray for the best? :-) 
   
   Sort of. I was about to dance for a while when saw creepy
   "ethX: BUG, tx ring is full" from tulip, which has the same bogus
   netif_wake_queue(). :-)
   
Note to Jeff, independant of what is being discussed here, a real
audit of drivers that blindly invoke netif_wake_queue() from transmit
timeout watchdog routine is in order at some point.  This is what
Alexey is referring to as "same bogus netif_wake_queue()".

   Well, full reset is difficult thing with lock-free acenic.
   Seems, it has to throttle card, wake up something at process context,
   to disable irq there and to reset nic like it happens at ifconfig down
   (or even module unload in face of hard hardware failure?)

It is simple, use schedule_work(), tg3.c does exactly this.
Only difference in acenic is need to use disable_irq(), that is all.

Re: acenic lockup

[Jeff Garzik]

On Thu, May 08, 2003 at 09:00:49AM -0700, David S. Miller wrote:
> Note to Jeff, independant of what is being discussed here, a real
> audit of drivers that blindly invoke netif_wake_queue() from transmit
> timeout watchdog routine is in order at some point.  This is what
> Alexey is referring to as "same bogus netif_wake_queue()".

Agreed.  Alexey has pointed out netif_wake_queue stupidities in
drivers before, and he's absolutely right.

	Jeff



P.S. Can you please email @pobox.com for stuff that is not Red Hat
specific?  I am trying to use @redhat.com for only official Red Hat
business.  My boss forces me to bk-commit as @redhat.com, otherwise you
would never see that address in public at all.

Re: acenic lockup

[David S. Miller]

   From: Jeff Garzik <jgarzik@redhat.com>
   Date: Thu, 8 May 2003 13:11:35 -0400
   
   P.S. Can you please email @pobox.com for stuff that is not Red Hat
   specific?
   
My apologies, I will do this in the future.

How much will this affect latency though? :-)

Re: acenic lockup

[Jeff Garzik]

On Thu, May 08, 2003 at 09:09:54AM -0700, David S. Miller wrote:
>    From: Jeff Garzik <jgarzik@redhat.com>
>    Date: Thu, 8 May 2003 13:11:35 -0400
>    
>    P.S. Can you please email @pobox.com for stuff that is not Red Hat
>    specific?
>    
> My apologies, I will do this in the future.
> 
> How much will this affect latency though? :-)

Should improve it :)  I didn't check my redhat.com email all weekend
this past weekend, for example.

	Jeff

Re: acenic lockup

[Jamal Hadi]

Jeff,

Dont forget that nice looking fixed tulip which gets rid of the
nonsense tx_timeout;->
http://www.cyberus.ca/~hadi/patches/tulip-sym-fixed-20030103.tgz

cheers,
jamal