Tigon3 5701 PCI-X recv performance problem

Tigon3 5701 PCI-X recv performance problem

[John Partridge]

I am seeing a problem with PCI-X recv performance on the Broadcom 5701 cards.
This is due to a known PCI-X errata with the DMA engine when buffers are non zero
offset aligned. As well as performance problems there are also a lot of kernel
unaligned access messages in the system log (hundereds of them) :-

kernel unaligned access to 0xe0000030174f382e, ip=0xe0000000047d1f80

This affects both 2.4x and 2.6x tg3.o drivers

Measure of performance WITHOUT fix :-
mig125:~ # nttcp -r -T -l262144 -w1024 -n1000 10.50.1.130 -l262144 -w1024
      Bytes  Real s   CPU s Real-MBit/s  CPU-MBit/s   Calls  Real-C/s   CPU-C/s
l2621440000  118.36   43.24    177.1846    484.9782 1818304  15362.52   42049.3
12621440000  118.34   12.70    177.2167   1651.3857   10000     84.50     787.4

I then applied a fix to the driver :-

line 2260 of linux-2.6.0-test6/drivers/net/tg3.c

-               if (len > RX_COPY_THRESHOLD) {
+               if (len > RX_COPY_THRESHOLD && tp->rx_offset == 2) {

Then I loaded a tg3.o module which has the fix, and ran the test again

Measure of performance WITH fix :-
mig125:~ # nttcp -r -T -l262144 -w1024 -n1000 10.50.1.130 -l262144 -w1024
      Bytes  Real s   CPU s Real-MBit/s  CPU-MBit/s   Calls  Real-C/s   CPU-C/s
l262144000    2.23    1.64    940.6910   1277.5033   35082  15736.26   21370.6
1262144000    2.22    1.43    944.0677   1470.4690    1000    450.17     701.2

This ONLY affects Broadcom 5701 based Gigabit Ethernet cards 5704 cards do not
have the same errata. I confirmed that the fix did not affect the performance
or functionality of 5704 cards, actually the fix ensures that the 5701 cards don't
go through the same code path as 5704 (becuase tp->rx_offset is 0 for 5701 so we
always align the buffers for the 5701 card).

John

-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com	

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

Oh yeah?  What are your numbers like if you just disable the ia64
kernel unaligned access printk()?

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

Not too much different

mig125:~ # nttcp -r -T -l262144 -w1024 -n1000 10.50.1.130 -l262144 -w1024
      Bytes  Real s   CPU s Real-MBit/s  CPU-MBit/s   Calls  Real-C/s   CPU-C/s
l318341120   10.00    9.96    254.6131    255.6215   22500   2249.47    2258.4
1318341120   10.00    0.41    254.6825   6156.0247   19430   1943.07   46966.7

David S. Miller wrote:
> Oh yeah?  What are your numbers like if you just disable the ia64
> kernel unaligned access printk()?

-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Wed, 08 Oct 2003 12:52:57 -0500
John Partridge <johnip@sgi.com> wrote:

> Not too much different

The problem is that your change is arch-dependant yet you make it
run on all platforms.

On x86 we don't want to do what your change is doing, the unaligned
accesses are cheap enough.

We need to abstract this, probably in the same way it is done in
the Tulip and other drivers which have similar issues.

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

OK, fair enough, you mean like :-

#if defined(__ia64__)
	if (len > RX_COPY_THRESHOLD && tp->rx_offset == 2) {
#else
	if (len > RX_COPY_THRESHOLD) {
#endif



David S. Miller wrote:
> On Wed, 08 Oct 2003 12:52:57 -0500
> John Partridge <johnip@sgi.com> wrote:
> 
> 
>>Not too much different
> 
> 
> The problem is that your change is arch-dependant yet you make it
> run on all platforms.
> 
> On x86 we don't want to do what your change is doing, the unaligned
> accesses are cheap enough.
> 
> We need to abstract this, probably in the same way it is done in
> the Tulip and other drivers which have similar issues.

-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com

Re: Tigon3 5701 PCI-X recv performance problem

[Steve Modica]

I think it'd be better to create some macro like this:

#ifdef FORCE_SKB_ALIGNMENT
# define SKB_COPY_CHECK(len) len > RX_COPY_THRESHOLD && tp->rx_offset == 2
#else
# define SKB_COPY_CHECK(len) len > RX_COPY_THRESHOLD
#endif

Then replace the code in the if with the new macro.  We can define 
FORCE_SKB_ALIGNMENT in our build environment as can others if necessary. 
otherwise, everyone does what they did before.

Steve



John Partridge wrote:
> OK, fair enough, you mean like :-
> 
> #if defined(__ia64__)
>     if (len > RX_COPY_THRESHOLD && tp->rx_offset == 2) {
> #else
>     if (len > RX_COPY_THRESHOLD) {
> #endif
> 
> 
> 
> David S. Miller wrote:
> 
>> On Wed, 08 Oct 2003 12:52:57 -0500
>> John Partridge <johnip@sgi.com> wrote:
>>
>>
>>> Not too much different
>>
>>
>>
>> The problem is that your change is arch-dependant yet you make it
>> run on all platforms.
>>
>> On x86 we don't want to do what your change is doing, the unaligned
>> accesses are cheap enough.
>>
>> We need to abstract this, probably in the same way it is done in
>> the Tulip and other drivers which have similar issues.
> 
> 


-- 
Steve Modica
work: 651-683-3224
MTS-Technical Lead
"Give a man a fish, and he will eat for a day, hit him with a fish and
he leaves you alone" - me

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Wed, 08 Oct 2003 14:11:47 -0500
Steve Modica <modica@sgi.com> wrote:

> I think it'd be better to create some macro like this:

Please, do this right.  Make a "CONFIG_UNALIGNED_VERY_SLOW"
or something that platforms can define, and then drivers can
ifdef upon.

Re: Tigon3 5701 PCI-X recv performance problem

[Steve Modica]

David S. Miller wrote:
> Oh yeah?  What are your numbers like if you just disable the ia64
> kernel unaligned access printk()?

Hi David,

it's definitely not the printk.  They have that throttled so it only prints once 
for a large number of occurances.

The problem is that on the Altix platform they have to deal with unaligned 
accesses via an exception handler.  This causes them to run through hundreds of 
instructions.  There's evidntally another mechanism in the cpu that's more 
efficient in daling with unaligned accesses, but we can't use that for some 
reason (atomicity I think, but I don't recall).

This extra memcpy when the buffer is not unaligned currently only impacts 5701 
(since it's the only chip for which the 2 byte pad is omitted) although the 
alignment issue would bite us no matter what the chip if the 2 byte pad is omitted.

Steve


-- 
Steve Modica
work: 651-683-3224
MTS-Technical Lead
"Give a man a fish, and he will eat for a day, hit him with a fish and
he leaves you alone" - me

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Wed, 08 Oct 2003 13:21:50 -0500
Steve Modica <modica@sgi.com> wrote:

> The problem is that on the Altix platform they have to deal with unaligned 
> accesses via an exception handler.

You mean, ia64.

Has anyone optimized the unaligned trap handler on ia64 (perhaps
coding it in raw assembler) to see what the situation looks like then?

Nobody wants to ever comment on this...

> This extra memcpy when the buffer is not unaligned currently only impacts 5701 

But it affects all platforms, not just ones that ia64 which have
the unaligned trap cost.  For example, we _DEFINITELY_ don't want
to do what your patch causes to happen on x86.

Re: Tigon3 5701 PCI-X recv performance problem

[Andi Kleen]

> The problem is that on the Altix platform they have to deal with unaligned 
> accesses via an exception handler.  This causes them to run through 
> hundreds of instructions.  There's evidntally another mechanism in the cpu 
> that's more efficient in daling with unaligned accesses, but we can't use 
> that for some reason (atomicity I think, but I don't recall).

Atomicity should not be needed to access a private skb.

Maybe you didn't want to change the core stack to use the unaligned
access mechanism?

In that case it may be better to fix the stack with some macro
that expands to the unaligned access on IA64 and a normal load 
on other architectures.

-Andi

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Wed, 8 Oct 2003 20:37:42 +0200
Andi Kleen <ak@suse.de> wrote:

> Maybe you didn't want to change the core stack to use the unaligned
> access mechanism?
> 
> In that case it may be better to fix the stack with some macro
> that expands to the unaligned access on IA64 and a normal load 
> on other architectures.

I have a very strong feeling that we'd really need both options to
arrive at an optimal implementation on all platforms.

Something that makes drivers copy packets, and something else that
traps packet header accesses.

But I don't like any of these solutions (and I know Linus will never
accept a set of changes that puts netdev_get_unaligned() macro usage
all over the entire networking layer).  Instead, we should do what
you proposed long ago.  Seperating the protocol headers from the
packet data.  Then we need only align the protocol headers.

In fact, I can suggest a very efficient implementation:

1) Driver allocates paged SKBs.  There is ~128 bytes of skb->data
   buffer area, and pages are chopped up into MTU'ish sized chunks
   and hung onto SKBs in the frag list.

2) The device is given the page buffers to receive packets into.

3) On RX, align skb->data (ie. skb_reserve(skb, 2) for ethernet
   drivers) and copy the first N bytes from the head of
   the paged buffer to skb->data, point the fraglist entry for the
   page at offset "N" into the page chunk buffer.

Problem solved.

Re: Tigon3 5701 PCI-X recv performance problem

[Andi Kleen]

> But I don't like any of these solutions (and I know Linus will never
> accept a set of changes that puts netdev_get_unaligned() macro usage
> all over the entire networking layer).  Instead, we should do what
> you proposed long ago.  Seperating the protocol headers from the
> packet data.  Then we need only align the protocol headers.

I agree that it would be the best solution, but isn't it a bit late
in 2.6 now for that? Sounds more like a great 2.7.0 project.

It's not that it's a new problem - we had this since the Alpha port
and it hasn't gotten more urgent suddenly.

For 2.6 short term probably some bandaid like the CONFIG_UNALIGNMENT_COSTLY 
and doing driver copies is better.

> 
> In fact, I can suggest a very efficient implementation:
> 
> 1) Driver allocates paged SKBs.  There is ~128 bytes of skb->data
>    buffer area, and pages are chopped up into MTU'ish sized chunks
>    and hung onto SKBs in the frag list.

Hmm - you mean it allocates a full page and does suballocation by itself?

The suballocation would need to be per CPU to be SMP efficient I guess,
which would complicate it.

Another drawback that the page allocators per CPU allocator is not as
strong as slabs, so it may perform not as good.

And it would require the skb driver accessible destructor that was resisted
against for so long to manage the suballocation lists ;-)

But with such a destructor you would actually not need to do own allocation,
because you could just convert the memory areas returned by slab to
(struct page *, offset) 

-Andi

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Wed, 8 Oct 2003 22:22:48 +0200
Andi Kleen <ak@suse.de> wrote:

> It's not that it's a new problem - we had this since the Alpha port
> and it hasn't gotten more urgent suddenly.

Frankly, I just want to shut all the ia64 users up because they keep
barking due to the kernel unaligned trap message that port spits out.

> Hmm - you mean it allocates a full page and does suballocation by itself?

We could write some helper routines.

> The suballocation would need to be per CPU to be SMP efficient I guess,
> which would complicate it.

Andi, stop right there, we're talking about mitigating the horrible
performance some $6.00 USD Taiwaneese network cards get on expensive
ia64 systems.  How much effort do you think we should drain into
optimizing this? :-)

Re: Tigon3 5701 PCI-X recv performance problem

[Andi Kleen]

> > The suballocation would need to be per CPU to be SMP efficient I guess,
> > which would complicate it.
> 
> Andi, stop right there, we're talking about mitigating the horrible
> performance some $6.00 USD Taiwaneese network cards get on expensive
> ia64 systems.  How much effort do you think we should drain into
> optimizing this? :-)

Well, this thread was about the tigon3 and I don't see that as an el cheapo
card. If SGI uses it on the Altix I guess they want it to perform well 
with many CPUs.

Anyways, it was just a brain dump regarding your idea. I personally
think it's better to just use slab to implement it, allocating pages
doesn't seem to have any advantages to me.

-Andi

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Wed, 8 Oct 2003 22:33:06 +0200
Andi Kleen <ak@suse.de> wrote:

> Well, this thread was about the tigon3 and I don't see that as an el cheapo
> card. If SGI uses it on the Altix I guess they want it to perform well 
> with many CPUs.

It's one of the oldest variants of the tg3 chip and it's full
of hardware bugs when used in PCI-X.

> I personally think it's better to just use slab to implement it,
> allocating pages doesn't seem to have any advantages to me.

The page chunk allocator is meant to make it easier to put the
non-header parts in the frag list of the SKB, see?  It means we
don't need to do anything special in the networking, all the
receive paths handle frag'd RX packets properly.

We can't take pages SLAB is using and attach them to the fraglist
of the SKB, kfree_skb() is going to try and put_page() on them.

If we used slab, we'd need to do something like:

1) skb->h.raw or whatever have special meaning and point to
   different buffers.

2) There's a skb->data2 or something like that.

Both are a lot of work compared to my suggestion.

Re: Tigon3 5701 PCI-X recv performance problem

[Andi Kleen]

On Wed, Oct 08, 2003 at 01:32:48PM -0700, David S. Miller wrote:
> On Wed, 8 Oct 2003 22:33:06 +0200
> Andi Kleen <ak@suse.de> wrote:
> 
> > Well, this thread was about the tigon3 and I don't see that as an el cheapo
> > card. If SGI uses it on the Altix I guess they want it to perform well 
> > with many CPUs.
> 
> It's one of the oldest variants of the tg3 chip and it's full
> of hardware bugs when used in PCI-X.

Ok. Why do we care about it then?  Copying should be fine for that.

> 
> > I personally think it's better to just use slab to implement it,
> > allocating pages doesn't seem to have any advantages to me.
> 
> The page chunk allocator is meant to make it easier to put the
> non-header parts in the frag list of the SKB, see?  It means we
> don't need to do anything special in the networking, all the
> receive paths handle frag'd RX packets properly.

Sure, but to handle the sub allocation you need a destructor per fragment.
(otherwise how do you want to share a page between different packets)

And when you have a destructor you can as well allocate from slab
and convert the virtual pointer to (struct page *, offset)

If you don't want to share pages between different packets: I don't
like this because it would increase memory use of networking with 1.5k
MTU by factor 2. I don't think that would be a good path to go
down, Linux networking is already too bloated.

BTW I think this all should be also ifdefed with CONFIG_SLOW_UNALIGNMENT.
I certainly don't want any of this on x86-64 where unalignment cost
one cycle only.

Another BTW Actually this technique would allow socket buffers in highmem, but
it's not really needed yet and all 32bit architecture which have high
mem usually have fast unalignment handling. 

-Andi

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Wed, 8 Oct 2003 22:46:18 +0200
Andi Kleen <ak@suse.de> wrote:

> On Wed, Oct 08, 2003 at 01:32:48PM -0700, David S. Miller wrote:
> > The page chunk allocator is meant to make it easier to put the
> > non-header parts in the frag list of the SKB, see?  It means we
> > don't need to do anything special in the networking, all the
> > receive paths handle frag'd RX packets properly.
> 
> Sure, but to handle the sub allocation you need a destructor per fragment.
> (otherwise how do you want to share a page between different packets)

Aha, no you don't, this is the beauty of it.

Let's say we've packed 4 packets into a page (or 10 in 2 pages,
whatever the optimal packing is), as you attach each chunk to a SKB
you up the page count (if the buffer straddles 2 or more pages you use
one frag entry for each of those pages and bump the count as
approprise).  As far as the networking is concerned, it's some page
cache page or whatever, it doesn't care.

Then kfree_skb(skb) just does the right thing by putting all the
pages, when the page count goes to zero it's free'd up.

> BTW I think this all should be also ifdefed with CONFIG_SLOW_UNALIGNMENT.
> I certainly don't want any of this on x86-64 where unalignment cost
> one cycle only.

I agree, I don't even want this rediculious crap on sparc64 where
I can make the unaligned trap handler 30 or 40 cycles or even less.

BTW, your highmem example is interesting, but even more interesting are
the cards that do the magic multiple-TCP-packet coalescing so that the
data parts are all page aligned.  They want infrastructure like this.

Re: Tigon3 5701 PCI-X recv performance problem

[Steve Modica]

Andi Kleen wrote:

> 
> For 2.6 short term probably some bandaid like the CONFIG_UNALIGNMENT_COSTLY 
> and doing driver copies is better.
> 

Question.

Is there a way for us to create a CONFIG entry like this, but also allow for a 
modules.conf argument?  (perhaps "options tg3 unalignment_costly")

Customers running Suse on Altix have to edit modules.conf anyhow so they use the 
tg3 module (instead of bcm).  So when they do this, we could easily have them 
add an argument to the modules.conf line and we avoid a kernel build. This let's 
us run with a standard ia64 kernel.

Steve

-- 
Steve Modica
work: 651-683-3224
MTS-Technical Lead
"Give a man a fish, and he will eat for a day, hit him with a fish and
he leaves you alone" - me

Re: Tigon3 5701 PCI-X recv performance problem

[Andi Kleen]

> Customers running Suse on Altix have to edit modules.conf anyhow so they 
> use the tg3 module (instead of bcm).  So when they do this, we could easily 
> have them add an argument to the modules.conf line and we avoid a kernel 
> build. This let's us run with a standard ia64 kernel.

Doesn't it already have a rx_copy_break argument? If yes just set it to
zero and it will always copy. If not add it.

-Andi

Re: Tigon3 5701 PCI-X recv performance problem

[Steve Modica]

Andi Kleen wrote:

>>Customers running Suse on Altix have to edit modules.conf anyhow so they 
>>use the tg3 module (instead of bcm).  So when they do this, we could easily 
>>have them add an argument to the modules.conf line and we avoid a kernel 
>>build. This let's us run with a standard ia64 kernel.
> 
> 
> Doesn't it already have a rx_copy_break argument? If yes just set it to
> zero and it will always copy. If not add it.

The problem with this solution is that it also impacts 5704 cards, which 
don't have the alignment issue. We are figuring all this out.  We may 
just switch over completely and replace the 5701 cards.

Re: Tigon3 5701 PCI-X recv performance problem

[Andi Kleen]

On Sat, Oct 11, 2003 at 08:17:26AM -0500, Steve Modica wrote:
> Andi Kleen wrote:
> 
> >>Customers running Suse on Altix have to edit modules.conf anyhow so they 
> >>use the tg3 module (instead of bcm).  So when they do this, we could 
> >>easily have them add an argument to the modules.conf line and we avoid a 
> >>kernel build. This let's us run with a standard ia64 kernel.
> >
> >
> >Doesn't it already have a rx_copy_break argument? If yes just set it to
> >zero and it will always copy. If not add it.
> 
> The problem with this solution is that it also impacts 5704 cards, which 
> don't have the alignment issue. We are figuring all this out.  We may 
> just switch over completely and replace the 5701 cards.

Then just don't set the argument to zero in modules.conf on the 5704cards. 
That could be even handled somewhere in user space.  Switching over sounds 
best of course, because the performance with the copying won't be great

-Andi

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Sat, 11 Oct 2003 15:19:21 +0200
Andi Kleen <ak@suse.de> wrote:

> Then just don't set the argument to zero in modules.conf on the 5704cards. 
> That could be even handled somewhere in user space.  Switching over sounds 
> best of course, because the performance with the copying won't be great

There is no "argument" we don't have a variable in the tg3 driver
that determines the "copybreak", it's a hard-coded macro define.

For them it's really easy to know when the 5701 hw bug case is present,
so they can do something like:

	if (len > RX_COPY_THRESHOLD
#ifdef CONFIG_UNALIGNED_EXPENSIVE
	    || tp->rx_offset == 2
#endif
	    ) {
        ...

And then only their platform and only when the buggy chips are present,
will get the desired change in behavior.

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

Could we create a module argument like "do_5701_align" and just give it a -1
default which does NOT do the copy but if we give the arg 1, 0, whatever it
DOES to the copy to align the buffs ?

Then inside the driver we define a macro to handle that ?

That way all you need is to include "tg3 do_5701_align=1" in modules.conf

That way all the default comiles work and no CONFIG option for people
who compile all defaults and don't want to re compile the kernel for
this issue.

Is that acceptable ?

John

David S. Miller wrote:
> On Sat, 11 Oct 2003 15:19:21 +0200
> Andi Kleen <ak@suse.de> wrote:
> 
> 
>>Then just don't set the argument to zero in modules.conf on the 5704cards. 
>>That could be even handled somewhere in user space.  Switching over sounds 
>>best of course, because the performance with the copying won't be great
> 
> 
> There is no "argument" we don't have a variable in the tg3 driver
> that determines the "copybreak", it's a hard-coded macro define.
> 
> For them it's really easy to know when the 5701 hw bug case is present,
> so they can do something like:
> 
> 	if (len > RX_COPY_THRESHOLD
> #ifdef CONFIG_UNALIGNED_EXPENSIVE
> 	    || tp->rx_offset == 2
> #endif
> 	    ) {
>         ...
> 
> And then only their platform and only when the buggy chips are present,
> will get the desired change in behavior.

-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Mon, 13 Oct 2003 14:53:54 -0500
John Partridge <johnip@sgi.com> wrote:

> Could we create a module argument like "do_5701_align" and just give it a -1
> default which does NOT do the copy but if we give the arg 1, 0, whatever it
> DOES to the copy to align the buffs ?

Absolutely not, we're not going to put super-hacky module parameters
into the driver just because you guys don't want to create and use the
CONFIG_UNALIGNED_EXPENSIVE config option needed to fix this properly.

> That way all the default comiles work and no CONFIG option for people
> who compile all defaults and don't want to re compile the kernel for
> this issue.
> 
> Is that acceptable ?

No it's not.

When you add this new config option, and set it on ia64, the only
thing that will need to be rebuilt is the tg3 driver, no other thing
in the entire kernel tree should trigger a dependency on this new
config option.

I don't understand why at all you guys are trying to avoid fixing this
the proper way, you will meet only resistence from me if you continue
doing that :)

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

The key point for me is, as David said yesterday, SGI users will only
have to recompile the tg3 driver with the CONFIG defined to get the desired
behavior, not hte whole kernel. They just use the recompiled driver with
whatever kernel they are using. Users who use SGI propack will already
have the (static) driver with the CONFIG defined by SGI, SuSE users
who use a tg3.o use it as a module anyway, so it's not a big deal
just to recompile tg3. The only problem I can see is if users use RedHat
Advanced Server, I believe the tg3 driver is static in the kernel so they
won't be able to just recompile the tg3 driver.

I do propose that we make the CONFIG a little more descriptive something like

  CONFIG_5701PCIX_UNALIGNED_EXPENSIVE

just to make it easier for users to see what it does.

John

David S. Miller wrote:
> On Sat, 11 Oct 2003 15:19:21 +0200
> Andi Kleen <ak@suse.de> wrote:
> 
> 
>>Then just don't set the argument to zero in modules.conf on the 5704cards. 
>>That could be even handled somewhere in user space.  Switching over sounds 
>>best of course, because the performance with the copying won't be great
> 
> 
> There is no "argument" we don't have a variable in the tg3 driver
> that determines the "copybreak", it's a hard-coded macro define.
> 
> For them it's really easy to know when the 5701 hw bug case is present,
> so they can do something like:
> 
> 	if (len > RX_COPY_THRESHOLD
> #ifdef CONFIG_UNALIGNED_EXPENSIVE
> 	    || tp->rx_offset == 2
> #endif
> 	    ) {
>         ...
> 
> And then only their platform and only when the buggy chips are present,
> will get the desired change in behavior.
> 

-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Tue, 14 Oct 2003 11:49:14 -0500
John Partridge <johnip@sgi.com> wrote:

> I do propose that we make the CONFIG a little more descriptive something like
> 
>   CONFIG_5701PCIX_UNALIGNED_EXPENSIVE
> 
> just to make it easier for users to see what it does.

That's grosser than gross.

We're trying to create a generic mechanism to fix this kind of
problem in _ALL_ drivers.  I will heavily thwart any attempt to
make this some kind of "tg3 thing".

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

I'm working on a patch for the tg3 driver for 2.6

Please can you look at this. I'm not too sure about the Kconfig entry as I have not done one before :-

--- linux/drivers/net/tg3.c     2003-11-10 18:28:10.000000000 -0600
+++ patch/drivers/net/tg3.c     2003-11-10 18:58:35.000000000 -0600
@@ -2257,7 +2257,11 @@

                 len = ((desc->idx_len & RXD_LEN_MASK) >> RXD_LEN_SHIFT) - 4; /* omit crc */

-               if (len > RX_COPY_THRESHOLD) {
+               if (len > RX_COPY_THRESHOLD
+#if defined(CONFIG_UNALIGNED_EXPENSIVE)
+                        && tp->rx_offset == 2
+#endif
+               ) {
                         int skb_size;

                         skb_size = tg3_alloc_rx_skb(tp, opaque_key,
--- linux/drivers/net/Kconfig   2003-10-25 13:44:36.000000000 -0500
+++ patch/drivers/net/Kconfig   2003-11-10 19:21:15.000000000 -0600
@@ -2017,6 +2017,9 @@
           To compile this driver as a module, choose M here: the module
           will be called tg3.  This is recommended.

+config CONFIG_UNALIGNED_EXPENSIVE
+       bool "Use Aligned SKB's for 5701 cards (for Itanium2 based systems)"
+       depends on TIGON3 && IA64
  endmenu

  #


Thks
John

-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Mon, 10 Nov 2003 19:24:38 -0600
John Partridge <johnip@sgi.com> wrote:

> Please can you look at this. I'm not too sure about the Kconfig entry as I have not done one before :-

It belongs in arch/${ARCH}/Kconfig not drivers/net/Kconfig

It's a static property of the architecture, not something the
user chooses one way or the other.

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

Is this OK ? I was not sure about the order should depends be beofre default or
the other way round ? or does it matter ? I suspect it does.

Thks
John


--- linux/drivers/net/tg3.c     2003-11-10 18:28:10.000000000 -0600
+++ patch/drivers/net/tg3.c     2003-11-10 18:58:35.000000000 -0600
@@ -2257,7 +2257,11 @@

                 len = ((desc->idx_len & RXD_LEN_MASK) >> RXD_LEN_SHIFT) - 4; /*                                                        omit crc */

-               if (len > RX_COPY_THRESHOLD) {
+               if (len > RX_COPY_THRESHOLD
+#if defined(CONFIG_UNALIGNED_EXPENSIVE)
+                        && tp->rx_offset == 2
+#endif
+               ) {
                         int skb_size;

                         skb_size = tg3_alloc_rx_skb(tp, opaque_key,
--- linux/arch/ia64/Kconfig     2003-10-25 13:44:46.000000000 -0500
+++ patch/arch/ia64/Kconfig     2003-11-11 14:01:42.000000000 -0600
@@ -468,6 +468,11 @@
           agent" (/sbin/hotplug) to load modules and set up software needed
           to use devices as you hotplug them.

+config CONFIG_UNALIGNED_EXPENSIVE
+       bool "Use Aligned SKB's for Tigon3 5701 network adapters"
+       depends on MCKINLEY
+       default y
+
  source "drivers/pci/hotplug/Kconfig"

  source "drivers/pcmcia/Kconfig"


-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Tue, 11 Nov 2003 14:04:50 -0600
John Partridge <johnip@sgi.com> wrote:

> Is this OK ? I was not sure about the order should depends be beofre default or
> the other way round ? or does it matter ? I suspect it does.

Why are you depending upon MCKINLEY?  Don't all ia64 cpus
give traps for unaligned memory accesses?

That is what this CONFIG option tells the whole kernel.

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

David S. Miller wrote:
> On Tue, 11 Nov 2003 14:04:50 -0600
> Why are you depending upon MCKINLEY?  Don't all ia64 cpus
> give traps for unaligned memory accesses?

I have no evidence of a problem with Itanium1, only on Itanium2 (MCKINLEY)
I have seen it on HP and SGI MCKINLEY

-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com

Re: Tigon3 5701 PCI-X recv performance problem

[Jeff Garzik]

John Partridge wrote:
> David S. Miller wrote:
> 
>> On Tue, 11 Nov 2003 14:04:50 -0600
>> Why are you depending upon MCKINLEY?  Don't all ia64 cpus
>> give traps for unaligned memory accesses?
> 
> 
> I have no evidence of a problem with Itanium1, only on Itanium2 (MCKINLEY)
> I have seen it on HP and SGI MCKINLEY


Ask the other David M about eepro100.c patches he wanted to send to me...

	Jeff

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

Jeff,

I have not seen the change for this go into 2.6 is there something else I should be
doing ?

Thanks
John

Jeff Garzik wrote:
> John Partridge wrote:
> 
>> David S. Miller wrote:
>>
>>> On Tue, 11 Nov 2003 14:04:50 -0600
>>> Why are you depending upon MCKINLEY?  Don't all ia64 cpus
>>> give traps for unaligned memory accesses?
>>
>>
>>
>> I have no evidence of a problem with Itanium1, only on Itanium2 
>> (MCKINLEY)
>> I have seen it on HP and SGI MCKINLEY
> 
> 
> 
> Ask the other David M about eepro100.c patches he wanted to send to me...
> 
>     Jeff
> 
> 
> 

-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

David S. Miller wrote:
> Why are you depending upon MCKINLEY?  Don't all ia64 cpus
> give traps for unaligned memory accesses?
> 
> That is what this CONFIG option tells the whole kernel.

OK, I made it ALL IA64, but I only know for sure about Itanium2's

--- linux/drivers/net/tg3.c     2003-11-10 18:28:10.000000000 -0600
+++ patch/drivers/net/tg3.c     2003-11-10 18:58:35.000000000 -0600
@@ -2257,7 +2257,11 @@

                 len = ((desc->idx_len & RXD_LEN_MASK) >> RXD_LEN_SHIFT) - 4; /* omit crc */

-               if (len > RX_COPY_THRESHOLD) {
+               if (len > RX_COPY_THRESHOLD
+#if defined(CONFIG_UNALIGNED_EXPENSIVE)
+                        && tp->rx_offset == 2
+#endif
+               ) {
                         int skb_size;

                         skb_size = tg3_alloc_rx_skb(tp, opaque_key,
--- linux/arch/ia64/Kconfig     2003-10-25 13:44:46.000000000 -0500
+++ patch/arch/ia64/Kconfig     2003-11-11 15:37:58.000000000 -0600
@@ -468,6 +468,11 @@
           agent" (/sbin/hotplug) to load modules and set up software needed
           to use devices as you hotplug them.

+config CONFIG_UNALIGNED_EXPENSIVE
+       bool "Use kernel aligned buffers"
+       depends on IA64
+       default y
+
  source "drivers/pci/hotplug/Kconfig"

  source "drivers/pcmcia/Kconfig"

-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Tue, 11 Nov 2003 15:39:49 -0600
John Partridge <johnip@sgi.com> wrote:

> OK, I made it ALL IA64, but I only know for sure about Itanium2's

This patch looks a lot better.

Jeff, perhaps you'll combine this with the thing David Mosberger wanted
and then merge?

Re: Tigon3 5701 PCI-X recv performance problem

[Andi Kleen]

> +config CONFIG_UNALIGNED_EXPENSIVE
> +       bool "Use kernel aligned buffers"

Drop the "bool" line. Then it will be invisible, which is the right thing here.
     
> +       depends on IA64
> +       default y
> +

-Andi

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

Thanks Andi,

Andi Kleen wrote:
> Drop the "bool" line. Then it will be invisible, which is the right thing here.


--- linux/drivers/net/tg3.c     2003-11-10 18:28:10.000000000 -0600
+++ patch/drivers/net/tg3.c     2003-11-10 18:58:35.000000000 -0600
@@ -2257,7 +2257,11 @@

                 len = ((desc->idx_len & RXD_LEN_MASK) >> RXD_LEN_SHIFT) - 4; /* omit crc */

-               if (len > RX_COPY_THRESHOLD) {
+               if (len > RX_COPY_THRESHOLD
+#if defined(CONFIG_UNALIGNED_EXPENSIVE)
+                        && tp->rx_offset == 2
+#endif
+               ) {
                         int skb_size;

                         skb_size = tg3_alloc_rx_skb(tp, opaque_key,
--- linux/arch/ia64/Kconfig     2003-10-25 13:44:46.000000000 -0500
+++ patch/arch/ia64/Kconfig     2003-11-11 19:45:06.000000000 -0600
@@ -468,6 +468,10 @@
           agent" (/sbin/hotplug) to load modules and set up software needed
           to use devices as you hotplug them.

+config CONFIG_UNALIGNED_EXPENSIVE
+       depends on IA64
+       default y
+
  source "drivers/pci/hotplug/Kconfig"

  source "drivers/pcmcia/Kconfig"


-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com

Re: Tigon3 5701 PCI-X recv performance problem

[Andi Kleen]

> --- linux/arch/ia64/Kconfig     2003-10-25 13:44:46.000000000 -0500
> +++ patch/arch/ia64/Kconfig     2003-11-11 19:45:06.000000000 -0600
> @@ -468,6 +468,10 @@
>           agent" (/sbin/hotplug) to load modules and set up software needed
>           to use devices as you hotplug them.
> 
> +config CONFIG_UNALIGNED_EXPENSIVE

Sorry to nitpick again and not catching it earlier, but are you sure it works 
this way? Normally the options in Kconfig are not prefixed with CONFIG_* 

So 

config UNALIGNED_EXPENSIVE 

would be likely correct

-Andi

> +       depends on IA64
> +       default y
> +
>  source "drivers/pci/hotplug/Kconfig"
> 
>  source "drivers/pcmcia/Kconfig"

Re: Tigon3 5701 PCI-X recv performance problem

[Jeff Garzik]

Andi Kleen wrote:
>>--- linux/arch/ia64/Kconfig     2003-10-25 13:44:46.000000000 -0500
>>+++ patch/arch/ia64/Kconfig     2003-11-11 19:45:06.000000000 -0600
>>@@ -468,6 +468,10 @@
>>          agent" (/sbin/hotplug) to load modules and set up software needed
>>          to use devices as you hotplug them.
>>
>>+config CONFIG_UNALIGNED_EXPENSIVE
> 
> 
> Sorry to nitpick again and not catching it earlier, but are you sure it works 
> this way? Normally the options in Kconfig are not prefixed with CONFIG_* 
> 
> So 
> 
> config UNALIGNED_EXPENSIVE 
> 
> would be likely correct


indeed.

For my part, I will apply the patch to drivers/net/tg3.c, which is fine, 
and will let the arch/ia64/Kconfig patch go through normal channels 
(since ia64 seems to be flowing these days).

	Jeff

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

Jeff Garzik wrote:
> Andi Kleen wrote:
>> Sorry to nitpick again and not catching it earlier, but are you sure 
>> it works this way? Normally the options in Kconfig are not prefixed 
>> with CONFIG_*
>> So
>> config UNALIGNED_EXPENSIVE
>> would be likely correct

Yes, that looks right to me. I'm still learning the whole 2.6 Kconfig thing

> 
> 
> 
> indeed.
> 
> For my part, I will apply the patch to drivers/net/tg3.c, which is fine, 
> and will let the arch/ia64/Kconfig patch go through normal channels 
> (since ia64 seems to be flowing these days).
> 
>     Jeff
> 
> 

Is this OK ?

--- linux/drivers/net/tg3.c     2003-11-10 18:28:10.000000000 -0600
+++ patch/drivers/net/tg3.c     2003-11-10 18:58:35.000000000 -0600
@@ -2257,7 +2257,11 @@

                 len = ((desc->idx_len & RXD_LEN_MASK) >> RXD_LEN_SHIFT) - 4; /* omit crc */

-               if (len > RX_COPY_THRESHOLD) {
+               if (len > RX_COPY_THRESHOLD
+#if defined(CONFIG_UNALIGNED_EXPENSIVE)
+                        && tp->rx_offset == 2
+#endif
+               ) {
                         int skb_size;

                         skb_size = tg3_alloc_rx_skb(tp, opaque_key,
--- linux/arch/ia64/Kconfig     2003-10-25 13:44:46.000000000 -0500
+++ patch/arch/ia64/Kconfig     2003-11-12 09:31:14.000000000 -0600
@@ -468,6 +468,10 @@
           agent" (/sbin/hotplug) to load modules and set up software needed
           to use devices as you hotplug them.

+config UNALIGNED_EXPENSIVE
+       depends on IA64
+       default y
+
  source "drivers/pci/hotplug/Kconfig"

  source "drivers/pcmcia/Kconfig"



-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com

Re: Tigon3 5701 PCI-X recv performance problem

[Christoph Hellwig]

> --- linux/arch/ia64/Kconfig     2003-10-25 13:44:46.000000000 -0500
> +++ patch/arch/ia64/Kconfig     2003-11-12 09:31:14.000000000 -0600
> @@ -468,6 +468,10 @@
>            agent" (/sbin/hotplug) to load modules and set up software needed
>            to use devices as you hotplug them.
> 
> +config UNALIGNED_EXPENSIVE

> +       depends on IA64

this line is not needed - if you're in the ia64 Kconfig it's always set.

Re: Tigon3 5701 PCI-X recv performance problem

[David S. Miller]

On Tue, 11 Nov 2003 14:04:50 -0600
John Partridge <johnip@sgi.com> wrote:

> +config CONFIG_UNALIGNED_EXPENSIVE
> +       bool "Use Aligned SKB's for Tigon3 5701 network adapters"
> +       depends on MCKINLEY
> +       default y
> +

Also, this help text is broken too.

I repeat: "THIS IS NOT A TIGON3 5701 NETWORK ADAPTER SPECIFIC HACK"

It is a config variable that all network drivers in the kernel can
test to make copybreak and related decisions for copy vs. read
directly packet handling.

Please stop making this a tigon3 specific thing.

Re: Tigon3 5701 PCI-X recv performance problem

[Jeff Garzik]

David S. Miller wrote:
> On Tue, 11 Nov 2003 14:04:50 -0600
> John Partridge <johnip@sgi.com> wrote:
> 
> 
>>+config CONFIG_UNALIGNED_EXPENSIVE
>>+       bool "Use Aligned SKB's for Tigon3 5701 network adapters"
>>+       depends on MCKINLEY
>>+       default y
>>+
> 
> 
> Also, this help text is broken too.
> 
> I repeat: "THIS IS NOT A TIGON3 5701 NETWORK ADAPTER SPECIFIC HACK"
> 
> It is a config variable that all network drivers in the kernel can
> test to make copybreak and related decisions for copy vs. read
> directly packet handling.
> 
> Please stop making this a tigon3 specific thing.


We should CC David Mosberger on this, too.  He's been wanting something 
like this (for, as you say, all network drivers) for a while now.

	Jeff

Re: Tigon3 5701 PCI-X recv performance problem

[John Partridge]

Andi Kleen wrote:
> Then just don't set the argument to zero in modules.conf on the 5704cards. 
> That could be even handled somewhere in user space.  Switching over sounds 
> best of course, because the performance with the copying won't be great

The problem with this is if you mix 5704 and 5701 cards in the same system
you are not seting it for the 5701's that need it.

John

-- 
John Partridge

Silicon Graphics Inc
Tel:  651-683-3428
Vnet: 233-3428
E-Mail: johnip@sgi.com