ath9k: several fixes ported to 2.6.27

We would get an oops on RX on -ENOMEM by passing NULL to the hardware on ath_rx_buf_link(). The oops would look something like this:

ath_rx_tasklet
...

RIP: ath_rx_buf_link

We correct this by handling the allocation for the next skb we will put in our RX tail directly on the ath_rx_tasklet() *prior* to sending up the last hardware processed skb. If we run out of memory this gauranteees we have skbs to work with while it simply drops new received frames.

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 drivers/net/wireless/ath9k/recv.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

drivers/net/wireless/ath9k/recv.c +14 -4

index 0941589..a4f92b2 100644
@@ -441,18 +441,16 @@ static void ath_rx_requeue(struct ath_softc *sc, struct sk_buff *skb)
  */
 static int ath_rx_indicate(struct ath_softc *sc,
 			   struct sk_buff *skb,
+			   struct sk_buff *nskb,
 			   struct ath_recv_status *status,
 			   u16 keyix)
 {
 	struct ath_buf *bf = ATH_RX_CONTEXT(skb)->ctx_rxbuf;
-	struct sk_buff *nskb;
 	int type;
 
 	/* indicate frame to the stack, which will free the old skb. */
 	type = ath__rx_indicate(sc, skb, status, keyix);
 
-	/* allocate a new skb and queue it to for H/W processing */
-	nskb = ath_rxbuf_alloc(sc, sc->sc_rxbufsize);
 	if (nskb != NULL) {
 		bf->bf_mpdu = nskb;
 		bf->bf_buf_addr = ath_skb_map_single(sc,

@@ -741,6 +739,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush)
 	struct ath_desc *ds;
 	struct ieee80211_hdr *hdr;
 	struct sk_buff *skb = NULL;
+	struct sk_buff *nskb = NULL;
 	struct ath_recv_status rx_status;
 	struct ath_hal *ah = sc->sc_ah;
 	int type, rx_processed = 0;

@@ -963,6 +962,17 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush)
 		 */
 		if (sc->sc_rxbufsize < ds->ds_rxstat.rs_datalen)
 			goto rx_next;
+
+		/* allocate a new skb and queue it to for H/W processing */
+		nskb = ath_rxbuf_alloc(sc, sc->sc_rxbufsize);
+
+		/* Diregard current RX'd frame and reuse the old skb */
+		if (!nskb) {
+			list_move_tail(&bf->list, &sc->sc_rxbuf);
+			ath_rx_buf_link(sc, bf);
+			goto rx_next;
+		}
+
 		/*
 		 * Sync and unmap the frame.  At this point we're
 		 * committed to passing the sk_buff somewhere so

@@ -1052,7 +1062,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush)
 
 		/* Pass frames up to the stack. */
 
-		type = ath_rx_indicate(sc, skb,
+		type = ath_rx_indicate(sc, skb, nskb,
 			&rx_status, ds->ds_rxstat.rs_keyix);
 
 		/*

Signature · 6 lines

1.5.6.rc2.15.g457bb.dirty

_______________________________________________
stable mailing list
stable at linux.kernel.org
http://linux.kernel.org/mailman/listinfo/stable

I don't see this patch upstream in Linus's tree. Am I just missing it and if so, do you know the git commit id?

Quoted 2 lines · Greg KH

I don't see this patch upstream in Linus's tree. Am I just missing it and if so, do you know the git commit id?

Nope sorry, John has yet to send his pending-fixes branch to davem and so forth, which would contain it AFAICT. Please correct me if I'm wrong John.

Quoted 7 lines · Luis R. Rodriguez

Quoted 2 lines · Greg KH

I don't see this patch upstream in Linus's tree. Am I just missing it and if so, do you know the git commit id?

Nope sorry, John has yet to send his pending-fixes branch to davem and so forth, which would contain it AFAICT. Please correct me if I'm wrong John.

It has been sent, and Dave sent his pull request to Linus today (or maybe yesterday). I'll try to dig-up the commit IDs tomorrow.

Quoted 9 lines · John W. Linville

Quoted 7 lines · Luis R. Rodriguez

Quoted 2 lines · Greg KH

I don't see this patch upstream in Linus's tree. Am I just missing it and if so, do you know the git commit id?

Nope sorry, John has yet to send his pending-fixes branch to davem and so forth, which would contain it AFAICT. Please correct me if I'm wrong John.

Quoted 2 lines

It has been sent, and Dave sent his pull request to Linus today (or maybe yesterday). I'll try to dig-up the commit IDs tomorrow.

Greg, I see two of these patches in 2.6.27-stable review patch but the one in this subject was not applied, any specific reason or was it just missed?

Quoted 7 lines · Luis R. Rodriguez

Quoted 2 lines · Greg KH

I don't see this patch upstream in Linus's tree. Am I just missing it and if so, do you know the git commit id?

Nope sorry, John has yet to send his pending-fixes branch to davem and so forth, which would contain it AFAICT. Please correct me if I'm wrong John.

It has been sent, and Dave sent his pull request to Linus today (or maybe yesterday). I'll try to dig-up the commit IDs tomorrow.

Quoted 9 lines · John W. Linville

Quoted 7 lines · Luis R. Rodriguez

Quoted 2 lines · Greg KH

I don't see this patch upstream in Linus's tree. Am I just missing it and if so, do you know the git commit id?

Nope sorry, John has yet to send his pending-fixes branch to davem and so forth, which would contain it AFAICT. Please correct me if I'm wrong John.

Quoted 2 lines

It has been sent, and Dave sent his pull request to Linus today (or maybe yesterday). I'll try to dig-up the commit IDs tomorrow.

Hmmm...I seem to have sent this to -next instead. Since it fixes an Oops, I guess I can requeue for 2.6.28.

Luis,
I tried to respond to your comment in bug 12110 but do not have a id yet.

Does this patch fix this bug?

Here is my response

I have the same problem but with wireless testing on a 2.6.28-rc6-wl kernel. I can run for hours if I do not use the wireless heavily.
But will crash within 10mins if I use NX.
(I have the NX parm turned on in the kernel)
I also get the same symptoms re a ping that will just stop/start, directly before a crash. Also it starts to use my swap file. I NEVER use swap normally.
Would you like me to apply the same patch and see if I can get some message?

Can I get a patch for 2.6.28-rc6-wl ?

wireless-testing is now on 2.6.28-rc7, please do a git pull and recompile and test that. It has some critical fixes for ath9k.

Luis,

Quoted 1 line

Quoted 3 lines

wireless-testing is now on 2.6.28-rc7, please do a git pull and recompile and test that. It has some critical fixes for ath9k.

Downloaded and installed, I had done a git-pull a few hours before and it was not there:)

I set up my system with NX running and was waiting to be up 2hours. Unfortunately it only made 1:50.

Luis,

Quoted 6 lines

Quoted 4 lines

So I think the driver could be fine, it could be application specific. Will also do a search of the NX site and see if they have any problems.

Will let you know how I go.

I see.. What is NX?

http://www.nomachine.com/
NX is really great and is easily the best remote control software. While I can get by with krdc I want to use NX if I can.

We run tests against ath9k over hours too and

Quoted 7 lines

I don't think we've seen this memory hog issue. But if your userspace application is hogging up application your kernel shoulod not panic, unless of course some -ENOMEM as part of your drivers is not being handled correctly. But the patches merged recently actually handle this for ath9k :) I at least have tested it with mem=200 and even using the swiotlb.

Yes I saw that you had included the -ENOMEM fix, so I was very hopeful that the problem would go away.
I must say the difference between rc6 and rc7 is really significant. Things are quicker and more stable for all the normal operations. I just do not see what is different about NX, I might try running wireshark on it. If you could fire up NX and it fails then at least we could say that it is not specific to my machine.
I should also point out that when researching this problem on the web I found that hundreds of people were having the same freezing problem.

My krdc has been running for 30mins with no problems, but the memory is going up.

Back in a bit

So do some other test -- run bitorrent, or iperf. That should rule out ath9k.

To test if its NX try testing it over a wired ethernet connection.

Luis,

Quoted 3 lines

So do some other test -- run bitorrent, or iperf. That should rule out ath9k.

Yes I was trying that, in the process of setting up ftp.

Quoted 2 lines

To test if its NX try testing it over a wired ethernet connection.

Yes, I should have thought of that, however I have problems with my eth0 port.

Anyway while looking around the nomachine site I found that my version of NX was pretty old. So I downloaded the latest.

Some observations

1) I have been up for over an hour on NX

2) The number of errors being generated are a LOT less, I am collecting stats.
3) I have changed the bandwidth back to 20mhz so I am only getting 54Mbs at the moment.

So the main reason I upgraded was because I felt that NX was causing the large number of TX/RX errors. I think has proved correct, but I will not know for sure until I put the bandwidth back up to 40mhz.

If this proves to be correct then I think the driver has a problem as there is no way that errors should cause such a hard lockup. So my question is how does the driver handle the fact that packets go missing?

Or is this problem handled further up in the tcp stack.

Lets stop talking about NX.

Quoted 4 lines

2) The number of errors being generated are a LOT less, I am collecting stats.
3) I have changed the bandwidth back to 20mhz so I am only getting 54Mbs at the moment.

You should be able to get more than 54Mbps on HT20 (802.11n HT20), the theoretical max for HT20 with short guard interval is 144 Mbit/s, the theoretical max for HT20 with long guard interval is 130 Mbps/s -- using only 2 stream. Your card should be 2-stream. For details of how to get this information please see:

http://wireless.kernel.org/en/developers/Documentation/ieee80211/802.11n#MCSRates

This doesn't mean you should get these, but it means the card can use these target rates for TX / RX, the actual throughput values seen will be different but can be higher than 54Mbps. Just to give you an idea with 802.11g I usually get highest about 24Mbps in practice.

Please run tests with UDP and TCP with iperf. You can do with by setting an iperf server with a station connected via Gibabit ethernet on the AP and then having the iperf client send data to the station. Do this then the other way around, having the iperf server on the wireless station and have the station send data to you.

Right now we're running some interoperability tests with different APs so we should hopefully start seeing issues perhaps users are reporting that we are not. What AP do you have?

Quoted 3 lines

So the main reason I upgraded was because I felt that NX was causing the large number of TX/RX errors. I think has proved correct, but I will not know for sure until I put the bandwidth back up to 40mhz.

It seems easier to run long tests with things like iperf no?

If the wireless frame didn't get to the AP the AP doesn't send an ACK and the STA retries. If using 802.11n AMPDU (the usual case for aggregation) then the STA will issue a block ack request, send data the AP sends a Block ACK in which it has a bitmap which indicates which frames went through and which never reached the AP. The STA then retransmits the frames that the bitmap indicates failed.

Luis,
The good news is the network has been working for the last 13 hours:) The only significant change I have made to the router is that I have forced it to go to channel 6.
My other settings are auto for the bit rate and 40Mhz for the bandwidth. The router is also set to pick either 11n or 11g.

For some reason the router is telling me that it has picked 11g for both machines? The one that is only 11g capable is running at 54Mbs but the 11n one is running at 14mcs or 130Mbs.

I did not know you could get 130
out of an 11g.

My point is that if I want to test 11n again I will probably have to buy a pci card and convert the other machine as well.

This fixes a few issues seen on MacBook Pros:

http://bugzilla.kernel.org/show_bug.cgi?id=11811

I've ported them to 2.6.27 to help with them being applied sooner. I've tried to keep them as small as possible. I don't have the ports for 2.6.28 though I think John will be sending them soon through his pending-fixes branch.

These are already applied on wireless-testing.

This fixes a few issues seen on MacBook Pros:

http://bugzilla.kernel.org/show_bug.cgi?id=11811

I've ported them to 2.6.27 to help with them being applied sooner.

Quoted 5 lines · Luis R. Rodriguez

This fixes a few issues seen on MacBook Pros:

http://bugzilla.kernel.org/show_bug.cgi?id=11811

I've ported them to 2.6.27 to help with them being applied sooner.

What do you mean by "sooner"? They need to be in Linus's tree before we can add them to the 2.6.27-stable tree. Please let us know when that happens, and what the git ids of them are, so we can add them to the next -stable release.