Subject: Re: [PATCH] [e1000]: Remove unnecessary tx_lock

From: jamal <[email-masked]>
Date: Sat, 05 Aug 2006 19:04:46 -0400

On Fri, 2006-04-08 at 20:25 +1000, Herbert Xu wrote:
> On Fri, Aug 04, 2006 at 06:16:03AM -0400, jamal wrote:
> > 
> > I still have my setup intact, so i will give this a whirl in about 3-4
> > hours. It does look promising - an atomic should be a lot cheaper than a
> > lock. Thanks Herbert.
> 
> Well it's not that good since the current code only takes the
> lock in exceptional circumstances while my patch does the atomic
> op unconditionally.

I run a variety of tests and it looks good i.e performance wise,
although lower than what i had before, certainly is an improvement over
what was there before (by about 1kpps in forwarding). It would probably
chew more CPU than the original driver (given the atomic) but as i said
before i have no philosophical qualms with that since it performs better
under stress ;->

Hardware is:
Dual via C3 1Ghz with a dual port e1000. The bottleneck is really
the 32 bit PCI bus in most cases. I tried running single port, dual;
low traffic, high traffic etc.

Interesting thing i noticed:
Under high speed forwarding test bidirectional (eth0<--forwarding-->eth1)

whenever __e1000_maybe_stop_tx() --> __netif_test_and_stop_queue() was 
called it was _guaranteed to find XOFF not set_. i.e 100% of the time. 

This seems to actually happen on eth0 only (i think the two ports share
the same PCI bridge and probably eth0 is biased?).

If you want me to try something else, send an incremental patch and i
can do it tomorrow.

cheers,
jamal