[PATCH 3/3] p54: fix WARN_ON at line 2247 of net/mac80211/rx.c

[PATCH 3/3] p54: fix WARN_ON at line 2247 of net/mac80211/rx.c

[Christian Lamparter]

This patch hopefully fixes a mac80211<->p54 interaction problem, which was
described by Larry Finger (ref: http://marc.info/?l=linux-wireless&m=123009889327707 )

I guess the warning was triggered by pending frames in the receive queue,
while we're doing a band change 5GHz.

Signed-off-by: Christian Lamparter <chunkeey@web.de>
---
Larry,

Are you still hammering your p54usb, or have you finished testing p54(usb)'s stability?
Because I'm waiting for your confirmation for this one. hohohoho.
---
diff -Nurp a/drivers/net/wireless/p54/p54common.c b/drivers/net/wireless/p54/p54common.c
--- a/drivers/net/wireless/p54/p54common.c	2008-12-26 17:33:58.000000000 +0100
+++ b/drivers/net/wireless/p54/p54common.c	2008-12-26 17:54:47.000000000 +0100
@@ -576,6 +576,7 @@ static int p54_rx_data(struct ieee80211_
 	u16 freq = le16_to_cpu(hdr->freq);
 	size_t header_len = sizeof(*hdr);
 	u32 tsf32;
+	u8 rate = hdr->rate & 0xf;
 
 	/*
 	 * If the device is in a unspecified state we have to
@@ -604,8 +605,11 @@ static int p54_rx_data(struct ieee80211_
 	rx_status.qual = (100 * hdr->rssi) / 127;
 	if (hdr->rate & 0x10)
 		rx_status.flag |= RX_FLAG_SHORTPRE;
-	rx_status.rate_idx = (dev->conf.channel->band == IEEE80211_BAND_2GHZ ?
-			hdr->rate : (hdr->rate - 4)) & 0xf;
+	if (dev->conf.channel->band == IEEE80211_BAND_5GHZ)
+		rx_status.rate_idx = (rate < 4) ? 0 : rate - 4;
+	else
+		rx_status.rate_idx = rate;
+
 	rx_status.freq = freq;
 	rx_status.band =  dev->conf.channel->band;
 	rx_status.antenna = hdr->antenna;

Re: [PATCH 3/3] p54: fix WARN_ON at line 2247 of net/mac80211/rx.c

[Larry Finger]

Christian Lamparter wrote:
> This patch hopefully fixes a mac80211<->p54 interaction problem, which was
> described by Larry Finger (ref: http://marc.info/?l=linux-wireless&m=123009889327707 )
> 
> I guess the warning was triggered by pending frames in the receive queue,
> while we're doing a band change 5GHz.
> 
> Signed-off-by: Christian Lamparter <chunkeey@web.de>
> ---
> Larry,
> 
> Are you still hammering your p54usb, or have you finished testing p54(usb)'s stability?
> Because I'm waiting for your confirmation for this one. hohohoho.

Merry Christmas (belated). Yes, this patch stopped the warnings, which started
as a result of using the US, rather than world, regdom.

There are some things p54usb cannot handle such as a simultaneous git pull and
full kernel compilation with the source volumes mounted with NFS. That could be
thermal, although I'm still worried about the O(1) allocations of skb's on
machines with 4K page size. Should I prepare a patch to reduce rx_mtu in the
critical cases?

Tested-by: Larry Finger <Larry.Finger@lwfinger.net>
---

Re: [PATCH 3/3] p54: fix WARN_ON at line 2247 of net/mac80211/rx.c

[Christian Lamparter]

On Friday 26 December 2008 19:49:49 Larry Finger wrote:
> Christian Lamparter wrote:
> > This patch hopefully fixes a mac80211<->p54 interaction problem, which was
> > described by Larry Finger (ref: http://marc.info/?l=linux-wireless&m=123009889327707 )
> > 
> > I guess the warning was triggered by pending frames in the receive queue,
> > while we're doing a band change 5GHz.
> > 
> > Signed-off-by: Christian Lamparter <chunkeey@web.de>
> > ---
> > Larry,
> > 
> > Are you still hammering your p54usb, or have you finished testing p54(usb)'s stability?
> > Because I'm waiting for your confirmation for this one. hohohoho.
> 
> Merry Christmas (belated). Yes, this patch stopped the warnings, which started
> as a result of using the US, rather than world, regdom.
heh, do you have any 5Ghz APs in your neighborhood?

> There are some things p54usb cannot handle such as a simultaneous git pull and
> full kernel compilation with the source volumes mounted with NFS. That could be
> thermal, although I'm still worried about the O(1) allocations of skb's on
> machines with 4K page size. Should I prepare a patch to reduce rx_mtu in the
> critical cases?
Yes please! But why only for critical cases?

The NITRO features are useless anyway, so why don't limit rx_mtu to:
rx_mtu = priv->tx_hdr_len /* USB devices */ + sizeof(struct p54_rx_data) + 4 /* rx alignment */ + IEEE80211_MAX_RTS_THRESHOLD;
(Note: In theory IEEE80211_MAX_FRAG_THRESHOLD should be used instead of RTS_THRESHOLD, but its only 7 bytes more.)

Regards,
	Chr