Re: Transmit timeout with E1000

[Rogier Wolff <R.E.Wolff@harddisk-recovery.nl>]

On Wed, Jan 11, 2006 at 03:11:47PM +0100, Eric Dumazet wrote:
> Rogier Wolff a écrit :
> >On Wed, Jan 11, 2006 at 02:43:49PM +0100, Erik Mouw wrote:
> >>The system only recovers after the Netdev watchdog found out that the
> >>transmit timed out. However, the e1000 register dump starts about 4 to
> >>5 seconds earlier: a possible workaround would be to trigger the
> >>timeout code path as soon as the register dump starts.
> >
> >Found a typo. 
> >
> >	Roger. 
> >
> >
> >--- e1000_main.c.orig	2006-01-11 14:53:23.000000000 +0100
> >+++ e1000_main.c	2006-01-11 14:53:38.000000000 +0100
> >@@ -3449,7 +3449,7 @@
> > 	}
> > 
> > 	for (i = 0; i < E1000_MAX_INTR; i++)
> >-		if (unlikely(!adapter->clean_rx(adapter, adapter->rx_ring) &
> >+		if (unlikely(!adapter->clean_rx(adapter, adapter->rx_ring) &&
> > 		   !e1000_clean_tx_irq(adapter, adapter->tx_ring)))
> > 			break;
> > 
> >
> >
> 
> I believe it's not a typo.
> 
> The intention is to call both clean_rx() and e1000_clean_tx_irq(), and 
> break of the loop if both said : There was no job pending.

Although (one of) the prototypes state(s) that it returns a boolean,
it is valid C to return the number of items pending.

And if one reports "2 more pending" and the other reports "1 more
pending", the "&" between the two becomes 2 & 1 => 0 / FALSE, while 2
&& 1 => TRUE.

I consider this a low prio typo, not likely to be a bug "right now",
but it is inviting someone to make it into a bug later on.

	Roger. 

-- 
+-- Rogier Wolff -- www.harddisk-recovery.nl -- 0800 220 20 20 --
| Files foetsie, bestanden kwijt, alle data weg?!
| Blijf kalm en neem contact op met Harddisk-recovery.nl!


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click