b44: regression in 2.6.22

b44: regression in 2.6.22

[Stephen Hemminger]

Something is broken with the b44 driver in 2.6.22-rc1 or later. Now bisecting.
The performance (with iperf) for receiving is normally 94Mbits or more.
But something happened that dropped performance to less than 1Mbit,
probably corrupted packets.

There is nothing obvious in the commit log for drivers/net/b44.c, so it
probably is something more general.


Looking at the code in b44_rx(), I see a couple unrelated of bugs:
1. In the small packet case it recycles the skb before copying data out... 
   Not good if new data arrives overwriting existing data.

2. Macros like RX_PKT_BUF_SZ that depend on local variables are evil!!



-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: b44: regression in 2.6.22

[Gary Zambrano]

On Fri, 2007-05-25 at 17:24 -0700, Stephen Hemminger wrote:
> Something is broken with the b44 driver in 2.6.22-rc1 or later. Now bisecting.
> The performance (with iperf) for receiving is normally 94Mbits or more.
> But something happened that dropped performance to less than 1Mbit,
> probably corrupted packets.
> 
> There is nothing obvious in the commit log for drivers/net/b44.c, so it
> probably is something more general.

netperf in 2.6.22-rc2 seems ok. I'll give iperf a try to see if I can
repro the problem.

Thanks,
Gary

Re: b44: regression in 2.6.22

[Michael Buesch]

On Saturday 26 May 2007 02:24:31 Stephen Hemminger wrote:
> Something is broken with the b44 driver in 2.6.22-rc1 or later. Now bisecting.
> The performance (with iperf) for receiving is normally 94Mbits or more.
> But something happened that dropped performance to less than 1Mbit,
> probably corrupted packets.
> 
> There is nothing obvious in the commit log for drivers/net/b44.c, so it
> probably is something more general.
> 
> 
> Looking at the code in b44_rx(), I see a couple unrelated of bugs:
> 1. In the small packet case it recycles the skb before copying data out... 
>    Not good if new data arrives overwriting existing data.
> 
> 2. Macros like RX_PKT_BUF_SZ that depend on local variables are evil!!

Very interesting!
2.6.22 doesn't include ssb, does it?

Adding CCs to make reporters of another bugreport aware of this.

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 5413 bytes --]

I send this again because my first mail accidently had html code in it and 
might have been filtered by some people.

On Saturday 26 May 2007, Michael Buesch wrote:
> On Saturday 26 May 2007 02:24:31 Stephen Hemminger wrote:
> > Something is broken with the b44 driver in 2.6.22-rc1 or later. Now
> > bisecting. The performance (with iperf) for receiving is normally 94Mbits
> > or more. But something happened that dropped performance to less than
> > 1Mbit, probably corrupted packets.
> >
> > There is nothing obvious in the commit log for drivers/net/b44.c, so it
> > probably is something more general.
> >
> >
> > Looking at the code in b44_rx(), I see a couple unrelated of bugs:
> > 1. In the small packet case it recycles the skb before copying data
> > out... Not good if new data arrives overwriting existing data.
> >
> > 2. Macros like RX_PKT_BUF_SZ that depend on local variables are evil!!
>
> Very interesting!
> 2.6.22 doesn't include ssb, does it?
>
> Adding CCs to make reporters of another bugreport aware of this.

I did some more tests with my BCM4401 and different kernels, here are the 
results:

2.6.21.1:

iperf:
[  5] local 192.168.1.2 port 58414 connected with 192.168.1.1 port 5001
[  5]  0.0-60.6 sec  1.13 MBytes    157 Kbits/sec
[  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 57837
[  4]  0.0-63.1 sec  2.82 MBytes    375 Kbits/sec

koala:~# ping -c10 192.168.1.1
PING 192.168.1.1 (192.168.1.1) 56(84) bytes of data.
64 bytes from 192.168.1.1: icmp_seq=1 ttl=64 time=0.241 ms
64 bytes from 192.168.1.1: icmp_seq=2 ttl=64 time=0.215 ms
64 bytes from 192.168.1.1: icmp_seq=3 ttl=64 time=0.230 ms
64 bytes from 192.168.1.1: icmp_seq=4 ttl=64 time=0.238 ms
64 bytes from 192.168.1.1: icmp_seq=5 ttl=64 time=0.229 ms
64 bytes from 192.168.1.1: icmp_seq=6 ttl=64 time=0.228 ms
64 bytes from 192.168.1.1: icmp_seq=7 ttl=64 time=0.231 ms
64 bytes from 192.168.1.1: icmp_seq=8 ttl=64 time=0.229 ms
64 bytes from 192.168.1.1: icmp_seq=9 ttl=64 time=0.228 ms
64 bytes from 192.168.1.1: icmp_seq=10 ttl=64 time=0.237 ms

--- 192.168.1.1 ping statistics ---
10 packets transmitted, 10 received, 0% packet loss, time 8998ms
rtt min/avg/max/mdev = 0.215/0.230/0.241/0.018 ms

The system was unusable while i ran the iperf test, when I moved the mouse it 
was only jumping around and doing anything like starting programs or 
switching the desktop first happend after iperf had finished it's test.

I did a http downlaod with wget and got 11.23M/s.


2.6.22-rc3:

[  5] local 192.168.1.2 port 46557 connected with 192.168.1.1 port 5001
[  5]  0.0-60.4 sec  58.9 MBytes  8.18 Mbits/sec
[  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 51633
[  4]  0.0-63.1 sec  7.27 MBytes    967 Kbits/sec

koala:~# ping -c10 192.168.1.1
PING 192.168.1.1 (192.168.1.1) 56(84) bytes of data.
64 bytes from 192.168.1.1: icmp_seq=1 ttl=64 time=0.243 ms
64 bytes from 192.168.1.1: icmp_seq=2 ttl=64 time=0.234 ms
64 bytes from 192.168.1.1: icmp_seq=3 ttl=64 time=0.238 ms
64 bytes from 192.168.1.1: icmp_seq=4 ttl=64 time=0.235 ms
64 bytes from 192.168.1.1: icmp_seq=5 ttl=64 time=0.230 ms
64 bytes from 192.168.1.1: icmp_seq=6 ttl=64 time=0.317 ms
64 bytes from 192.168.1.1: icmp_seq=7 ttl=64 time=0.232 ms
64 bytes from 192.168.1.1: icmp_seq=8 ttl=64 time=0.232 ms
64 bytes from 192.168.1.1: icmp_seq=9 ttl=64 time=0.228 ms
64 bytes from 192.168.1.1: icmp_seq=10 ttl=64 time=0.238 ms

--- 192.168.1.1 ping statistics ---
10 packets transmitted, 10 received, 0% packet loss, time 8997ms
rtt min/avg/max/mdev = 0.228/0.242/0.317/0.031 ms

System responsiveness was the same as with 2.6.21.1.

wget got 11.23M/s, again same as 2.6.21.1.


2.6.22-rc2-mm1:

[  5] local 192.168.1.2 port 42198 connected with 192.168.1.1 port 5001
[  5]  0.0-60.1 sec    402 MBytes  56.1 Mbits/sec
[  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 48598
[  4]  0.0-63.0 sec    177 MBytes  23.6 Mbits/sec

koala:~# ping -c10 192.168.1.1
PING 192.168.1.1 (192.168.1.1) 56(84) bytes of data.
64 bytes from 192.168.1.1: icmp_seq=1 ttl=64 time=39.8 ms
64 bytes from 192.168.1.1: icmp_seq=2 ttl=64 time=52.7 ms
64 bytes from 192.168.1.1: icmp_seq=3 ttl=64 time=86.7 ms
64 bytes from 192.168.1.1: icmp_seq=4 ttl=64 time=8.22 ms
64 bytes from 192.168.1.1: icmp_seq=5 ttl=64 time=32.1 ms
64 bytes from 192.168.1.1: icmp_seq=6 ttl=64 time=56.0 ms
64 bytes from 192.168.1.1: icmp_seq=7 ttl=64 time=80.0 ms
64 bytes from 192.168.1.1: icmp_seq=8 ttl=64 time=1.52 ms
64 bytes from 192.168.1.1: icmp_seq=9 ttl=64 time=25.4 ms
64 bytes from 192.168.1.1: icmp_seq=10 ttl=64 time=49.3 ms

--- 192.168.1.1 ping statistics ---
10 packets transmitted, 10 received, 0% packet loss, time 9000ms
rtt min/avg/max/mdev = 1.526/43.207/86.700/26.369 ms

Here system responsiveness was ok whil I ran iperf, I didn't notic anything 
anomalous.

When I tried the wget http download the tranfer did stall and from this point 
on I couldn't send or receive anything on my BCM4401 anymore. Taken the 
interface down and up again didn't help anything. I wonder if this is Uwe's 
problem

on all the kernels there apperaded nothing special in dmesg.

for the iperf test I connect my BCM4401 directly with an e100. The system with 
the e100 was iperf server and ran fine all over the test.

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

On Sunday 27 May 2007 21:25:17 Maximilian Engelhardt wrote:
> 2.6.22-rc3:
> 
> [  5] local 192.168.1.2 port 46557 connected with 192.168.1.1 port 5001
> [  5]  0.0-60.4 sec  58.9 MBytes  8.18 Mbits/sec
> [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 51633
> [  4]  0.0-63.1 sec  7.27 MBytes    967 Kbits/sec

Why do we have two different measurements here? Is one TX and one RX?
Which one?

> koala:~# ping -c10 192.168.1.1
> PING 192.168.1.1 (192.168.1.1) 56(84) bytes of data.
> 64 bytes from 192.168.1.1: icmp_seq=1 ttl=64 time=0.243 ms
> 64 bytes from 192.168.1.1: icmp_seq=2 ttl=64 time=0.234 ms
> 64 bytes from 192.168.1.1: icmp_seq=3 ttl=64 time=0.238 ms
> 64 bytes from 192.168.1.1: icmp_seq=4 ttl=64 time=0.235 ms
> 64 bytes from 192.168.1.1: icmp_seq=5 ttl=64 time=0.230 ms
> 64 bytes from 192.168.1.1: icmp_seq=6 ttl=64 time=0.317 ms
> 64 bytes from 192.168.1.1: icmp_seq=7 ttl=64 time=0.232 ms
> 64 bytes from 192.168.1.1: icmp_seq=8 ttl=64 time=0.232 ms
> 64 bytes from 192.168.1.1: icmp_seq=9 ttl=64 time=0.228 ms
> 64 bytes from 192.168.1.1: icmp_seq=10 ttl=64 time=0.238 ms
> 
> --- 192.168.1.1 ping statistics ---
> 10 packets transmitted, 10 received, 0% packet loss, time 8997ms
> rtt min/avg/max/mdev = 0.228/0.242/0.317/0.031 ms
> 
> System responsiveness was the same as with 2.6.21.1.
> 
> wget got 11.23M/s, again same as 2.6.21.1.
> 
> 
> 2.6.22-rc2-mm1:
> 
> [  5] local 192.168.1.2 port 42198 connected with 192.168.1.1 port 5001
> [  5]  0.0-60.1 sec    402 MBytes  56.1 Mbits/sec
> [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 48598
> [  4]  0.0-63.0 sec    177 MBytes  23.6 Mbits/sec

So with -mm (with ssb) you actually get better performace
then with plain 2.6.22-rc3?

Can you elaborate a bit more about what you get an what you expect
on which kernel?

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 3265 bytes --]

On Sunday 27 May 2007, Michael Buesch wrote:
> On Sunday 27 May 2007 21:25:17 Maximilian Engelhardt wrote:
> > 2.6.22-rc3:
> >
> > [  5] local 192.168.1.2 port 46557 connected with 192.168.1.1 port 5001
> > [  5]  0.0-60.4 sec  58.9 MBytes  8.18 Mbits/sec
> > [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 51633
> > [  4]  0.0-63.1 sec  7.27 MBytes    967 Kbits/sec
>
> Why do we have two different measurements here? Is one TX and one RX?
> Which one?

Yes, the first is TX (BCM4401 --> e100) and the second is RX. Both are tcp 
connections. I think iperf does display the ip addresses wrong in the second 
connection, but that's another issue.

>
> > koala:~# ping -c10 192.168.1.1
> > PING 192.168.1.1 (192.168.1.1) 56(84) bytes of data.
> > 64 bytes from 192.168.1.1: icmp_seq=1 ttl=64 time=0.243 ms
> > 64 bytes from 192.168.1.1: icmp_seq=2 ttl=64 time=0.234 ms
> > 64 bytes from 192.168.1.1: icmp_seq=3 ttl=64 time=0.238 ms
> > 64 bytes from 192.168.1.1: icmp_seq=4 ttl=64 time=0.235 ms
> > 64 bytes from 192.168.1.1: icmp_seq=5 ttl=64 time=0.230 ms
> > 64 bytes from 192.168.1.1: icmp_seq=6 ttl=64 time=0.317 ms
> > 64 bytes from 192.168.1.1: icmp_seq=7 ttl=64 time=0.232 ms
> > 64 bytes from 192.168.1.1: icmp_seq=8 ttl=64 time=0.232 ms
> > 64 bytes from 192.168.1.1: icmp_seq=9 ttl=64 time=0.228 ms
> > 64 bytes from 192.168.1.1: icmp_seq=10 ttl=64 time=0.238 ms
> >
> > --- 192.168.1.1 ping statistics ---
> > 10 packets transmitted, 10 received, 0% packet loss, time 8997ms
> > rtt min/avg/max/mdev = 0.228/0.242/0.317/0.031 ms
> >
> > System responsiveness was the same as with 2.6.21.1.
> >
> > wget got 11.23M/s, again same as 2.6.21.1.
> >
> >
> > 2.6.22-rc2-mm1:
> >
> > [  5] local 192.168.1.2 port 42198 connected with 192.168.1.1 port 5001
> > [  5]  0.0-60.1 sec    402 MBytes  56.1 Mbits/sec
> > [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 48598
> > [  4]  0.0-63.0 sec    177 MBytes  23.6 Mbits/sec
>
> So with -mm (with ssb) you actually get better performace
> then with plain 2.6.22-rc3?
>
> Can you elaborate a bit more about what you get an what you expect
> on which kernel?

When I ran 2.6.21.1 or 2.6.22-rc3 without any debugging tools just in normal 
use I didn't notice any problems. It did work fine as I would expect it.
I think the wget and ping tests here are as they should be.

With 2.6.22-rc2-mm1 I noticed that connections seem to be slower. The ping 
test does confirm this, because here response times are very high. As far as 
I can remember the wget download rate was a bit slower than 2.6.21.1 or 
2.6.22-rc3 till it stalled.
I would expect it to be someting like the other two kernels. The two problems 
I see are the high ping times and the fact that the card stopped working.

I don't know why the iperf results are so different from my personal 
experience. I guess the fact that I get so bad results with 2.6.21.1 and 
2.6.22-rc3 is that iperf does something that causes the system to be 
extremely slow and thus degrading performance. This could be a bug somewhere 
in the b44 driver of 2.6.21.1 and 2.6.22-RC3 that has unintended been fixed 
by the ssb switch, but that's only a roughly guess.

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

On Sunday 27 May 2007 22:36:39 Maximilian Engelhardt wrote:
> When I ran 2.6.21.1 or 2.6.22-rc3 without any debugging tools just in normal 
> use I didn't notice any problems. It did work fine as I would expect it.
> I think the wget and ping tests here are as they should be.
> 
> With 2.6.22-rc2-mm1 I noticed that connections seem to be slower. The ping 
> test does confirm this, because here response times are very high. As far as 
> I can remember the wget download rate was a bit slower than 2.6.21.1 or 
> 2.6.22-rc3 till it stalled.
> I would expect it to be someting like the other two kernels. The two problems 
> I see are the high ping times and the fact that the card stopped working.
> 
> I don't know why the iperf results are so different from my personal 
> experience. I guess the fact that I get so bad results with 2.6.21.1 and 
> 2.6.22-rc3 is that iperf does something that causes the system to be 
> extremely slow and thus degrading performance. This could be a bug somewhere 
> in the b44 driver of 2.6.21.1 and 2.6.22-RC3 that has unintended been fixed 
> by the ssb switch, but that's only a roughly guess.

Ok. I guess (Yes I do :D) that there is an IRQ storm or something like that,
because you say that your system is becoming very slow and unresponsive.
It sounds like an IRQ is not ACKed correctly and so keeps triggering and
stalling the system. I'll take a look at a few diffs...
Do you see significant differences in the "hi" and/or "si" times in top?
Do you see a significant difference in the /proc/interrupts count. For
example that the kernel that works worse generates 10 times the IRQ count
for the same amount of data.

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

On Sunday 27 May 2007 21:25:17 Maximilian Engelhardt wrote:
> 2.6.21.1:
> [  5] local 192.168.1.2 port 58414 connected with 192.168.1.1 port 5001
> [  5]  0.0-60.6 sec  1.13 MBytes    157 Kbits/sec
> [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 57837
> [  4]  0.0-63.1 sec  2.82 MBytes    375 Kbits/sec

> 2.6.22-rc3:
> [  5] local 192.168.1.2 port 46557 connected with 192.168.1.1 port 5001
> [  5]  0.0-60.4 sec  58.9 MBytes  8.18 Mbits/sec
> [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 51633
> [  4]  0.0-63.1 sec  7.27 MBytes    967 Kbits/sec

This is the diff between these two kernels.
I'm not sure why you see a much better TX throughput here.

Can you re-check to make sure it's not just some test-jitter?


--- linux-2.6.21.1/drivers/net/b44.c    2007-05-27 22:58:01.000000000 +0200
+++ linux-2.6.22-rc3/drivers/net/b44.c  2007-05-27 23:01:44.000000000 +0200
@@ -825,12 +825,11 @@
                        if (copy_skb == NULL)
                                goto drop_it_no_recycle;
 
-                       copy_skb->dev = bp->dev;
                        skb_reserve(copy_skb, 2);
                        skb_put(copy_skb, len);
                        /* DMA sync done above, copy just the actual packet */
-                       memcpy(copy_skb->data, skb->data+bp->rx_offset, len);
-
+                       skb_copy_from_linear_data_offset(skb, bp->rx_offset,
+                                                        copy_skb->data, len);
                        skb = copy_skb;
                }
                skb->ip_summed = CHECKSUM_NONE;
@@ -1007,7 +1006,8 @@
                        goto err_out;
                }
 
-               memcpy(skb_put(bounce_skb, len), skb->data, skb->len);
+               skb_copy_from_linear_data(skb, skb_put(bounce_skb, len),
+                                         skb->len);
                dev_kfree_skb_any(skb);
                skb = bounce_skb;
        }

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

On Sunday 27 May 2007 23:13:32 Michael Buesch wrote:
> On Sunday 27 May 2007 21:25:17 Maximilian Engelhardt wrote:
> > 2.6.21.1:
> > [  5] local 192.168.1.2 port 58414 connected with 192.168.1.1 port 5001
> > [  5]  0.0-60.6 sec  1.13 MBytes    157 Kbits/sec
> > [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 57837
> > [  4]  0.0-63.1 sec  2.82 MBytes    375 Kbits/sec
> 
> > 2.6.22-rc3:
> > [  5] local 192.168.1.2 port 46557 connected with 192.168.1.1 port 5001
> > [  5]  0.0-60.4 sec  58.9 MBytes  8.18 Mbits/sec
> > [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 51633
> > [  4]  0.0-63.1 sec  7.27 MBytes    967 Kbits/sec
> 
> This is the diff between these two kernels.
> I'm not sure why you see a much better TX throughput here.
> 
> Can you re-check to make sure it's not just some test-jitter?

Oh, eh, and what I forgot to ask:
Do you know an old kernel that works perfectly well for you,
so I can look at a diff between this one and anything >=2.6.21.1.

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 2664 bytes --]

On Sunday 27 May 2007, Michael Buesch wrote:
> On Sunday 27 May 2007 22:36:39 Maximilian Engelhardt wrote:
> > When I ran 2.6.21.1 or 2.6.22-rc3 without any debugging tools just in
> > normal use I didn't notice any problems. It did work fine as I would
> > expect it. I think the wget and ping tests here are as they should be.
> >
> > With 2.6.22-rc2-mm1 I noticed that connections seem to be slower. The
> > ping test does confirm this, because here response times are very high.
> > As far as I can remember the wget download rate was a bit slower than
> > 2.6.21.1 or 2.6.22-rc3 till it stalled.
> > I would expect it to be someting like the other two kernels. The two
> > problems I see are the high ping times and the fact that the card stopped
> > working.
> >
> > I don't know why the iperf results are so different from my personal
> > experience. I guess the fact that I get so bad results with 2.6.21.1 and
> > 2.6.22-rc3 is that iperf does something that causes the system to be
> > extremely slow and thus degrading performance. This could be a bug
> > somewhere in the b44 driver of 2.6.21.1 and 2.6.22-RC3 that has
> > unintended been fixed by the ssb switch, but that's only a roughly guess.
>
> Ok. I guess (Yes I do :D) that there is an IRQ storm or something like
> that, because you say that your system is becoming very slow and
> unresponsive. It sounds like an IRQ is not ACKed correctly and so keeps
> triggering and stalling the system. I'll take a look at a few diffs...
> Do you see significant differences in the "hi" and/or "si" times in top?
> Do you see a significant difference in the /proc/interrupts count. For
> example that the kernel that works worse generates 10 times the IRQ count
> for the same amount of data.

ok, here are the results:

Using 2.6.22-rc3 I get lot's of hi during TX and lots of hi and si during RX.
Using 2.6.22-rc3-mm1 hi and si are significantly lower.
It's difficult to give absolute numbers, because top refreshes very slow, but 
with 2.6.22-rc3 hi is about 30% during TX and RX and si is 0% during TX and 
50% during RX. With Using 2.6.22-rc3-mm1 hi is 0% during TX and 0.3% during 
RX and si is 10% during TX and 0% during RX.

When I do the same test on both kernels I get about 10 times (yes, it's really 
about ten times like in your example) more interrupts with 2.6.22-rc3 than 
with 2.6.22-rc3-mm1.

An additional thing I noticed it that it's not the BCM4401 card that stops 
working but my e100 card. If I take the e100 card down and up again the 
connection is working again, so the BCM4401 doesn't have a "stops working" 
bug for me.

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]

On Sunday 27 May 2007, Michael Buesch wrote:
> On Sunday 27 May 2007 23:13:32 Michael Buesch wrote:
> > On Sunday 27 May 2007 21:25:17 Maximilian Engelhardt wrote:
> > > 2.6.21.1:
> > > [  5] local 192.168.1.2 port 58414 connected with 192.168.1.1 port 5001
> > > [  5]  0.0-60.6 sec  1.13 MBytes    157 Kbits/sec
> > > [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 57837
> > > [  4]  0.0-63.1 sec  2.82 MBytes    375 Kbits/sec
> > >
> > > 2.6.22-rc3:
> > > [  5] local 192.168.1.2 port 46557 connected with 192.168.1.1 port 5001
> > > [  5]  0.0-60.4 sec  58.9 MBytes  8.18 Mbits/sec
> > > [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 51633
> > > [  4]  0.0-63.1 sec  7.27 MBytes    967 Kbits/sec
> >
> > This is the diff between these two kernels.
> > I'm not sure why you see a much better TX throughput here.
> >
> > Can you re-check to make sure it's not just some test-jitter?
>
> Oh, eh, and what I forgot to ask:
> Do you know an old kernel that works perfectly well for you,
> so I can look at a diff between this one and anything >=2.6.21.1.

I don't know any, most older kernels did work fine for me, but I never user 
iperf there so I guess if the bug is there also I simply didn't trigger it.
If you think it's usefull I could go back and try different kernels, but that 
would take some time.
Except the iperf bug 2.6.21.1 and 2.6.22-rc3 work fine.

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 3445 bytes --]

On Sunday 27 May 2007, Michael Buesch wrote:
> On Sunday 27 May 2007 21:25:17 Maximilian Engelhardt wrote:
> > 2.6.21.1:
> > [  5] local 192.168.1.2 port 58414 connected with 192.168.1.1 port 5001
> > [  5]  0.0-60.6 sec  1.13 MBytes    157 Kbits/sec
> > [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 57837
> > [  4]  0.0-63.1 sec  2.82 MBytes    375 Kbits/sec
> >
> > 2.6.22-rc3:
> > [  5] local 192.168.1.2 port 46557 connected with 192.168.1.1 port 5001
> > [  5]  0.0-60.4 sec  58.9 MBytes  8.18 Mbits/sec
> > [  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 51633
> > [  4]  0.0-63.1 sec  7.27 MBytes    967 Kbits/sec
>
> This is the diff between these two kernels.
> I'm not sure why you see a much better TX throughput here.
>
> Can you re-check to make sure it's not just some test-jitter?
>
2.6.21.1:

[  5] local 192.168.1.2 port 54423 connected with 192.168.1.1 port 5001
[  5]  0.0-60.3 sec  3.06 MBytes    426 Kbits/sec
[  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 41053
[  4]  0.0-163.0 sec    130 MBytes  6.67 Mbits/sec


2.6.22-rc3:

[  5] local 192.168.1.2 port 46002 connected with 192.168.1.1 port 5001
[  5]  0.0-61.5 sec  84.0 MBytes  11.5 Mbits/sec
[  4] local 192.168.1.2 port 5001 connected with 192.168.1.1 port 44379
[  4]  0.0-93.8 sec  30.6 MBytes  2.74 Mbits/sec

For TX the iperf server reports the same values as the client (all values are 
from the client) but for RX they are differen:

2.6.21.1: (iperf server log):

[  5] local 192.168.1.1 port 5001 connected with 192.168.1.2 port 54423
[  5]  0.0-60.5 sec  3.06 MBytes    425 Kbits/sec
[  5] local 192.168.1.1 port 41053 connected with 192.168.1.2 port 5001
[  5]  0.0-63.1 sec    130 MBytes  17.2 Mbits/sec


2.6.22-rc3 (iperf server log):

[  4] local 192.168.1.1 port 5001 connected with 192.168.1.2 port 46002
[  4]  0.0-61.6 sec  84.0 MBytes  11.5 Mbits/sec
[  4] local 192.168.1.1 port 44379 connected with 192.168.1.2 port 5001
[  4]  0.0-63.3 sec  30.6 MBytes  4.06 Mbits/sec

I have no idea how iperf internally works and what can cause such different 
results here.

>
> --- linux-2.6.21.1/drivers/net/b44.c    2007-05-27 22:58:01.000000000 +0200
> +++ linux-2.6.22-rc3/drivers/net/b44.c  2007-05-27 23:01:44.000000000 +0200
> @@ -825,12 +825,11 @@
>                         if (copy_skb == NULL)
>                                 goto drop_it_no_recycle;
>
> -                       copy_skb->dev = bp->dev;
>                         skb_reserve(copy_skb, 2);
>                         skb_put(copy_skb, len);
>                         /* DMA sync done above, copy just the actual packet
> */ -                       memcpy(copy_skb->data, skb->data+bp->rx_offset,
> len); -
> +                       skb_copy_from_linear_data_offset(skb,
> bp->rx_offset, +                                                       
> copy_skb->data, len); skb = copy_skb;
>                 }
>                 skb->ip_summed = CHECKSUM_NONE;
> @@ -1007,7 +1006,8 @@
>                         goto err_out;
>                 }
>
> -               memcpy(skb_put(bounce_skb, len), skb->data, skb->len);
> +               skb_copy_from_linear_data(skb, skb_put(bounce_skb, len),
> +                                         skb->len);
>                 dev_kfree_skb_any(skb);
>                 skb = bounce_skb;
>         }



[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

Ok, another question: On which CPU architecture are you?

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 139 bytes --]

On Monday 28 May 2007, Michael Buesch wrote:
> Ok, another question: On which CPU architecture are you?

maxi@koala:~$ uname -m
i686

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

Can you give 2.6.16 a try? The diff is not that big and we might
be able to find out what broke if you find out 2.6.16 works.
You can also try later kernels like .17, .18, .19 to further
reduce the patch. (You could also git-bisect, if you have the time).

git-diff v2.6.16..v2.6.22-rc3 drivers/net/b44.c

diff --git a/drivers/net/b44.c b/drivers/net/b44.c
index c3267e4..879a2ff 100644
--- a/drivers/net/b44.c
+++ b/drivers/net/b44.c
@@ -2,6 +2,7 @@
  *
  * Copyright (C) 2002 David S. Miller (davem-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org)
  * Fixed by Pekka Pietikainen (pp-YuCZbdju05vHOG6cAo2yLw@public.gmane.org)
+ * Copyright (C) 2006 Broadcom Corporation.
  *
  * Distribute under GPL.
  */
@@ -28,8 +29,8 @@ #include "b44.h"
 
 #define DRV_MODULE_NAME		"b44"
 #define PFX DRV_MODULE_NAME	": "
-#define DRV_MODULE_VERSION	"0.97"
-#define DRV_MODULE_RELDATE	"Nov 30, 2005"
+#define DRV_MODULE_VERSION	"1.01"
+#define DRV_MODULE_RELDATE	"Jun 16, 2006"
 
 #define B44_DEF_MSG_ENABLE	  \
 	(NETIF_MSG_DRV		| \
@@ -58,7 +59,6 @@ #define B44_TX_RING_SIZE		512
 #define B44_DEF_TX_RING_PENDING		(B44_TX_RING_SIZE - 1)
 #define B44_TX_RING_BYTES	(sizeof(struct dma_desc) * \
 				 B44_TX_RING_SIZE)
-#define B44_DMA_MASK 0x3fffffff
 
 #define TX_RING_GAP(BP)	\
 	(B44_TX_RING_SIZE - (BP)->tx_pending)
@@ -74,6 +74,15 @@ #define TX_PKT_BUF_SZ		(B44_MAX_MTU + ET
 /* minimum number of free TX descriptors required to wake up TX process */
 #define B44_TX_WAKEUP_THRESH		(B44_TX_RING_SIZE / 4)
 
+/* b44 internal pattern match filter info */
+#define B44_PATTERN_BASE	0x400
+#define B44_PATTERN_SIZE	0x80
+#define B44_PMASK_BASE		0x600
+#define B44_PMASK_SIZE		0x10
+#define B44_MAX_PATTERNS	16
+#define B44_ETHIPV6UDP_HLEN	62
+#define B44_ETHIPV4UDP_HLEN	42
+
 static char version[] __devinitdata =
 	DRV_MODULE_NAME ".c:v" DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")\n";
 
@@ -100,7 +109,12 @@ MODULE_DEVICE_TABLE(pci, b44_pci_tbl);
 
 static void b44_halt(struct b44 *);
 static void b44_init_rings(struct b44 *);
-static void b44_init_hw(struct b44 *);
+
+#define B44_FULL_RESET		1
+#define B44_FULL_RESET_SKIP_PHY	2
+#define B44_PARTIAL_RESET	3
+
+static void b44_init_hw(struct b44 *, int);
 
 static int dma_desc_align_mask;
 static int dma_desc_sync_size;
@@ -136,7 +150,7 @@ static inline unsigned long br32(const s
 	return readl(bp->regs + reg);
 }
 
-static inline void bw32(const struct b44 *bp, 
+static inline void bw32(const struct b44 *bp,
 			unsigned long reg, unsigned long val)
 {
 	writel(val, bp->regs + reg);
@@ -286,13 +300,13 @@ static void __b44_cam_write(struct b44 *
 	val |= ((u32) data[4]) <<  8;
 	val |= ((u32) data[5]) <<  0;
 	bw32(bp, B44_CAM_DATA_LO, val);
-	val = (CAM_DATA_HI_VALID | 
+	val = (CAM_DATA_HI_VALID |
 	       (((u32) data[0]) << 8) |
 	       (((u32) data[1]) << 0));
 	bw32(bp, B44_CAM_DATA_HI, val);
 	bw32(bp, B44_CAM_CTRL, (CAM_CTRL_WRITE |
 			    (index << CAM_CTRL_INDEX_SHIFT)));
-	b44_wait_bit(bp, B44_CAM_CTRL, CAM_CTRL_BUSY, 100, 1);	
+	b44_wait_bit(bp, B44_CAM_CTRL, CAM_CTRL_BUSY, 100, 1);
 }
 
 static inline void __b44_disable_ints(struct b44 *bp)
@@ -410,25 +424,18 @@ static void __b44_set_flow_ctrl(struct b
 
 static void b44_set_flow_ctrl(struct b44 *bp, u32 local, u32 remote)
 {
-	u32 pause_enab = bp->flags & (B44_FLAG_TX_PAUSE |
-				      B44_FLAG_RX_PAUSE);
+	u32 pause_enab = 0;
 
-	if (local & ADVERTISE_PAUSE_CAP) {
-		if (local & ADVERTISE_PAUSE_ASYM) {
-			if (remote & LPA_PAUSE_CAP)
-				pause_enab |= (B44_FLAG_TX_PAUSE |
-					       B44_FLAG_RX_PAUSE);
-			else if (remote & LPA_PAUSE_ASYM)
-				pause_enab |= B44_FLAG_RX_PAUSE;
-		} else {
-			if (remote & LPA_PAUSE_CAP)
-				pause_enab |= (B44_FLAG_TX_PAUSE |
-					       B44_FLAG_RX_PAUSE);
-		}
-	} else if (local & ADVERTISE_PAUSE_ASYM) {
-		if ((remote & LPA_PAUSE_CAP) &&
-		    (remote & LPA_PAUSE_ASYM))
-			pause_enab |= B44_FLAG_TX_PAUSE;
+	/* The driver supports only rx pause by default because
+	   the b44 mac tx pause mechanism generates excessive
+	   pause frames.
+	   Use ethtool to turn on b44 tx pause if necessary.
+	 */
+	if ((local & ADVERTISE_PAUSE_CAP) &&
+	    (local & ADVERTISE_PAUSE_ASYM)){
+		if ((remote & LPA_PAUSE_ASYM) &&
+		    !(remote & LPA_PAUSE_CAP))
+			pause_enab |= B44_FLAG_RX_PAUSE;
 	}
 
 	__b44_set_flow_ctrl(bp, pause_enab);
@@ -608,8 +615,7 @@ static void b44_tx(struct b44 *bp)
 		struct ring_info *rp = &bp->tx_buffers[cons];
 		struct sk_buff *skb = rp->skb;
 
-		if (unlikely(skb == NULL))
-			BUG();
+		BUG_ON(skb == NULL);
 
 		pci_unmap_single(bp->pdev,
 				 pci_unmap_addr(rp, mapping),
@@ -657,9 +663,11 @@ static int b44_alloc_rx_skb(struct b44 *
 
 	/* Hardware bug work-around, the chip is unable to do PCI DMA
 	   to/from anything above 1GB :-( */
-	if (mapping + RX_PKT_BUF_SZ > B44_DMA_MASK) {
+	if (dma_mapping_error(mapping) ||
+		mapping + RX_PKT_BUF_SZ > DMA_30BIT_MASK) {
 		/* Sigh... */
-		pci_unmap_single(bp->pdev, mapping, RX_PKT_BUF_SZ,PCI_DMA_FROMDEVICE);
+		if (!dma_mapping_error(mapping))
+			pci_unmap_single(bp->pdev, mapping, RX_PKT_BUF_SZ,PCI_DMA_FROMDEVICE);
 		dev_kfree_skb_any(skb);
 		skb = __dev_alloc_skb(RX_PKT_BUF_SZ,GFP_DMA);
 		if (skb == NULL)
@@ -667,8 +675,10 @@ static int b44_alloc_rx_skb(struct b44 *
 		mapping = pci_map_single(bp->pdev, skb->data,
 					 RX_PKT_BUF_SZ,
 					 PCI_DMA_FROMDEVICE);
-		if (mapping + RX_PKT_BUF_SZ > B44_DMA_MASK) {
-			pci_unmap_single(bp->pdev, mapping, RX_PKT_BUF_SZ,PCI_DMA_FROMDEVICE);
+		if (dma_mapping_error(mapping) ||
+			mapping + RX_PKT_BUF_SZ > DMA_30BIT_MASK) {
+			if (!dma_mapping_error(mapping))
+				pci_unmap_single(bp->pdev, mapping, RX_PKT_BUF_SZ,PCI_DMA_FROMDEVICE);
 			dev_kfree_skb_any(skb);
 			return -ENOMEM;
 		}
@@ -710,7 +720,7 @@ static void b44_recycle_rx(struct b44 *b
 	struct ring_info *src_map, *dest_map;
 	struct rx_header *rh;
 	int dest_idx;
-	u32 ctrl;
+	__le32 ctrl;
 
 	dest_idx = dest_idx_unmasked & (B44_RX_RING_SIZE - 1);
 	dest_desc = &bp->rx_ring[dest_idx];
@@ -746,7 +756,7 @@ static void b44_recycle_rx(struct b44 *b
 		                             dest_idx * sizeof(dest_desc),
 		                             DMA_BIDIRECTIONAL);
 
-	pci_dma_sync_single_for_device(bp->pdev, src_desc->addr,
+	pci_dma_sync_single_for_device(bp->pdev, le32_to_cpu(src_desc->addr),
 				       RX_PKT_BUF_SZ,
 				       PCI_DMA_FROMDEVICE);
 }
@@ -772,7 +782,7 @@ static int b44_rx(struct b44 *bp, int bu
 					    RX_PKT_BUF_SZ,
 					    PCI_DMA_FROMDEVICE);
 		rh = (struct rx_header *) skb->data;
-		len = cpu_to_le16(rh->len);
+		len = le16_to_cpu(rh->len);
 		if ((len > (RX_PKT_BUF_SZ - bp->rx_offset)) ||
 		    (rh->flags & cpu_to_le16(RX_FLAG_ERRORS))) {
 		drop_it:
@@ -788,7 +798,7 @@ static int b44_rx(struct b44 *bp, int bu
 			do {
 				udelay(2);
 				barrier();
-				len = cpu_to_le16(rh->len);
+				len = le16_to_cpu(rh->len);
 			} while (len == 0 && i++ < 5);
 			if (len == 0)
 				goto drop_it;
@@ -815,12 +825,11 @@ static int b44_rx(struct b44 *bp, int bu
 			if (copy_skb == NULL)
 				goto drop_it_no_recycle;
 
-			copy_skb->dev = bp->dev;
 			skb_reserve(copy_skb, 2);
 			skb_put(copy_skb, len);
 			/* DMA sync done above, copy just the actual packet */
-			memcpy(copy_skb->data, skb->data+bp->rx_offset, len);
-
+			skb_copy_from_linear_data_offset(skb, bp->rx_offset,
+							 copy_skb->data, len);
 			skb = copy_skb;
 		}
 		skb->ip_summed = CHECKSUM_NONE;
@@ -873,12 +882,14 @@ static int b44_poll(struct net_device *n
 	}
 
 	if (bp->istat & ISTAT_ERRORS) {
-		spin_lock_irq(&bp->lock);
+		unsigned long flags;
+
+		spin_lock_irqsave(&bp->lock, flags);
 		b44_halt(bp);
 		b44_init_rings(bp);
-		b44_init_hw(bp);
+		b44_init_hw(bp, B44_FULL_RESET_SKIP_PHY);
 		netif_wake_queue(bp->dev);
-		spin_unlock_irq(&bp->lock);
+		spin_unlock_irqrestore(&bp->lock, flags);
 		done = 1;
 	}
 
@@ -890,7 +901,7 @@ static int b44_poll(struct net_device *n
 	return (done ? 0 : 1);
 }
 
-static irqreturn_t b44_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+static irqreturn_t b44_interrupt(int irq, void *dev_id)
 {
 	struct net_device *dev = dev_id;
 	struct b44 *bp = netdev_priv(dev);
@@ -902,8 +913,9 @@ static irqreturn_t b44_interrupt(int irq
 	istat = br32(bp, B44_ISTAT);
 	imask = br32(bp, B44_IMASK);
 
-	/* ??? What the fuck is the purpose of the interrupt mask
-	 * ??? register if we have to mask it out by hand anyways?
+	/* The interrupt mask register controls which interrupt bits
+	 * will actually raise an interrupt to the CPU when set by hw/firmware,
+	 * but doesn't mask off the bits.
 	 */
 	istat &= imask;
 	if (istat) {
@@ -945,7 +957,7 @@ static void b44_tx_timeout(struct net_de
 
 	b44_halt(bp);
 	b44_init_rings(bp);
-	b44_init_hw(bp);
+	b44_init_hw(bp, B44_FULL_RESET);
 
 	spin_unlock_irq(&bp->lock);
 
@@ -974,9 +986,10 @@ static int b44_start_xmit(struct sk_buff
 	}
 
 	mapping = pci_map_single(bp->pdev, skb->data, len, PCI_DMA_TODEVICE);
-	if (mapping + len > B44_DMA_MASK) {
+	if (dma_mapping_error(mapping) || mapping + len > DMA_30BIT_MASK) {
 		/* Chip can't handle DMA to/from >1GB, use bounce buffer */
-		pci_unmap_single(bp->pdev, mapping, len, PCI_DMA_TODEVICE);
+		if (!dma_mapping_error(mapping))
+			pci_unmap_single(bp->pdev, mapping, len, PCI_DMA_TODEVICE);
 
 		bounce_skb = __dev_alloc_skb(TX_PKT_BUF_SZ,
 					     GFP_ATOMIC|GFP_DMA);
@@ -985,14 +998,16 @@ static int b44_start_xmit(struct sk_buff
 
 		mapping = pci_map_single(bp->pdev, bounce_skb->data,
 					 len, PCI_DMA_TODEVICE);
-		if (mapping + len > B44_DMA_MASK) {
-			pci_unmap_single(bp->pdev, mapping,
+		if (dma_mapping_error(mapping) || mapping + len > DMA_30BIT_MASK) {
+			if (!dma_mapping_error(mapping))
+				pci_unmap_single(bp->pdev, mapping,
 					 len, PCI_DMA_TODEVICE);
 			dev_kfree_skb_any(bounce_skb);
 			goto err_out;
 		}
 
-		memcpy(skb_put(bounce_skb, len), skb->data, skb->len);
+		skb_copy_from_linear_data(skb, skb_put(bounce_skb, len),
+					  skb->len);
 		dev_kfree_skb_any(skb);
 		skb = bounce_skb;
 	}
@@ -1060,11 +1075,11 @@ static int b44_change_mtu(struct net_dev
 	b44_halt(bp);
 	dev->mtu = new_mtu;
 	b44_init_rings(bp);
-	b44_init_hw(bp);
+	b44_init_hw(bp, B44_FULL_RESET);
 	spin_unlock_irq(&bp->lock);
 
 	b44_enable_ints(bp);
-	
+
 	return 0;
 }
 
@@ -1210,7 +1225,8 @@ static int b44_alloc_consistent(struct b
 		                             DMA_TABLE_BYTES,
 		                             DMA_BIDIRECTIONAL);
 
-		if (rx_ring_dma + size > B44_DMA_MASK) {
+		if (dma_mapping_error(rx_ring_dma) ||
+			rx_ring_dma + size > DMA_30BIT_MASK) {
 			kfree(rx_ring);
 			goto out_err;
 		}
@@ -1236,7 +1252,8 @@ static int b44_alloc_consistent(struct b
 		                             DMA_TABLE_BYTES,
 		                             DMA_TO_DEVICE);
 
-		if (tx_ring_dma + size > B44_DMA_MASK) {
+		if (dma_mapping_error(tx_ring_dma) ||
+			tx_ring_dma + size > DMA_30BIT_MASK) {
 			kfree(tx_ring);
 			goto out_err;
 		}
@@ -1271,7 +1288,7 @@ static void b44_chip_reset(struct b44 *b
 	if (ssb_is_core_up(bp)) {
 		bw32(bp, B44_RCV_LAZY, 0);
 		bw32(bp, B44_ENET_CTRL, ENET_CTRL_DISABLE);
-		b44_wait_bit(bp, B44_ENET_CTRL, ENET_CTRL_DISABLE, 100, 1);
+		b44_wait_bit(bp, B44_ENET_CTRL, ENET_CTRL_DISABLE, 200, 1);
 		bw32(bp, B44_DMATX_CTRL, 0);
 		bp->tx_prod = bp->tx_cons = 0;
 		if (br32(bp, B44_DMARX_STAT) & DMARX_STAT_EMASK) {
@@ -1339,6 +1356,9 @@ static int b44_set_mac_addr(struct net_d
 	if (netif_running(dev))
 		return -EBUSY;
 
+	if (!is_valid_ether_addr(addr->sa_data))
+		return -EINVAL;
+
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
 
 	spin_lock_irq(&bp->lock);
@@ -1352,13 +1372,15 @@ static int b44_set_mac_addr(struct net_d
  * packet processing.  Invoked with bp->lock held.
  */
 static void __b44_set_rx_mode(struct net_device *);
-static void b44_init_hw(struct b44 *bp)
+static void b44_init_hw(struct b44 *bp, int reset_kind)
 {
 	u32 val;
 
 	b44_chip_reset(bp);
-	b44_phy_reset(bp);
-	b44_setup_phy(bp);
+	if (reset_kind == B44_FULL_RESET) {
+		b44_phy_reset(bp);
+		b44_setup_phy(bp);
+	}
 
 	/* Enable CRC32, set proper LED modes and power on PHY */
 	bw32(bp, B44_MAC_CTRL, MAC_CTRL_CRC32_ENAB | MAC_CTRL_PHY_LEDCTRL);
@@ -1372,16 +1394,21 @@ static void b44_init_hw(struct b44 *bp)
 	bw32(bp, B44_TXMAXLEN, bp->dev->mtu + ETH_HLEN + 8 + RX_HEADER_LEN);
 
 	bw32(bp, B44_TX_WMARK, 56); /* XXX magic */
-	bw32(bp, B44_DMATX_CTRL, DMATX_CTRL_ENABLE);
-	bw32(bp, B44_DMATX_ADDR, bp->tx_ring_dma + bp->dma_offset);
-	bw32(bp, B44_DMARX_CTRL, (DMARX_CTRL_ENABLE |
-			      (bp->rx_offset << DMARX_CTRL_ROSHIFT)));
-	bw32(bp, B44_DMARX_ADDR, bp->rx_ring_dma + bp->dma_offset);
+	if (reset_kind == B44_PARTIAL_RESET) {
+		bw32(bp, B44_DMARX_CTRL, (DMARX_CTRL_ENABLE |
+				      (bp->rx_offset << DMARX_CTRL_ROSHIFT)));
+	} else {
+		bw32(bp, B44_DMATX_CTRL, DMATX_CTRL_ENABLE);
+		bw32(bp, B44_DMATX_ADDR, bp->tx_ring_dma + bp->dma_offset);
+		bw32(bp, B44_DMARX_CTRL, (DMARX_CTRL_ENABLE |
+				      (bp->rx_offset << DMARX_CTRL_ROSHIFT)));
+		bw32(bp, B44_DMARX_ADDR, bp->rx_ring_dma + bp->dma_offset);
 
-	bw32(bp, B44_DMARX_PTR, bp->rx_pending);
-	bp->rx_prod = bp->rx_pending;	
+		bw32(bp, B44_DMARX_PTR, bp->rx_pending);
+		bp->rx_prod = bp->rx_pending;
 
-	bw32(bp, B44_MIB_CTRL, MIB_CTRL_CLR_ON_READ);
+		bw32(bp, B44_MIB_CTRL, MIB_CTRL_CLR_ON_READ);
+	}
 
 	val = br32(bp, B44_ENET_CTRL);
 	bw32(bp, B44_ENET_CTRL, (val | ENET_CTRL_ENABLE));
@@ -1397,11 +1424,11 @@ static int b44_open(struct net_device *d
 		goto out;
 
 	b44_init_rings(bp);
-	b44_init_hw(bp);
+	b44_init_hw(bp, B44_FULL_RESET);
 
 	b44_check_phy(bp);
 
-	err = request_irq(dev->irq, b44_interrupt, SA_SHIRQ, dev->name, dev);
+	err = request_irq(dev->irq, b44_interrupt, IRQF_SHARED, dev->name, dev);
 	if (unlikely(err < 0)) {
 		b44_chip_reset(bp);
 		b44_free_rings(bp);
@@ -1441,11 +1468,145 @@ #ifdef CONFIG_NET_POLL_CONTROLLER
 static void b44_poll_controller(struct net_device *dev)
 {
 	disable_irq(dev->irq);
-	b44_interrupt(dev->irq, dev, NULL);
+	b44_interrupt(dev->irq, dev);
 	enable_irq(dev->irq);
 }
 #endif
 
+static void bwfilter_table(struct b44 *bp, u8 *pp, u32 bytes, u32 table_offset)
+{
+	u32 i;
+	u32 *pattern = (u32 *) pp;
+
+	for (i = 0; i < bytes; i += sizeof(u32)) {
+		bw32(bp, B44_FILT_ADDR, table_offset + i);
+		bw32(bp, B44_FILT_DATA, pattern[i / sizeof(u32)]);
+	}
+}
+
+static int b44_magic_pattern(u8 *macaddr, u8 *ppattern, u8 *pmask, int offset)
+{
+	int magicsync = 6;
+	int k, j, len = offset;
+	int ethaddr_bytes = ETH_ALEN;
+
+	memset(ppattern + offset, 0xff, magicsync);
+	for (j = 0; j < magicsync; j++)
+		set_bit(len++, (unsigned long *) pmask);
+
+	for (j = 0; j < B44_MAX_PATTERNS; j++) {
+		if ((B44_PATTERN_SIZE - len) >= ETH_ALEN)
+			ethaddr_bytes = ETH_ALEN;
+		else
+			ethaddr_bytes = B44_PATTERN_SIZE - len;
+		if (ethaddr_bytes <=0)
+			break;
+		for (k = 0; k< ethaddr_bytes; k++) {
+			ppattern[offset + magicsync +
+				(j * ETH_ALEN) + k] = macaddr[k];
+			len++;
+			set_bit(len, (unsigned long *) pmask);
+		}
+	}
+	return len - 1;
+}
+
+/* Setup magic packet patterns in the b44 WOL
+ * pattern matching filter.
+ */
+static void b44_setup_pseudo_magicp(struct b44 *bp)
+{
+
+	u32 val;
+	int plen0, plen1, plen2;
+	u8 *pwol_pattern;
+	u8 pwol_mask[B44_PMASK_SIZE];
+
+	pwol_pattern = kmalloc(B44_PATTERN_SIZE, GFP_KERNEL);
+	if (!pwol_pattern) {
+		printk(KERN_ERR PFX "Memory not available for WOL\n");
+		return;
+	}
+
+	/* Ipv4 magic packet pattern - pattern 0.*/
+	memset(pwol_pattern, 0, B44_PATTERN_SIZE);
+	memset(pwol_mask, 0, B44_PMASK_SIZE);
+	plen0 = b44_magic_pattern(bp->dev->dev_addr, pwol_pattern, pwol_mask,
+				  B44_ETHIPV4UDP_HLEN);
+
+   	bwfilter_table(bp, pwol_pattern, B44_PATTERN_SIZE, B44_PATTERN_BASE);
+   	bwfilter_table(bp, pwol_mask, B44_PMASK_SIZE, B44_PMASK_BASE);
+
+	/* Raw ethernet II magic packet pattern - pattern 1 */
+	memset(pwol_pattern, 0, B44_PATTERN_SIZE);
+	memset(pwol_mask, 0, B44_PMASK_SIZE);
+	plen1 = b44_magic_pattern(bp->dev->dev_addr, pwol_pattern, pwol_mask,
+				  ETH_HLEN);
+
+   	bwfilter_table(bp, pwol_pattern, B44_PATTERN_SIZE,
+		       B44_PATTERN_BASE + B44_PATTERN_SIZE);
+  	bwfilter_table(bp, pwol_mask, B44_PMASK_SIZE,
+		       B44_PMASK_BASE + B44_PMASK_SIZE);
+
+	/* Ipv6 magic packet pattern - pattern 2 */
+	memset(pwol_pattern, 0, B44_PATTERN_SIZE);
+	memset(pwol_mask, 0, B44_PMASK_SIZE);
+	plen2 = b44_magic_pattern(bp->dev->dev_addr, pwol_pattern, pwol_mask,
+				  B44_ETHIPV6UDP_HLEN);
+
+   	bwfilter_table(bp, pwol_pattern, B44_PATTERN_SIZE,
+		       B44_PATTERN_BASE + B44_PATTERN_SIZE + B44_PATTERN_SIZE);
+  	bwfilter_table(bp, pwol_mask, B44_PMASK_SIZE,
+		       B44_PMASK_BASE + B44_PMASK_SIZE + B44_PMASK_SIZE);
+
+	kfree(pwol_pattern);
+
+	/* set these pattern's lengths: one less than each real length */
+	val = plen0 | (plen1 << 8) | (plen2 << 16) | WKUP_LEN_ENABLE_THREE;
+	bw32(bp, B44_WKUP_LEN, val);
+
+	/* enable wakeup pattern matching */
+	val = br32(bp, B44_DEVCTRL);
+	bw32(bp, B44_DEVCTRL, val | DEVCTRL_PFE);
+
+}
+
+static void b44_setup_wol(struct b44 *bp)
+{
+	u32 val;
+	u16 pmval;
+
+	bw32(bp, B44_RXCONFIG, RXCONFIG_ALLMULTI);
+
+	if (bp->flags & B44_FLAG_B0_ANDLATER) {
+
+		bw32(bp, B44_WKUP_LEN, WKUP_LEN_DISABLE);
+
+		val = bp->dev->dev_addr[2] << 24 |
+			bp->dev->dev_addr[3] << 16 |
+			bp->dev->dev_addr[4] << 8 |
+			bp->dev->dev_addr[5];
+		bw32(bp, B44_ADDR_LO, val);
+
+		val = bp->dev->dev_addr[0] << 8 |
+			bp->dev->dev_addr[1];
+		bw32(bp, B44_ADDR_HI, val);
+
+		val = br32(bp, B44_DEVCTRL);
+		bw32(bp, B44_DEVCTRL, val | DEVCTRL_MPM | DEVCTRL_PFE);
+
+ 	} else {
+ 		b44_setup_pseudo_magicp(bp);
+ 	}
+
+	val = br32(bp, B44_SBTMSLOW);
+	bw32(bp, B44_SBTMSLOW, val | SBTMSLOW_PE);
+
+	pci_read_config_word(bp->pdev, SSB_PMCSR, &pmval);
+	pci_write_config_word(bp->pdev, SSB_PMCSR, pmval | SSB_PE);
+
+}
+
 static int b44_close(struct net_device *dev)
 {
 	struct b44 *bp = netdev_priv(dev);
@@ -1471,6 +1632,11 @@ #endif
 
 	netif_poll_enable(dev);
 
+	if (bp->flags & B44_FLAG_WOL_ENABLE) {
+		b44_init_hw(bp, B44_PARTIAL_RESET);
+		b44_setup_wol(bp);
+	}
+
 	b44_free_consistent(bp);
 
 	return 0;
@@ -1543,18 +1709,19 @@ static void __b44_set_rx_mode(struct net
 		bw32(bp, B44_RXCONFIG, val);
 	} else {
 		unsigned char zero[6] = {0, 0, 0, 0, 0, 0};
-		int i = 0;
+		int i = 1;
 
 		__b44_set_mac_addr(bp);
 
-		if (dev->flags & IFF_ALLMULTI)
+		if ((dev->flags & IFF_ALLMULTI) ||
+		    (dev->mc_count > B44_MCAST_TABLE_SIZE))
 			val |= RXCONFIG_ALLMULTI;
 		else
 			i = __b44_load_mcast(bp, dev);
-		
-		for (; i < 64; i++) {
-			__b44_cam_write(bp, zero, i);			
-		}
+
+		for (; i < 64; i++)
+			__b44_cam_write(bp, zero, i);
+
 		bw32(bp, B44_RXCONFIG, val);
         	val = br32(bp, B44_CAM_CTRL);
 	        bw32(bp, B44_CAM_CTRL, val | CAM_CTRL_ENABLE);
@@ -1616,8 +1783,6 @@ static int b44_get_settings(struct net_d
 {
 	struct b44 *bp = netdev_priv(dev);
 
-	if (!netif_running(dev))
-		return -EAGAIN;
 	cmd->supported = (SUPPORTED_Autoneg);
 	cmd->supported |= (SUPPORTED_100baseT_Half |
 			  SUPPORTED_100baseT_Full |
@@ -1645,6 +1810,12 @@ static int b44_get_settings(struct net_d
 		XCVR_INTERNAL : XCVR_EXTERNAL;
 	cmd->autoneg = (bp->flags & B44_FLAG_FORCE_LINK) ?
 		AUTONEG_DISABLE : AUTONEG_ENABLE;
+	if (cmd->autoneg == AUTONEG_ENABLE)
+		cmd->advertising |= ADVERTISED_Autoneg;
+	if (!netif_running(dev)){
+		cmd->speed = 0;
+		cmd->duplex = 0xff;
+	}
 	cmd->maxtxpkt = 0;
 	cmd->maxrxpkt = 0;
 	return 0;
@@ -1654,9 +1825,6 @@ static int b44_set_settings(struct net_d
 {
 	struct b44 *bp = netdev_priv(dev);
 
-	if (!netif_running(dev))
-		return -EAGAIN;
-
 	/* We do not support gigabit. */
 	if (cmd->autoneg == AUTONEG_ENABLE) {
 		if (cmd->advertising &
@@ -1673,28 +1841,39 @@ static int b44_set_settings(struct net_d
 	spin_lock_irq(&bp->lock);
 
 	if (cmd->autoneg == AUTONEG_ENABLE) {
-		bp->flags &= ~B44_FLAG_FORCE_LINK;
-		bp->flags &= ~(B44_FLAG_ADV_10HALF |
+		bp->flags &= ~(B44_FLAG_FORCE_LINK |
+			       B44_FLAG_100_BASE_T |
+			       B44_FLAG_FULL_DUPLEX |
+			       B44_FLAG_ADV_10HALF |
 			       B44_FLAG_ADV_10FULL |
 			       B44_FLAG_ADV_100HALF |
 			       B44_FLAG_ADV_100FULL);
-		if (cmd->advertising & ADVERTISE_10HALF)
-			bp->flags |= B44_FLAG_ADV_10HALF;
-		if (cmd->advertising & ADVERTISE_10FULL)
-			bp->flags |= B44_FLAG_ADV_10FULL;
-		if (cmd->advertising & ADVERTISE_100HALF)
-			bp->flags |= B44_FLAG_ADV_100HALF;
-		if (cmd->advertising & ADVERTISE_100FULL)
-			bp->flags |= B44_FLAG_ADV_100FULL;
+		if (cmd->advertising == 0) {
+			bp->flags |= (B44_FLAG_ADV_10HALF |
+				      B44_FLAG_ADV_10FULL |
+				      B44_FLAG_ADV_100HALF |
+				      B44_FLAG_ADV_100FULL);
+		} else {
+			if (cmd->advertising & ADVERTISED_10baseT_Half)
+				bp->flags |= B44_FLAG_ADV_10HALF;
+			if (cmd->advertising & ADVERTISED_10baseT_Full)
+				bp->flags |= B44_FLAG_ADV_10FULL;
+			if (cmd->advertising & ADVERTISED_100baseT_Half)
+				bp->flags |= B44_FLAG_ADV_100HALF;
+			if (cmd->advertising & ADVERTISED_100baseT_Full)
+				bp->flags |= B44_FLAG_ADV_100FULL;
+		}
 	} else {
 		bp->flags |= B44_FLAG_FORCE_LINK;
+		bp->flags &= ~(B44_FLAG_100_BASE_T | B44_FLAG_FULL_DUPLEX);
 		if (cmd->speed == SPEED_100)
 			bp->flags |= B44_FLAG_100_BASE_T;
 		if (cmd->duplex == DUPLEX_FULL)
 			bp->flags |= B44_FLAG_FULL_DUPLEX;
 	}
 
-	b44_setup_phy(bp);
+	if (netif_running(dev))
+		b44_setup_phy(bp);
 
 	spin_unlock_irq(&bp->lock);
 
@@ -1730,12 +1909,12 @@ static int b44_set_ringparam(struct net_
 
 	b44_halt(bp);
 	b44_init_rings(bp);
-	b44_init_hw(bp);
+	b44_init_hw(bp, B44_FULL_RESET);
 	netif_wake_queue(bp->dev);
 	spin_unlock_irq(&bp->lock);
 
 	b44_enable_ints(bp);
-	
+
 	return 0;
 }
 
@@ -1773,14 +1952,14 @@ static int b44_set_pauseparam(struct net
 	if (bp->flags & B44_FLAG_PAUSE_AUTO) {
 		b44_halt(bp);
 		b44_init_rings(bp);
-		b44_init_hw(bp);
+		b44_init_hw(bp, B44_FULL_RESET);
 	} else {
 		__b44_set_flow_ctrl(bp, bp->flags);
 	}
 	spin_unlock_irq(&bp->lock);
 
 	b44_enable_ints(bp);
-	
+
 	return 0;
 }
 
@@ -1815,12 +1994,40 @@ static void b44_get_ethtool_stats(struct
 	spin_unlock_irq(&bp->lock);
 }
 
-static struct ethtool_ops b44_ethtool_ops = {
+static void b44_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	struct b44 *bp = netdev_priv(dev);
+
+	wol->supported = WAKE_MAGIC;
+	if (bp->flags & B44_FLAG_WOL_ENABLE)
+		wol->wolopts = WAKE_MAGIC;
+	else
+		wol->wolopts = 0;
+	memset(&wol->sopass, 0, sizeof(wol->sopass));
+}
+
+static int b44_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	struct b44 *bp = netdev_priv(dev);
+
+	spin_lock_irq(&bp->lock);
+	if (wol->wolopts & WAKE_MAGIC)
+		bp->flags |= B44_FLAG_WOL_ENABLE;
+	else
+		bp->flags &= ~B44_FLAG_WOL_ENABLE;
+	spin_unlock_irq(&bp->lock);
+
+	return 0;
+}
+
+static const struct ethtool_ops b44_ethtool_ops = {
 	.get_drvinfo		= b44_get_drvinfo,
 	.get_settings		= b44_get_settings,
 	.set_settings		= b44_set_settings,
 	.nway_reset		= b44_nway_reset,
 	.get_link		= ethtool_op_get_link,
+	.get_wol		= b44_get_wol,
+	.set_wol		= b44_set_wol,
 	.get_ringparam		= b44_get_ringparam,
 	.set_ringparam		= b44_set_ringparam,
 	.get_pauseparam		= b44_get_pauseparam,
@@ -1853,10 +2060,10 @@ out:
 static int b44_read_eeprom(struct b44 *bp, u8 *data)
 {
 	long i;
-	u16 *ptr = (u16 *) data;
+	__le16 *ptr = (__le16 *) data;
 
 	for (i = 0; i < 128; i += 2)
-		ptr[i / 2] = readw(bp->regs + 4096 + i);
+		ptr[i / 2] = cpu_to_le16(readw(bp->regs + 4096 + i));
 
 	return 0;
 }
@@ -1876,6 +2083,12 @@ static int __devinit b44_get_invariants(
 	bp->dev->dev_addr[3] = eeprom[80];
 	bp->dev->dev_addr[4] = eeprom[83];
 	bp->dev->dev_addr[5] = eeprom[82];
+
+	if (!is_valid_ether_addr(&bp->dev->dev_addr[0])){
+		printk(KERN_ERR PFX "Invalid MAC address found in EEPROM\n");
+		return -EINVAL;
+	}
+
 	memcpy(bp->dev->perm_addr, bp->dev->dev_addr, bp->dev->addr_len);
 
 	bp->phy_addr = eeprom[90] & 0x1f;
@@ -1890,9 +2103,13 @@ static int __devinit b44_get_invariants(
 	bp->core_unit = ssb_core_unit(bp);
 	bp->dma_offset = SB_PCI_DMA;
 
-	/* XXX - really required? 
+	/* XXX - really required?
 	   bp->flags |= B44_FLAG_BUGGY_TXPTR;
          */
+
+ 	if (ssb_get_core_rev(bp) >= 7)
+ 		bp->flags |= B44_FLAG_B0_ANDLATER;
+
 out:
 	return err;
 }
@@ -1911,13 +2128,14 @@ static int __devinit b44_init_one(struct
 
 	err = pci_enable_device(pdev);
 	if (err) {
-		printk(KERN_ERR PFX "Cannot enable PCI device, "
+		dev_err(&pdev->dev, "Cannot enable PCI device, "
 		       "aborting.\n");
 		return err;
 	}
 
 	if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM)) {
-		printk(KERN_ERR PFX "Cannot find proper PCI device "
+		dev_err(&pdev->dev,
+			"Cannot find proper PCI device "
 		       "base address, aborting.\n");
 		err = -ENODEV;
 		goto err_out_disable_pdev;
@@ -1925,24 +2143,22 @@ static int __devinit b44_init_one(struct
 
 	err = pci_request_regions(pdev, DRV_MODULE_NAME);
 	if (err) {
-		printk(KERN_ERR PFX "Cannot obtain PCI resources, "
-		       "aborting.\n");
+		dev_err(&pdev->dev,
+			"Cannot obtain PCI resources, aborting.\n");
 		goto err_out_disable_pdev;
 	}
 
 	pci_set_master(pdev);
 
-	err = pci_set_dma_mask(pdev, (u64) B44_DMA_MASK);
+	err = pci_set_dma_mask(pdev, (u64) DMA_30BIT_MASK);
 	if (err) {
-		printk(KERN_ERR PFX "No usable DMA configuration, "
-		       "aborting.\n");
+		dev_err(&pdev->dev, "No usable DMA configuration, aborting.\n");
 		goto err_out_free_res;
 	}
-	
-	err = pci_set_consistent_dma_mask(pdev, (u64) B44_DMA_MASK);
+
+	err = pci_set_consistent_dma_mask(pdev, (u64) DMA_30BIT_MASK);
 	if (err) {
-		printk(KERN_ERR PFX "No usable DMA configuration, "
-		       "aborting.\n");
+		dev_err(&pdev->dev, "No usable DMA configuration, aborting.\n");
 		goto err_out_free_res;
 	}
 
@@ -1951,7 +2167,7 @@ static int __devinit b44_init_one(struct
 
 	dev = alloc_etherdev(sizeof(*bp));
 	if (!dev) {
-		printk(KERN_ERR PFX "Etherdev alloc failed, aborting.\n");
+		dev_err(&pdev->dev, "Etherdev alloc failed, aborting.\n");
 		err = -ENOMEM;
 		goto err_out_free_res;
 	}
@@ -1972,8 +2188,7 @@ static int __devinit b44_init_one(struct
 
 	bp->regs = ioremap(b44reg_base, b44reg_len);
 	if (bp->regs == 0UL) {
-		printk(KERN_ERR PFX "Cannot map device registers, "
-		       "aborting.\n");
+		dev_err(&pdev->dev, "Cannot map device registers, aborting.\n");
 		err = -ENOMEM;
 		goto err_out_free_dev;
 	}
@@ -2003,8 +2218,8 @@ #endif
 
 	err = b44_get_invariants(bp);
 	if (err) {
-		printk(KERN_ERR PFX "Problem fetching invariants of chip, "
-		       "aborting.\n");
+		dev_err(&pdev->dev,
+			"Problem fetching invariants of chip, aborting.\n");
 		goto err_out_iounmap;
 	}
 
@@ -2024,8 +2239,7 @@ #endif
 
 	err = register_netdev(dev);
 	if (err) {
-		printk(KERN_ERR PFX "Cannot register net device, "
-		       "aborting.\n");
+		dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
 		goto err_out_iounmap;
 	}
 
@@ -2033,6 +2247,11 @@ #endif
 
 	pci_save_state(bp->pdev);
 
+	/* Chip reset provides power to the b44 MAC & PCI cores, which
+	 * is necessary for MAC register access.
+	 */
+	b44_chip_reset(bp);
+
 	printk(KERN_INFO "%s: Broadcom 4400 10/100BaseT Ethernet ", dev->name);
 	for (i = 0; i < 6; i++)
 		printk("%2.2x%c", dev->dev_addr[i],
@@ -2078,16 +2297,20 @@ static int b44_suspend(struct pci_dev *p
 
 	del_timer_sync(&bp->timer);
 
-	spin_lock_irq(&bp->lock); 
+	spin_lock_irq(&bp->lock);
 
 	b44_halt(bp);
-	netif_carrier_off(bp->dev); 
+	netif_carrier_off(bp->dev);
 	netif_device_detach(bp->dev);
 	b44_free_rings(bp);
 
 	spin_unlock_irq(&bp->lock);
 
 	free_irq(dev->irq, dev);
+	if (bp->flags & B44_FLAG_WOL_ENABLE) {
+		b44_init_hw(bp, B44_PARTIAL_RESET);
+		b44_setup_wol(bp);
+	}
 	pci_disable_device(pdev);
 	return 0;
 }
@@ -2096,21 +2319,32 @@ static int b44_resume(struct pci_dev *pd
 {
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct b44 *bp = netdev_priv(dev);
+	int rc = 0;
 
 	pci_restore_state(pdev);
-	pci_enable_device(pdev);
+	rc = pci_enable_device(pdev);
+	if (rc) {
+		printk(KERN_ERR PFX "%s: pci_enable_device failed\n",
+			dev->name);
+		return rc;
+	}
+
 	pci_set_master(pdev);
 
 	if (!netif_running(dev))
 		return 0;
 
-	if (request_irq(dev->irq, b44_interrupt, SA_SHIRQ, dev->name, dev))
+	rc = request_irq(dev->irq, b44_interrupt, IRQF_SHARED, dev->name, dev);
+	if (rc) {
 		printk(KERN_ERR PFX "%s: request_irq failed\n", dev->name);
+		pci_disable_device(pdev);
+		return rc;
+	}
 
 	spin_lock_irq(&bp->lock);
 
 	b44_init_rings(bp);
-	b44_init_hw(bp);
+	b44_init_hw(bp, B44_FULL_RESET);
 	netif_device_attach(bp->dev);
 	spin_unlock_irq(&bp->lock);
 
@@ -2139,7 +2373,7 @@ static int __init b44_init(void)
 	dma_desc_align_mask = ~(dma_desc_align_size - 1);
 	dma_desc_sync_size = max_t(unsigned int, dma_desc_align_size, sizeof(struct dma_desc));
 
-	return pci_module_init(&b44_driver);
+	return pci_register_driver(&b44_driver);
 }
 
 static void __exit b44_cleanup(void)

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

Can you also test the following patch?
I think there's a bug in b44 that is doesn't properly discard
shared IRQs, so it might possibly generate a NAPI storm, dunno.
Worth a try.

Index: linux-2.6.22-rc3/drivers/net/b44.c
===================================================================
--- linux-2.6.22-rc3.orig/drivers/net/b44.c	2007-05-27 23:01:44.000000000 +0200
+++ linux-2.6.22-rc3/drivers/net/b44.c	2007-05-28 12:48:27.000000000 +0200
@@ -911,6 +911,8 @@ static irqreturn_t b44_interrupt(int irq
 	spin_lock(&bp->lock);
 
 	istat = br32(bp, B44_ISTAT);
+	if (istat == 0xFFFFFFFF)
+		goto out; /* Shared IRQ not for us */
 	imask = br32(bp, B44_IMASK);
 
 	/* The interrupt mask register controls which interrupt bits
@@ -942,6 +944,7 @@ irq_ack:
 		bw32(bp, B44_ISTAT, istat);
 		br32(bp, B44_ISTAT);
 	}
+out:
 	spin_unlock(&bp->lock);
 	return IRQ_RETVAL(handled);
 }


-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

On Monday 28 May 2007, Michael Buesch wrote:
> Can you give 2.6.16 a try? The diff is not that big and we might
> be able to find out what broke if you find out 2.6.16 works.
> You can also try later kernels like .17, .18, .19 to further
> reduce the patch. (You could also git-bisect, if you have the time).
>
I did some testing and compiled some kernels and here are the results:

I was able to find out what causes the problems for me.  I did build two 
2.6.21.3 kernels, and one does work fine and the other doesn't.

This is a diff of the kernel configs I used:

--- /usr/src/linux-2.6.21.3-oldconfig1/.config  2007-05-28 13:41:15.000000000 
+0200
+++ /usr/src/linux-2.6.21.3/.config     2007-05-28 14:46:08.000000000 +0200
@@ -1,7 +1,7 @@
 #
 # Automatically generated make config: don't edit
 # Linux kernel version: 2.6.21.3
-# Mon May 28 13:41:15 2007
+# Mon May 28 14:46:09 2007
 #
 CONFIG_X86_32=y
 CONFIG_GENERIC_TIME=y
@@ -32,7 +32,7 @@
 #
 # General setup
 #
-CONFIG_LOCALVERSION="-oldconfig1"
+CONFIG_LOCALVERSION=""
 CONFIG_LOCALVERSION_AUTO=y
 CONFIG_SWAP=y
 CONFIG_SYSVIPC=y
@@ -108,9 +108,9 @@
 #
 # Processor type and features
 #
-# CONFIG_TICK_ONESHOT is not set
+CONFIG_TICK_ONESHOT=y
 # CONFIG_NO_HZ is not set
-# CONFIG_HIGH_RES_TIMERS is not set
+CONFIG_HIGH_RES_TIMERS=y
 # CONFIG_SMP is not set
 CONFIG_X86_PC=y
 # CONFIG_X86_ELAN is not set

The -oldconfig1 is the kernel that had no problems and the other shows the b44 
problem. So if High Resolution Timer Support is disabled everything works 
fine and if I enable it the problems do appear again.

I didn't test this on my 2.6.22-rc3 kernel yet, but I guess disabling High 
Resolution Timer Support will also solve the problem there.

The older kernels I tried also work perfectly fine and they didn't have the 
High Resolution Timer Support yet.

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

On Monday 28 May 2007, Michael Buesch wrote:
> Can you also test the following patch?
> I think there's a bug in b44 that is doesn't properly discard
> shared IRQs, so it might possibly generate a NAPI storm, dunno.
> Worth a try.
>
> Index: linux-2.6.22-rc3/drivers/net/b44.c
> ===================================================================
> --- linux-2.6.22-rc3.orig/drivers/net/b44.c	2007-05-27 23:01:44.000000000
> +0200 +++ linux-2.6.22-rc3/drivers/net/b44.c	2007-05-28 12:48:27.000000000
> +0200 @@ -911,6 +911,8 @@ static irqreturn_t b44_interrupt(int irq
>  	spin_lock(&bp->lock);
>
>  	istat = br32(bp, B44_ISTAT);
> +	if (istat == 0xFFFFFFFF)
> +		goto out; /* Shared IRQ not for us */
>  	imask = br32(bp, B44_IMASK);
>
>  	/* The interrupt mask register controls which interrupt bits
> @@ -942,6 +944,7 @@ irq_ack:
>  		bw32(bp, B44_ISTAT, istat);
>  		br32(bp, B44_ISTAT);
>  	}
> +out:
>  	spin_unlock(&bp->lock);
>  	return IRQ_RETVAL(handled);
>  }

I did try this patch on a affected kernel, but I didn't notice any big 
difference. Perhaps the kernel is a bit less slow during the test, but It's 
hard to tell.

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

On Monday 28 May 2007 16:12:12 Maximilian Engelhardt wrote:
> On Monday 28 May 2007, Michael Buesch wrote:
> > Can you also test the following patch?
> > I think there's a bug in b44 that is doesn't properly discard
> > shared IRQs, so it might possibly generate a NAPI storm, dunno.
> > Worth a try.
> >
> > Index: linux-2.6.22-rc3/drivers/net/b44.c
> > ===================================================================
> > --- linux-2.6.22-rc3.orig/drivers/net/b44.c	2007-05-27 23:01:44.000000000
> > +0200 +++ linux-2.6.22-rc3/drivers/net/b44.c	2007-05-28 12:48:27.000000000
> > +0200 @@ -911,6 +911,8 @@ static irqreturn_t b44_interrupt(int irq
> >  	spin_lock(&bp->lock);
> >
> >  	istat = br32(bp, B44_ISTAT);
> > +	if (istat == 0xFFFFFFFF)
> > +		goto out; /* Shared IRQ not for us */
> >  	imask = br32(bp, B44_IMASK);
> >
> >  	/* The interrupt mask register controls which interrupt bits
> > @@ -942,6 +944,7 @@ irq_ack:
> >  		bw32(bp, B44_ISTAT, istat);
> >  		br32(bp, B44_ISTAT);
> >  	}
> > +out:
> >  	spin_unlock(&bp->lock);
> >  	return IRQ_RETVAL(handled);
> >  }
> 
> I did try this patch on a affected kernel, but I didn't notice any big 
> difference. Perhaps the kernel is a bit less slow during the test, but It's 
> hard to tell.

Ok, but anyway. I think this is a bug and needs to be fixed this way. Gary?

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

On Monday 28 May 2007 16:09:46 Maximilian Engelhardt wrote:
> On Monday 28 May 2007, Michael Buesch wrote:
> > Can you give 2.6.16 a try? The diff is not that big and we might
> > be able to find out what broke if you find out 2.6.16 works.
> > You can also try later kernels like .17, .18, .19 to further
> > reduce the patch. (You could also git-bisect, if you have the time).
> >
> I did some testing and compiled some kernels and here are the results:
> 
> I was able to find out what causes the problems for me.  I did build two 
> 2.6.21.3 kernels, and one does work fine and the other doesn't.
> 
> This is a diff of the kernel configs I used:
> 
> --- /usr/src/linux-2.6.21.3-oldconfig1/.config  2007-05-28 13:41:15.000000000 
> +0200
> +++ /usr/src/linux-2.6.21.3/.config     2007-05-28 14:46:08.000000000 +0200
> @@ -1,7 +1,7 @@
>  #
>  # Automatically generated make config: don't edit
>  # Linux kernel version: 2.6.21.3
> -# Mon May 28 13:41:15 2007
> +# Mon May 28 14:46:09 2007
>  #
>  CONFIG_X86_32=y
>  CONFIG_GENERIC_TIME=y
> @@ -32,7 +32,7 @@
>  #
>  # General setup
>  #
> -CONFIG_LOCALVERSION="-oldconfig1"
> +CONFIG_LOCALVERSION=""
>  CONFIG_LOCALVERSION_AUTO=y
>  CONFIG_SWAP=y
>  CONFIG_SYSVIPC=y
> @@ -108,9 +108,9 @@
>  #
>  # Processor type and features
>  #
> -# CONFIG_TICK_ONESHOT is not set
> +CONFIG_TICK_ONESHOT=y
>  # CONFIG_NO_HZ is not set
> -# CONFIG_HIGH_RES_TIMERS is not set
> +CONFIG_HIGH_RES_TIMERS=y
>  # CONFIG_SMP is not set
>  CONFIG_X86_PC=y
>  # CONFIG_X86_ELAN is not set
> 
> The -oldconfig1 is the kernel that had no problems and the other shows the b44 
> problem. So if High Resolution Timer Support is disabled everything works 
> fine and if I enable it the problems do appear again.
> 
> I didn't test this on my 2.6.22-rc3 kernel yet, but I guess disabling High 
> Resolution Timer Support will also solve the problem there.
> 
> The older kernels I tried also work perfectly fine and they didn't have the 
> High Resolution Timer Support yet.

So, that's interesting, indeed.
Any idea what's going on, someone? Thomas?

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Thomas Gleixner]

On Mon, 2007-05-28 at 17:14 +0200, Michael Buesch wrote:
> > The -oldconfig1 is the kernel that had no problems and the other shows the b44 
> > problem. So if High Resolution Timer Support is disabled everything works 
> > fine and if I enable it the problems do appear again.
> > 
> > I didn't test this on my 2.6.22-rc3 kernel yet, but I guess disabling High 
> > Resolution Timer Support will also solve the problem there.
> > 
> > The older kernels I tried also work perfectly fine and they didn't have the 
> > High Resolution Timer Support yet.
> 
> So, that's interesting, indeed.
> Any idea what's going on, someone? Thomas?

Not off the top of my head.

Maximilian, does the kernel work otherwise (I mean aside of the b44
driver) ? 

Can you please keep CONFIG_HIGH_RES_TIMERS and CONFIG_NOHZ and try the
following combinations on the kernel command line:

1) highres=off nohz=off (should be the same as your working config)
2) highres=off
3) nohz=off

Michael, is anything in the b44 driver timer driven ?

Thanks,

	tglx

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

On Monday 28 May 2007 17:32:51 Thomas Gleixner wrote:
> On Mon, 2007-05-28 at 17:14 +0200, Michael Buesch wrote:
> > > The -oldconfig1 is the kernel that had no problems and the other shows the b44 
> > > problem. So if High Resolution Timer Support is disabled everything works 
> > > fine and if I enable it the problems do appear again.
> > > 
> > > I didn't test this on my 2.6.22-rc3 kernel yet, but I guess disabling High 
> > > Resolution Timer Support will also solve the problem there.
> > > 
> > > The older kernels I tried also work perfectly fine and they didn't have the 
> > > High Resolution Timer Support yet.
> > 
> > So, that's interesting, indeed.
> > Any idea what's going on, someone? Thomas?
> 
> Not off the top of my head.
> 
> Maximilian, does the kernel work otherwise (I mean aside of the b44
> driver) ? 
> 
> Can you please keep CONFIG_HIGH_RES_TIMERS and CONFIG_NOHZ and try the
> following combinations on the kernel command line:
> 
> 1) highres=off nohz=off (should be the same as your working config)
> 2) highres=off
> 3) nohz=off
> 
> Michael, is anything in the b44 driver timer driven ?

NAPI perhaps. I don't know. Steve may know.

The only timer in b44 is to update stats every second. I doubt
that this can cause such an issue. It's not involved in transmission.

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 1446 bytes --]

On Monday 28 May 2007, Thomas Gleixner wrote:
> On Mon, 2007-05-28 at 17:14 +0200, Michael Buesch wrote:
> > > The -oldconfig1 is the kernel that had no problems and the other shows
> > > the b44 problem. So if High Resolution Timer Support is disabled
> > > everything works fine and if I enable it the problems do appear again.
> > >
> > > I didn't test this on my 2.6.22-rc3 kernel yet, but I guess disabling
> > > High Resolution Timer Support will also solve the problem there.
> > >
> > > The older kernels I tried also work perfectly fine and they didn't have
> > > the High Resolution Timer Support yet.
> >
> > So, that's interesting, indeed.
> > Any idea what's going on, someone? Thomas?
>
> Not off the top of my head.
>
> Maximilian, does the kernel work otherwise (I mean aside of the b44
> driver) ?
>
> Can you please keep CONFIG_HIGH_RES_TIMERS and CONFIG_NOHZ and try the
> following combinations on the kernel command line:
>
> 1) highres=off nohz=off (should be the same as your working config)
> 2) highres=off
> 3) nohz=off

I tested this with my 2.6.22-rc3 kernel, here are the results:

without any special boot parameters: problem does appear
highres=off nohz=off: problem does not appear
highres=off: problem does not appear
nohz=off: problem does appear

I additionally built my 2.6.22-rc2-mm1 kernel without High Resolution Timer, 
but the high ping problem is still there.

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Thomas Gleixner]

On Mon, 2007-05-28 at 19:44 +0200, Maximilian Engelhardt wrote:
> > Can you please keep CONFIG_HIGH_RES_TIMERS and CONFIG_NOHZ and try the
> > following combinations on the kernel command line:
> >
> > 1) highres=off nohz=off (should be the same as your working config)
> > 2) highres=off
> > 3) nohz=off
> 
> I tested this with my 2.6.22-rc3 kernel, here are the results:
> 
> without any special boot parameters: problem does appear
> highres=off nohz=off: problem does not appear
> highres=off: problem does not appear
> nohz=off: problem does appear

Is there any other strange behavior of the high res enabled kernel than
the b44 problem ?

> I additionally built my 2.6.22-rc2-mm1 kernel without High Resolution Timer, 
> but the high ping problem is still there.

Hmm, that's mysterious. Wild guess is that highres exposes the hidden
"feature" in a different way than rc2-mm1 does.

	tglx

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 1855 bytes --]

On Monday 28 May 2007, Thomas Gleixner wrote:
> On Mon, 2007-05-28 at 19:44 +0200, Maximilian Engelhardt wrote:
> > > Can you please keep CONFIG_HIGH_RES_TIMERS and CONFIG_NOHZ and try the
> > > following combinations on the kernel command line:
> > >
> > > 1) highres=off nohz=off (should be the same as your working config)
> > > 2) highres=off
> > > 3) nohz=off
> >
> > I tested this with my 2.6.22-rc3 kernel, here are the results:
> >
> > without any special boot parameters: problem does appear
> > highres=off nohz=off: problem does not appear
> > highres=off: problem does not appear
> > nohz=off: problem does appear
>
> Is there any other strange behavior of the high res enabled kernel than
> the b44 problem ?

I didn't notice anything.

>
> > I additionally built my 2.6.22-rc2-mm1 kernel without High Resolution
> > Timer, but the high ping problem is still there.
>
> Hmm, that's mysterious. Wild guess is that highres exposes the hidden
> "feature" in a different way than rc2-mm1 does.

I think the bug in 2.6.21/22-rc3 is a different one that the one in 
2.6.22-rc2-mm1, but that's also only a wild guess :)

I'll explain this a bit:
In 2.6.21/22-rc3 is the same b44 driver that has been in the stock kernels for 
some time. With this driver and High Resolution Timer turned on I get 
problems using iperf. The problems are that the systems becomes really slow 
and unresponsive.  Michael Buesch thought this could be an IRQ storm which 
sounds logical to me. This bug did never happen to me before I startet the 
iperf test.

The other issue happens only with 2.6.22-rc2-mm1 which includes the b44 ssb 
spilt. It's independed wether High Resolution Timer is turned on or off I 
always get very varying and high ping times. The iperf-test doesn't show the 
problems from 2.6.21/22-rc3.

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Thomas Gleixner]

On Mon, 2007-05-28 at 22:55 +0200, Maximilian Engelhardt wrote:
> > > I additionally built my 2.6.22-rc2-mm1 kernel without High Resolution
> > > Timer, but the high ping problem is still there.
> >
> > Hmm, that's mysterious. Wild guess is that highres exposes the hidden
> > "feature" in a different way than rc2-mm1 does.
> 
> I think the bug in 2.6.21/22-rc3 is a different one that the one in 
> 2.6.22-rc2-mm1, but that's also only a wild guess :)
> 
> I'll explain this a bit:
> In 2.6.21/22-rc3 is the same b44 driver that has been in the stock kernels for 
> some time. With this driver and High Resolution Timer turned on I get 
> problems using iperf. The problems are that the systems becomes really slow 
> and unresponsive.  Michael Buesch thought this could be an IRQ storm which 
> sounds logical to me. This bug did never happen to me before I startet the 
> iperf test.

Can you please apply

http://www.tglx.de/projects/hrtimers/2.6.22-rc3/patch-2.6.22-rc3-hrt1.patch

on top of rc3 and check, whether it has any effect on your problem.

> The other issue happens only with 2.6.22-rc2-mm1 which includes the b44 ssb 
> spilt. It's independed wether High Resolution Timer is turned on or off I 
> always get very varying and high ping times. The iperf-test doesn't show the 
> problems from 2.6.21/22-rc3.

Neither with nor without highres ?

	tglx

Re: b44: regression in 2.6.22 (resend)

[Gary Zambrano]

On Mon, 2007-05-28 at 13:55 -0700, Maximilian Engelhardt wrote:
> On Monday 28 May 2007, Thomas Gleixner wrote:
> > On Mon, 2007-05-28 at 19:44 +0200, Maximilian Engelhardt wrote:
> > > > Can you please keep CONFIG_HIGH_RES_TIMERS and CONFIG_NOHZ and try the
> > > > following combinations on the kernel command line:
> > > >
> > > > 1) highres=off nohz=off (should be the same as your working config)
> > > > 2) highres=off
> > > > 3) nohz=off
> > >
> > > I tested this with my 2.6.22-rc3 kernel, here are the results:
> > >
> > > without any special boot parameters: problem does appear
> > > highres=off nohz=off: problem does not appear
> > > highres=off: problem does not appear
> > > nohz=off: problem does appear
> >
> > Is there any other strange behavior of the high res enabled kernel than
> > the b44 problem ?
> 
> I didn't notice anything.
> 
> >
> > > I additionally built my 2.6.22-rc2-mm1 kernel without High Resolution
> > > Timer, but the high ping problem is still there.
> >
> > Hmm, that's mysterious. Wild guess is that highres exposes the hidden
> > "feature" in a different way than rc2-mm1 does.
> 
> I think the bug in 2.6.21/22-rc3 is a different one that the one in 
> 2.6.22-rc2-mm1, but that's also only a wild guess :)
> 
> I'll explain this a bit:
> In 2.6.21/22-rc3 is the same b44 driver that has been in the stock kernels for 
> some time. With this driver and High Resolution Timer turned on I get 
> problems using iperf. The problems are that the systems becomes really slow 
> and unresponsive.  Michael Buesch thought this could be an IRQ storm which 
> sounds logical to me. This bug did never happen to me before I startet the 
> iperf test.


Can you please check to see if you notice anything out of the ordinary
using netperf in place of iperf in your high res timer on/off testbed?

Thanks,
Gary

Re: b44: regression in 2.6.22 (resend)

[Gary Zambrano]

On Mon, 2007-05-28 at 16:55 +0200, Michael Buesch wrote:
> On Monday 28 May 2007 16:12:12 Maximilian Engelhardt wrote:
> > On Monday 28 May 2007, Michael Buesch wrote:
> > > Can you also test the following patch?
> > > I think there's a bug in b44 that is doesn't properly discard
> > > shared IRQs, so it might possibly generate a NAPI storm, dunno.
> > > Worth a try.
> > >
> > > Index: linux-2.6.22-rc3/drivers/net/b44.c
> > > ===================================================================
> > > --- linux-2.6.22-rc3.orig/drivers/net/b44.c	2007-05-27 23:01:44.000000000
> > > +0200 +++ linux-2.6.22-rc3/drivers/net/b44.c	2007-05-28 12:48:27.000000000
> > > +0200 @@ -911,6 +911,8 @@ static irqreturn_t b44_interrupt(int irq
> > >  	spin_lock(&bp->lock);
> > >
> > >  	istat = br32(bp, B44_ISTAT);
> > > +	if (istat == 0xFFFFFFFF)
> > > +		goto out; /* Shared IRQ not for us */
> > >  	imask = br32(bp, B44_IMASK);
> > >
> > >  	/* The interrupt mask register controls which interrupt bits
> > > @@ -942,6 +944,7 @@ irq_ack:
> > >  		bw32(bp, B44_ISTAT, istat);
> > >  		br32(bp, B44_ISTAT);
> > >  	}
> > > +out:
> > >  	spin_unlock(&bp->lock);
> > >  	return IRQ_RETVAL(handled);
> > >  }
> > 
> > I did try this patch on a affected kernel, but I didn't notice any big 
> > difference. Perhaps the kernel is a bit less slow during the test, but It's 
> > hard to tell.
> 
> Ok, but anyway. I think this is a bug and needs to be fixed this way. Gary?
> 

Thanks Michael.
No, I don't think this is a bug and it does not need to be fixed.
Thanks,
Gary

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 2665 bytes --]

On Tuesday 29 May 2007, Gary Zambrano wrote:
> On Mon, 2007-05-28 at 13:55 -0700, Maximilian Engelhardt wrote:
> > On Monday 28 May 2007, Thomas Gleixner wrote:
> > > On Mon, 2007-05-28 at 19:44 +0200, Maximilian Engelhardt wrote:
> > > > > Can you please keep CONFIG_HIGH_RES_TIMERS and CONFIG_NOHZ and try
> > > > > the following combinations on the kernel command line:
> > > > >
> > > > > 1) highres=off nohz=off (should be the same as your working config)
> > > > > 2) highres=off
> > > > > 3) nohz=off
> > > >
> > > > I tested this with my 2.6.22-rc3 kernel, here are the results:
> > > >
> > > > without any special boot parameters: problem does appear
> > > > highres=off nohz=off: problem does not appear
> > > > highres=off: problem does not appear
> > > > nohz=off: problem does appear
> > >
> > > Is there any other strange behavior of the high res enabled kernel than
> > > the b44 problem ?
> >
> > I didn't notice anything.
> >
> > > > I additionally built my 2.6.22-rc2-mm1 kernel without High Resolution
> > > > Timer, but the high ping problem is still there.
> > >
> > > Hmm, that's mysterious. Wild guess is that highres exposes the hidden
> > > "feature" in a different way than rc2-mm1 does.
> >
> > I think the bug in 2.6.21/22-rc3 is a different one that the one in
> > 2.6.22-rc2-mm1, but that's also only a wild guess :)
> >
> > I'll explain this a bit:
> > In 2.6.21/22-rc3 is the same b44 driver that has been in the stock
> > kernels for some time. With this driver and High Resolution Timer turned
> > on I get problems using iperf. The problems are that the systems becomes
> > really slow and unresponsive.  Michael Buesch thought this could be an
> > IRQ storm which sounds logical to me. This bug did never happen to me
> > before I startet the iperf test.
>
> Can you please check to see if you notice anything out of the ordinary
> using netperf in place of iperf in your high res timer on/off testbed?

ok, here are the results, I also had a look at the cpu kernel usage.
'good' means that the kernel responsiveness during the test was as I would 
expect it and I didn't notice any problems.

highres enabled:

netperf: 80%sy 15%si (good)
iperf: not really messureable (bad, problem described above)

highres disabled:

netperf: 80%sy 15%si (good)
iperf:  5%sy 30%hi 15%si (good)


for test tests I did run the following commands:
netperf -l 60 192.168.1.1
iperf -c 192.168.1.1 -r -t 60

I also tried to run iperf without any additional arguments (iperf -c 
192.168.1.1) on the problematic kernel but the result is the same as the 
command I wrote above.

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 1637 bytes --]

On Monday 28 May 2007, Thomas Gleixner wrote:
> On Mon, 2007-05-28 at 22:55 +0200, Maximilian Engelhardt wrote:
> > > > I additionally built my 2.6.22-rc2-mm1 kernel without High Resolution
> > > > Timer, but the high ping problem is still there.
> > >
> > > Hmm, that's mysterious. Wild guess is that highres exposes the hidden
> > > "feature" in a different way than rc2-mm1 does.
> >
> > I think the bug in 2.6.21/22-rc3 is a different one that the one in
> > 2.6.22-rc2-mm1, but that's also only a wild guess :)
> >
> > I'll explain this a bit:
> > In 2.6.21/22-rc3 is the same b44 driver that has been in the stock
> > kernels for some time. With this driver and High Resolution Timer turned
> > on I get problems using iperf. The problems are that the systems becomes
> > really slow and unresponsive.  Michael Buesch thought this could be an
> > IRQ storm which sounds logical to me. This bug did never happen to me
> > before I startet the iperf test.
>
> Can you please apply
>
> http://www.tglx.de/projects/hrtimers/2.6.22-rc3/patch-2.6.22-rc3-hrt1.patch
>
> on top of rc3 and check, whether it has any effect on your problem.
>
The patch didn't change anything.

> > The other issue happens only with 2.6.22-rc2-mm1 which includes the b44
> > ssb spilt. It's independed wether High Resolution Timer is turned on or
> > off I always get very varying and high ping times. The iperf-test doesn't
> > show the problems from 2.6.21/22-rc3.
>
> Neither with nor without highres ?

Yes, it doesn't matter if highres is turned on or off. iperf never showed the 
problem from 2.6.21/22-rc3.

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

On Tuesday 29 May 2007 16:14:35 Gary Zambrano wrote:
> On Mon, 2007-05-28 at 16:55 +0200, Michael Buesch wrote:
> > On Monday 28 May 2007 16:12:12 Maximilian Engelhardt wrote:
> > > On Monday 28 May 2007, Michael Buesch wrote:
> > > > Can you also test the following patch?
> > > > I think there's a bug in b44 that is doesn't properly discard
> > > > shared IRQs, so it might possibly generate a NAPI storm, dunno.
> > > > Worth a try.
> > > >
> > > > Index: linux-2.6.22-rc3/drivers/net/b44.c
> > > > ===================================================================
> > > > --- linux-2.6.22-rc3.orig/drivers/net/b44.c	2007-05-27 23:01:44.000000000
> > > > +0200 +++ linux-2.6.22-rc3/drivers/net/b44.c	2007-05-28 12:48:27.000000000
> > > > +0200 @@ -911,6 +911,8 @@ static irqreturn_t b44_interrupt(int irq
> > > >  	spin_lock(&bp->lock);
> > > >
> > > >  	istat = br32(bp, B44_ISTAT);
> > > > +	if (istat == 0xFFFFFFFF)
> > > > +		goto out; /* Shared IRQ not for us */
> > > >  	imask = br32(bp, B44_IMASK);
> > > >
> > > >  	/* The interrupt mask register controls which interrupt bits
> > > > @@ -942,6 +944,7 @@ irq_ack:
> > > >  		bw32(bp, B44_ISTAT, istat);
> > > >  		br32(bp, B44_ISTAT);
> > > >  	}
> > > > +out:
> > > >  	spin_unlock(&bp->lock);
> > > >  	return IRQ_RETVAL(handled);
> > > >  }
> > > 
> > > I did try this patch on a affected kernel, but I didn't notice any big 
> > > difference. Perhaps the kernel is a bit less slow during the test, but It's 
> > > hard to tell.
> > 
> > Ok, but anyway. I think this is a bug and needs to be fixed this way. Gary?
> > 
> 
> Thanks Michael.
> No, I don't think this is a bug and it does not need to be fixed.

Are you sure? I'm not so sure, because
1) On bcm43xx the reverse engineers told us that the card
   returns 0xFFFFFFFF for no-irq-pending. Since b44 and bcm43xx
   are very similiar in IRQ and DMA I just thought it would
   be the case there, too. Just guessing.
2) PCMCIA cards usually return all-ones if you try to read a
   register of a card that's been removed. So it's good
   practice to check for this and bail out early in the IRQ
   path. Do PCMCIA cards (PC-card, not neccessarily a real
   16bit PCMCIA card) for b44 exist?

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Stephen Hemminger]

I am busy bisecting the real cause. Unfortunately, oprofile doesn't work
on the laptop, and build time sucks...

This how I think the IRQ should work:

--- a/drivers/net/b44.c	2007-05-29 09:47:53.000000000 -0700
+++ b/drivers/net/b44.c	2007-05-29 09:49:50.000000000 -0700
@@ -908,9 +908,11 @@ static irqreturn_t b44_interrupt(int irq
 	u32 istat, imask;
 	int handled = 0;
 
-	spin_lock(&bp->lock);
-
 	istat = br32(bp, B44_ISTAT);
+	if (istat == 0 || istat == ~0)
+		return IRQ_NONE;
+
+	spin_lock(&bp->lock);
 	imask = br32(bp, B44_IMASK);
 
 	/* The interrupt mask register controls which interrupt bits

Re: b44: regression in 2.6.22 (resend)

[Gary Zambrano]

On Tue, 2007-05-29 at 22:45 +0200, Michael Buesch wrote:
> On Tuesday 29 May 2007 16:14:35 Gary Zambrano wrote:
> > On Mon, 2007-05-28 at 16:55 +0200, Michael Buesch wrote:
> > > On Monday 28 May 2007 16:12:12 Maximilian Engelhardt wrote:
> > > > On Monday 28 May 2007, Michael Buesch wrote:
> > > > > Can you also test the following patch?
> > > > > I think there's a bug in b44 that is doesn't properly discard
> > > > > shared IRQs, so it might possibly generate a NAPI storm, dunno.
> > > > > Worth a try.
> > > > >
> > > > > Index: linux-2.6.22-rc3/drivers/net/b44.c
> > > > > ===================================================================
> > > > > --- linux-2.6.22-rc3.orig/drivers/net/b44.c	2007-05-27 23:01:44.000000000
> > > > > +0200 +++ linux-2.6.22-rc3/drivers/net/b44.c	2007-05-28 12:48:27.000000000
> > > > > +0200 @@ -911,6 +911,8 @@ static irqreturn_t b44_interrupt(int irq
> > > > >  	spin_lock(&bp->lock);
> > > > >
> > > > >  	istat = br32(bp, B44_ISTAT);
> > > > > +	if (istat == 0xFFFFFFFF)
> > > > > +		goto out; /* Shared IRQ not for us */
> > > > >  	imask = br32(bp, B44_IMASK);
> > > > >
> > > > >  	/* The interrupt mask register controls which interrupt bits
> > > > > @@ -942,6 +944,7 @@ irq_ack:
> > > > >  		bw32(bp, B44_ISTAT, istat);
> > > > >  		br32(bp, B44_ISTAT);
> > > > >  	}
> > > > > +out:
> > > > >  	spin_unlock(&bp->lock);
> > > > >  	return IRQ_RETVAL(handled);
> > > > >  }
> > > > 
> > > > I did try this patch on a affected kernel, but I didn't notice any big 
> > > > difference. Perhaps the kernel is a bit less slow during the test, but It's 
> > > > hard to tell.
> > > 
> > > Ok, but anyway. I think this is a bug and needs to be fixed this way. Gary?
> > > 
> > 
> > Thanks Michael.
> > No, I don't think this is a bug and it does not need to be fixed.
> 
> Are you sure? I'm not so sure, because
> 1) On bcm43xx the reverse engineers told us that the card
>    returns 0xFFFFFFFF for no-irq-pending. Since b44 and bcm43xx
>    are very similiar in IRQ and DMA I just thought it would
>    be the case there, too. Just guessing.
The b44 interrupt status reg returns a value of 0 if no interrupts are
pending. The b44 uses a mask to determine which bits (events) can
generate device interrupts on the system. If the masked interrupt status
register bits are not asserted, then the b44 will return to the system
with handled = 0. 
So, I think the way the b44 interrupt code is written should be ok and
not a bug. 


> 2) PCMCIA cards usually return all-ones if you try to read a
>    register of a card that's been removed. So it's good
>    practice to check for this and bail out early in the IRQ
>    path. Do PCMCIA cards (PC-card, not neccessarily a real
>    16bit PCMCIA card) for b44 exist?

I do not know of any pccard application of the b44. As far as I know
b44s live on motherboards and in the wireless soc.

Thanks,
Gary

Re: b44: regression in 2.6.22 (resend)

[Gary Zambrano]

On Tue, 2007-05-29 at 18:39 -0400, Jeff Garzik wrote:

> We check for 0xffffffff because that is often how a fault is indicated, 
> when the memory location is read during or immediately after hotplug (or 
> if the PCI bus is truly faulty).  So for most hardware, you see
> 
> tmp = read(irq status)
> if (!tmp)
> 	return irq-none		/* no irq events raised */
> if (tmp == 0xffffffff)
> 	return irq-none		/* hot unplug or h/w fault */
> 
> and the method that determines no interrupt handling is needed.
> 

I guess you are right, but then shouldn't the driver be checking for
faults in other parts of the code too? What if a fault/hotplug occurs
immediately after an interrupt, but before a tx?
Thanks,
Gary

Re: b44: regression in 2.6.22 (resend)

[Jeff Garzik]

Gary Zambrano wrote:
> The b44 interrupt status reg returns a value of 0 if no interrupts are
> pending. The b44 uses a mask to determine which bits (events) can
> generate device interrupts on the system. If the masked interrupt status
> register bits are not asserted, then the b44 will return to the system
> with handled = 0. 
> So, I think the way the b44 interrupt code is written should be ok and
> not a bug. 


This is normal.

We check for 0xffffffff because that is often how a fault is indicated, 
when the memory location is read during or immediately after hotplug (or 
if the PCI bus is truly faulty).  So for most hardware, you see

tmp = read(irq status)
if (!tmp)
	return irq-none		/* no irq events raised */
if (tmp == 0xffffffff)
	return irq-none		/* hot unplug or h/w fault */

and the method that determines no interrupt handling is needed.

Regards,

	Jeff

Re: b44: regression in 2.6.22 (resend)

[Michael Buesch]

On Tuesday 29 May 2007 23:36:51 Gary Zambrano wrote:
> On Tue, 2007-05-29 at 18:39 -0400, Jeff Garzik wrote:
> 
> > We check for 0xffffffff because that is often how a fault is indicated, 
> > when the memory location is read during or immediately after hotplug (or 
> > if the PCI bus is truly faulty).  So for most hardware, you see
> > 
> > tmp = read(irq status)
> > if (!tmp)
> > 	return irq-none		/* no irq events raised */
> > if (tmp == 0xffffffff)
> > 	return irq-none		/* hot unplug or h/w fault */
> > 
> > and the method that determines no interrupt handling is needed.
> > 
> 
> I guess you are right, but then shouldn't the driver be checking for
> faults in other parts of the code too? What if a fault/hotplug occurs
> immediately after an interrupt, but before a tx?
> Thanks,

Well, in general it's not desired (or even possible) to check every
single MMIO access. General practice is to check on entering the IRQ handler
and on values from registers or DMA which could crash the driver.
For example on DMA we get some cookie back from the device in the TX
status report that says which packet this was associated to.
That value is used to look up a table.
In bcm43xx I initialize that to 0 in the driver, which is not a valid value.
Later I check for that. So if the device is unplugged before DMA was
on that value was complete it will recognize it and it won't crash.

In general we can only do our very best to prevent bad sideeffects from
a pull-in-full-operation. We can't do much here anyway. Best we can do
is to _try_ to prevent a crash. It might not always be 100% possible
to do so, though.

-- 
Greetings Michael.

Re: b44: regression in 2.6.22 (resend)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

On Monday 28 May 2007, Thomas Gleixner wrote:
> On Mon, 2007-05-28 at 19:44 +0200, Maximilian Engelhardt wrote:
> > > Can you please keep CONFIG_HIGH_RES_TIMERS and CONFIG_NOHZ and try the
> > > following combinations on the kernel command line:
> > >
> > > 1) highres=off nohz=off (should be the same as your working config)
> > > 2) highres=off
> > > 3) nohz=off
> >
> > I tested this with my 2.6.22-rc3 kernel, here are the results:
> >
> > without any special boot parameters: problem does appear
> > highres=off nohz=off: problem does not appear
> > highres=off: problem does not appear
> > nohz=off: problem does appear
>
> Is there any other strange behavior of the high res enabled kernel than
> the b44 problem ?

I didn't notice anything in the past (as I wrote). But today I did some tests 
for an updated version of the p54 mac80211 wlan driver and I noticed exactly 
the same problem:

when booting with highres=off everything is fine.
But when I boot an highres enabled kernel and I do the iperf-test with the p54 
driver, my systems becomes unresponsive during the test. It seems to be 
exactly the same problem I have with the b44 driver.
So this might not be a bug in the b44 code but a bug somewhere in the linux 
networking code.

I did the test with an 2.6.22-rc3-git4 kernel and the p54 driver built 
external as module. 

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: b44: regression in 2.6.22 (resend)

[Thomas Gleixner]

On Sun, 2007-06-03 at 18:26 +0200, Maximilian Engelhardt wrote:
> > Is there any other strange behavior of the high res enabled kernel than
> > the b44 problem ?
> 
> I didn't notice anything in the past (as I wrote). But today I did some tests 
> for an updated version of the p54 mac80211 wlan driver and I noticed exactly 
> the same problem:
> 
> when booting with highres=off everything is fine.
> But when I boot an highres enabled kernel and I do the iperf-test with the p54 
> driver, my systems becomes unresponsive during the test. It seems to be 
> exactly the same problem I have with the b44 driver.
> So this might not be a bug in the b44 code but a bug somewhere in the linux 
> networking code.
> 
> I did the test with an 2.6.22-rc3-git4 kernel and the p54 driver built 
> external as module. 

Can you look at iperf to figure out, whether it does some weird timer
stuff (high frequency interval timer or such) ? Either check the code or
strace it.

	tglx

Re: b44: regression in 2.6.22 (resend)

[Stephen Hemminger]

On Mon, 04 Jun 2007 08:39:48 +0200
Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:

> On Sun, 2007-06-03 at 18:26 +0200, Maximilian Engelhardt wrote:
> > > Is there any other strange behavior of the high res enabled kernel than
> > > the b44 problem ?
> > 
> > I didn't notice anything in the past (as I wrote). But today I did some tests 
> > for an updated version of the p54 mac80211 wlan driver and I noticed exactly 
> > the same problem:
> > 
> > when booting with highres=off everything is fine.
> > But when I boot an highres enabled kernel and I do the iperf-test with the p54 
> > driver, my systems becomes unresponsive during the test. It seems to be 
> > exactly the same problem I have with the b44 driver.
> > So this might not be a bug in the b44 code but a bug somewhere in the linux 
> > networking code.
> > 
> > I did the test with an 2.6.22-rc3-git4 kernel and the p54 driver built 
> > external as module. 
> 
> Can you look at iperf to figure out, whether it does some weird timer
> stuff (high frequency interval timer or such) ? Either check the code or
> strace it.
> 
> 	tglx
> 
> 


It is the receiver doing a tight loop doing gettimeofday/recv calls.


sendto(-1227715616, 0xc, 3085438964, 0, {...}, 3067249832) = 0
gettimeofday({1180973726, 981615}, NULL) = 0
gettimeofday({1180973726, 981751}, NULL) = 0
futex(0x8055c64, 0x5 /* FUTEX_??? */, 1) = 1
futex(0x8055c90, FUTEX_WAKE, 1) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 982754}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 983790}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 984355}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 984706}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 985111}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 985499}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 986088}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 986436}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 986916}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 987397}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 987872}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 988440}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 988823}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 989314}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 990029}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 990890}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 991803}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 992616}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 993105}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 993585}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 994014}, NULL) = 0
...

recv(4, "45678901234567890123456789012345"..., 8192, 0) = 1448
gettimeofday({1180973757, 172437}, NULL) = 0
recv(4, "23456789012345678901234567890123"..., 8192, 0) = 1448
gettimeofday({1180973757, 172576}, NULL) = 0
recv(4, "01234567890123456789012345678901"..., 8192, 0) = 1632
gettimeofday({1180973757, 172752}, NULL) = 0
recv(4, "", 8192, 0)                    = 0
gettimeofday({1180973757, 172797}, NULL) = 0
gettimeofday({1180973757, 172817}, NULL) = 0
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL)                 = 0
close(4)                     = 0
futex(0x8055d04, 0x5 /* FUTEX_??? */, 1) = 1
futex(0x8055d30, FUTEX_WAKE, 1) = 0
_exit(0)

-- 
Stephen Hemminger <shemminger-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>

Re: b44: regression in 2.6.22 (resend)

[Thomas Gleixner]

On Mon, 2007-06-04 at 09:09 -0700, Stephen Hemminger wrote:
> > > I did the test with an 2.6.22-rc3-git4 kernel and the p54 driver built 
> > > external as module. 
> > 
> > Can you look at iperf to figure out, whether it does some weird timer
> > stuff (high frequency interval timer or such) ? Either check the code or
> > strace it.
>
> It is the receiver doing a tight loop doing gettimeofday/recv calls.
> 
> 
> sendto(-1227715616, 0xc, 3085438964, 0, {...}, 3067249832) = 0
> gettimeofday({1180973726, 981615}, NULL) = 0
> gettimeofday({1180973726, 981751}, NULL) = 0
> futex(0x8055c64, 0x5 /* FUTEX_??? */, 1) = 1
> futex(0x8055c90, FUTEX_WAKE, 1) = 0
> recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
> gettimeofday({1180973726, 982754}, NULL) = 0
> recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
> gettimeofday({1180973726, 983790}, NULL) = 0

Well, gettimeofday() is not affected by the highres code, but

> nanosleep({0, 0}, NULL) = 0
> nanosleep({0, 0}, NULL) = 0

is. The nanosleep call with a relative timeout of 0 returns immediately
with highres enabled, while it sleeps at least until the next tick
arrives when highres is off. Are there more of those stupid sleeps in
the code ?

	tglx

iperf: performance regression (was b44 driver problem?)

[Stephen Hemminger]

On Mon, 04 Jun 2007 18:35:58 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> On Mon, 2007-06-04 at 09:09 -0700, Stephen Hemminger wrote:
> > > > I did the test with an 2.6.22-rc3-git4 kernel and the p54 driver built 
> > > > external as module. 
> > > 
> > > Can you look at iperf to figure out, whether it does some weird timer
> > > stuff (high frequency interval timer or such) ? Either check the code or
> > > strace it.
> >
> > It is the receiver doing a tight loop doing gettimeofday/recv calls.
> > 
> > 
> > sendto(-1227715616, 0xc, 3085438964, 0, {...}, 3067249832) = 0
> > gettimeofday({1180973726, 981615}, NULL) = 0
> > gettimeofday({1180973726, 981751}, NULL) = 0
> > futex(0x8055c64, 0x5 /* FUTEX_??? */, 1) = 1
> > futex(0x8055c90, FUTEX_WAKE, 1) = 0
> > recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
> > gettimeofday({1180973726, 982754}, NULL) = 0
> > recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
> > gettimeofday({1180973726, 983790}, NULL) = 0
> 
> Well, gettimeofday() is not affected by the highres code, but
> 
> > nanosleep({0, 0}, NULL) = 0
> > nanosleep({0, 0}, NULL) = 0
> 
> is. The nanosleep call with a relative timeout of 0 returns immediately
> with highres enabled, while it sleeps at least until the next tick
> arrives when highres is off. Are there more of those stupid sleeps in
> the code ?

GLIBC pthread_mutex does it, YES it is a problem!
Looks like the old behavior is required for ABI compatibility.

iperf server has several threads. One thread is using pthread_mutex_lock
to wait for the other thread.  It looks like pthread_mutex_lock is using
nanosleep as yield().

Multi-thread strace shows how this could kill performance.
These are with old 2.6.20 kernel that doesn't have the problem:

sendto(-1210930208, 0xc, 3085438964, 0, {...}, 3084035240) = 0
socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 3
setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4)         = 0
bind(3, {sa_family=AF_INET, sin_port=htons(5001), sin_addr=inet_addr("0.0.0.0")}, 16)   = 0
listen(3, 5)                            = 0
futex(0x8055c64, 0x5 /* FUTEX_??? */, 1) = 1
accept(3, {sa_family=AF_INET, sin_port=htons(49973), sin_addr=inet_addr("192.168.0.14")}, [16]) = 4
getsockname(4, {sa_family=AF_INET, sin_port=htons(5001), sin_addr=inet_addr("192.168.0.12")}, [16]) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 24, 0) = 24
mmap2(NULL, 8392704, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6528000
mprotect(0xb6528000, 4096, PROT_NONE)   = 0
clone(child_stack=0xb6d284a4, flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID|CLONE_DETACHED, parent_tidptr=0xb6d28bd8, {entry_number:6, base_addr:0xb6d28b90, limit:1048575, seg_32bit:1, contents:0, read_exec_only:0, limit_in_pages:1, seg_not_present:0, useable:1}, child_tidptr=0xb6d28bd8) = 5622
accept(3, 0x8058b98, [128]) = ? ERESTARTSYS (To be restarted)

==============

sendto(-1219322912, 0xc, 3085438964, 0, {...}, 3075642536) = 0
futex(0x8055d04, 0x5 /* FUTEX_??? */, 1) = 1
futex(0x8055d30, FUTEX_WAKE, 1) = 1
futex(0x8055c64, FUTEX_WAIT, 1, NULL) = 0
futex(0x8055d04, 0x5 /* FUTEX_??? */, 1) = 1
futex(0x8055d30, FUTEX_WAKE, 1)         = 1
futex(0x8055c90, FUTEX_WAKE, 1)         = 0
getsockopt(3, SOL_SOCKET, SO_RCVBUF, [87380], [4]) = 0
fstat64(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 0), ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb7fb0000
write(1, "--------------------------------"..., 61) = 61
write(1, "Server listening on TCP port 500"..., 34) = 34
write(1, "TCP window size: 85.3 KByte (def"..., 38) = 38
write(1, "--------------------------------"..., 61) = 61
nanosleep({0, 0}, NULL)                 = 0
futex(0x8055d04, 0x5 /* FUTEX_??? */, 1) = 1
futex(0x8055d30, FUTEX_WAKE, 1)         = 1
futex(0x8055c64, FUTEX_WAIT, 3, NULL) = 0
futex(0x8055c90, FUTEX_WAIT, 2, NULL)         = -1 EAGAIN (Resource temporarily unavailable)
futex(0x8055d04, 0x5 /* FUTEX_??? */, 1)                               = 1
futex(0x8055d30, FUTEX_WAKE, 1)                          = 0
futex(0x8055c90, FUTEX_WAKE, 1)                               = 0
write(1, "[  4] local 192.168.0.12 port 50"..., 74)                          = 74
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL)                      = 0
nanosleep({0, 0}, NULL)                           = 0
nanosleep({0, 0}, NULL) = 0

...

nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL)                 = 0
write(1, "[  4]  0.0-30.2 sec    336 MByte"..., 50)                     = 50
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL) = 0
futex(0x8055d04, 0x5 /* FUTEX_??? */, 1) = 1
futex(0x8055d30, FUTEX_WAKE, 1) = 0
futex(0x8055c64, FUTEX_WAIT, 5, NULL) = -1 EINTR (Interrupted system call)

===============

sendto(-1227715616, 0xc, 3085438964, 0, {...}, 3067249832) = 0
gettimeofday({1180973726, 981615}, NULL) = 0
gettimeofday({1180973726, 981751}, NULL) = 0
futex(0x8055c64, 0x5 /* FUTEX_??? */, 1) = 1
futex(0x8055c90, FUTEX_WAKE, 1) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 982754}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 983790}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
gettimeofday({1180973726, 984355}, NULL) = 0
recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192

...

gettimeofday({1180973757, 172338}, NULL) = 0
recv(4, "45678901234567890123456789012345"..., 8192, 0) = 1448
gettimeofday({1180973757, 172437}, NULL) = 0
recv(4, "23456789012345678901234567890123"..., 8192, 0) = 1448
gettimeofday({1180973757, 172576}, NULL) = 0
recv(4, "01234567890123456789012345678901"..., 8192, 0) = 1632
gettimeofday({1180973757, 172752}, NULL) = 0
recv(4, "", 8192, 0)                    = 0
gettimeofday({1180973757, 172797}, NULL) = 0
gettimeofday({1180973757, 172817}, NULL) = 0
nanosleep({0, 0}, NULL) = 0
nanosleep({0, 0}, NULL)                 = 0
close(4)                     = 0
futex(0x8055d04, 0x5 /* FUTEX_??? */, 1) = 1
futex(0x8055d30, FUTEX_WAKE, 1) = 0
_exit(0)                                = ?

Re: iperf: performance regression (was b44 driver problem?)

[Thomas Gleixner]

On Mon, 2007-06-04 at 09:59 -0700, Stephen Hemminger wrote:
> > > gettimeofday({1180973726, 982754}, NULL) = 0
> > > recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
> > > gettimeofday({1180973726, 983790}, NULL) = 0
> > 
> > Well, gettimeofday() is not affected by the highres code, but
> > 
> > > nanosleep({0, 0}, NULL) = 0
> > > nanosleep({0, 0}, NULL) = 0
> > 
> > is. The nanosleep call with a relative timeout of 0 returns immediately
> > with highres enabled, while it sleeps at least until the next tick
> > arrives when highres is off. Are there more of those stupid sleeps in
> > the code ?
> 
> GLIBC pthread_mutex does it, YES it is a problem!
> Looks like the old behavior is required for ABI compatibility.
>
> iperf server has several threads. One thread is using pthread_mutex_lock
> to wait for the other thread.  It looks like pthread_mutex_lock is using
> nanosleep as yield().

I doubt that. This is in the iperf code itself.

void thread_rest ( void ) {
#if defined( HAVE_THREAD )
#if defined( HAVE_POSIX_THREAD )
    // TODO add checks for sched_yield or pthread_yield and call that
    // if available
    usleep( 0 );

----------^^^^

It results in a nanosleep({0,0}, NULL)

	tglx

Re: iperf: performance regression (was b44 driver problem?)

[Stephen Hemminger]

On Mon, 04 Jun 2007 19:32:48 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> On Mon, 2007-06-04 at 09:59 -0700, Stephen Hemminger wrote:
> > > > gettimeofday({1180973726, 982754}, NULL) = 0
> > > > recv(4, "\0\0\0\0\0\0\0\1\0\0\23\211\0\0\0\0\0\0\0\0\377\377\364"..., 8192, 0) = 8192
> > > > gettimeofday({1180973726, 983790}, NULL) = 0
> > > 
> > > Well, gettimeofday() is not affected by the highres code, but
> > > 
> > > > nanosleep({0, 0}, NULL) = 0
> > > > nanosleep({0, 0}, NULL) = 0
> > > 
> > > is. The nanosleep call with a relative timeout of 0 returns immediately
> > > with highres enabled, while it sleeps at least until the next tick
> > > arrives when highres is off. Are there more of those stupid sleeps in
> > > the code ?
> > 
> > GLIBC pthread_mutex does it, YES it is a problem!
> > Looks like the old behavior is required for ABI compatibility.
> >
> > iperf server has several threads. One thread is using pthread_mutex_lock
> > to wait for the other thread.  It looks like pthread_mutex_lock is using
> > nanosleep as yield().
> 
> I doubt that. This is in the iperf code itself.
> 
> void thread_rest ( void ) {
> #if defined( HAVE_THREAD )
> #if defined( HAVE_POSIX_THREAD )
>     // TODO add checks for sched_yield or pthread_yield and call that
>     // if available
>     usleep( 0 );
> 
> ----------^^^^
> 
> It results in a nanosleep({0,0}, NULL)
> 
> 	tglx
> 

Yes, the following patch makes iperf work better than ever.
But are other broken applications going to have same problem.
Sounds like the old "who runs first" fork() problems.

--- iperf-2.0.2/compat/Thread.c.orig	2005-05-03 08:15:51.000000000 -0700
+++ iperf-2.0.2/compat/Thread.c	2007-06-04 10:54:21.000000000 -0700
@@ -405,9 +405,13 @@
 void thread_rest ( void ) {
 #if defined( HAVE_THREAD )
 #if defined( HAVE_POSIX_THREAD )
-    // TODO add checks for sched_yield or pthread_yield and call that
-    // if available
+
+#if defined( _POSIX_PRIORITY_SCHEDULING )
+    sched_yield();
+#else
     usleep( 0 );
+#endif
+
 #else // Win32
     SwitchToThread( );
 #endif




-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: iperf: performance regression (was b44 driver problem?)

[Thomas Gleixner]

On Mon, 2007-06-04 at 10:51 -0700, Stephen Hemminger wrote:
> > I doubt that. This is in the iperf code itself.
> > 
> > void thread_rest ( void ) {
> > #if defined( HAVE_THREAD )
> > #if defined( HAVE_POSIX_THREAD )
> >     // TODO add checks for sched_yield or pthread_yield and call that
> >     // if available
> >     usleep( 0 );
> > 
> > ----------^^^^
> > 
> > It results in a nanosleep({0,0}, NULL)
> > 
> > 	tglx
> > 
> 
> Yes, the following patch makes iperf work better than ever.
> But are other broken applications going to have same problem.
> Sounds like the old "who runs first" fork() problems.

Not really. The fork() "who runs first" problem is nowhere specified.

usleep(0) is well defined:

.... If the value of useconds is 0, then the call has no effect.

So the call into the kernel has been wrong for quite a time.

	tglx

Re: iperf: performance regression (was b44 driver problem?)

[Thomas Gleixner]

On Mon, 2007-06-04 at 21:00 +0200, Thomas Gleixner wrote:
> > 
> > Yes, the following patch makes iperf work better than ever.
> > But are other broken applications going to have same problem.
> > Sounds like the old "who runs first" fork() problems.
> 
> Not really. The fork() "who runs first" problem is nowhere specified.
> 
> usleep(0) is well defined:
> 
> .... If the value of useconds is 0, then the call has no effect.
> 
> So the call into the kernel has been wrong for quite a time.
> 

Just for clarification: I'm not saying that we should break the (broken)
user space ABI. I'm going to work out a patch which prints out a warning
(limited number per boot) and emulating the old behavior by a call to
yield() along with an entry into (mis)feature-removal.txt.

	tglx

Re: iperf: performance regression (was b44 driver problem?)

[Ingo Molnar]

* Stephen Hemminger <shemminger@linux-foundation.org> wrote:

> Yes, the following patch makes iperf work better than ever. But are 
> other broken applications going to have same problem. Sounds like the 
> old "who runs first" fork() problems.

this is the first such app and really, and even for this app: i've been 
frequently running iperf on -rt kernels for _years_ and never noticed 
how buggy its 'locking' code was, and that it would under some 
circumstances use up the whole CPU on high-res timers.

If this were a widespread problem then the right solution would be to 
preserve the old behavior. The child-runs-first thing was an unspecified 
detail and many apps grew to depend on how the kernel did it - and when 
we changed it all hell broke lose. Even today, when i switch off 
child-runs-first, Gnome would not start up because some of its startup 
threads are racy and hang :-/

I googled for usleep(0) quickly (on google.com/codesearch and on 
google.com) and it didnt seem to suggest that the problem is widespread. 
(3 hits on the code-search site, all were false positives.)

so ... if this situation were even just a little bit more serious i'd 
argue for working this around and implementing some API quirk. But right 
now i'm leaning towards just saying that the iperf fix exists and fixes 
the problem, and that we already have kernels out with the new behavior. 
Maybe we should add a once-per-boot printk to flesh out any other apps? 
I'd be surprised if there was more than iperf.

	Ingo

Re: iperf: performance regression (was b44 driver problem?)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

On Monday 04 June 2007, Ingo Molnar wrote:
> * Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > Yes, the following patch makes iperf work better than ever. But are
> > other broken applications going to have same problem. Sounds like the
> > old "who runs first" fork() problems.
>
> this is the first such app and really, and even for this app: i've been
> frequently running iperf on -rt kernels for _years_ and never noticed
> how buggy its 'locking' code was, and that it would under some
> circumstances use up the whole CPU on high-res timers.

I must admit I don't know much about that topic, but there is one thing I 
don't understand. Why is iperf (even if it's buggy) able to use up the whole 
cpu? I didn't run it as root but as my normal user so it should have limited 
rights. Shouldn't the linux scheduler distribute cpu time among all running 
processes?

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: iperf: performance regression (was b44 driver problem?)

[Stephen Hemminger]

On Mon, 4 Jun 2007 21:47:59 +0200
Maximilian Engelhardt <maxi@daemonizer.de> wrote:

> On Monday 04 June 2007, Ingo Molnar wrote:
> > * Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > > Yes, the following patch makes iperf work better than ever. But are
> > > other broken applications going to have same problem. Sounds like the
> > > old "who runs first" fork() problems.
> >
> > this is the first such app and really, and even for this app: i've been
> > frequently running iperf on -rt kernels for _years_ and never noticed
> > how buggy its 'locking' code was, and that it would under some
> > circumstances use up the whole CPU on high-res timers.
> 
> I must admit I don't know much about that topic, but there is one thing I 
> don't understand. Why is iperf (even if it's buggy) able to use up the whole 
> cpu? I didn't run it as root but as my normal user so it should have limited 
> rights. Shouldn't the linux scheduler distribute cpu time among all running 
> processes?

In this case, there are two threads. One is receiving data and the other
is spinning checking on progress. If the spinning thread doesn't yield,
it will end up using it's whole quantum (10ms at 100hz), before the
scheduler lets the receiver run again. If the receiving thread doesn't
get to run then on a UP the performance stinks.

The problem only showed up laptop because most of my other systems are
SMP (or fake SMP/HT), and usually set HZ to 1000 not 100.

It kind of reminds me of the family road trip with the kids in the back
seat going "Are we there yet?"

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: iperf: performance regression (was b44 driver problem?)

[Maximilian Engelhardt]

[-- Attachment #1: Type: text/plain, Size: 2138 bytes --]

On Monday 04 June 2007, Stephen Hemminger wrote:
> On Mon, 4 Jun 2007 21:47:59 +0200
>
> Maximilian Engelhardt <maxi-OwNUvPV92VfddJNmlsFzeA@public.gmane.org> wrote:
> > On Monday 04 June 2007, Ingo Molnar wrote:
> > > * Stephen Hemminger <shemminger-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
> > > > Yes, the following patch makes iperf work better than ever. But are
> > > > other broken applications going to have same problem. Sounds like the
> > > > old "who runs first" fork() problems.
> > >
> > > this is the first such app and really, and even for this app: i've been
> > > frequently running iperf on -rt kernels for _years_ and never noticed
> > > how buggy its 'locking' code was, and that it would under some
> > > circumstances use up the whole CPU on high-res timers.
> >
> > I must admit I don't know much about that topic, but there is one thing I
> > don't understand. Why is iperf (even if it's buggy) able to use up the
> > whole cpu? I didn't run it as root but as my normal user so it should
> > have limited rights. Shouldn't the linux scheduler distribute cpu time
> > among all running processes?
>
> In this case, there are two threads. One is receiving data and the other
> is spinning checking on progress. If the spinning thread doesn't yield,
> it will end up using it's whole quantum (10ms at 100hz), before the
> scheduler lets the receiver run again. If the receiving thread doesn't
> get to run then on a UP the performance stinks.
>
Ok, let's see if I got this right:
If there are other processes that want cpu time they will get it after the 
quantum for the iperf thread is used up. So cpu time will be distributed 
among other processes, but it takes some time until they get it and this 
increases latency.

> The problem only showed up laptop because most of my other systems are
> SMP (or fake SMP/HT), and usually set HZ to 1000 not 100.

Hm, on my laptop (Pentium M) I have configured CONFIG_HZ_300 and CONFIG_NO_HZ.
On my desktop PC (Athlon 2000+, also UP) I also have CONFIG_HZ_300 and 
CONFIG_NO_HZ but didn't notice the problem.

Maxi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]