Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH 3/3] ath9k: Remove unused paprd_txok
From: Vasanthakumar Thiagarajan @ 2010-06-24  9:42 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless
In-Reply-To: <1277372566-22883-1-git-send-email-vasanth@atheros.com>

Signed-off-by: Vasanthakumar Thiagarajan <vasanth@atheros.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |    1 -
 drivers/net/wireless/ath/ath9k/xmit.c  |    6 ++----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 6c561fa..72d5e52 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -563,7 +563,6 @@ struct ath_softc {
 	struct mutex mutex;
 	struct work_struct paprd_work;
 	struct completion paprd_complete;
-	int paprd_txok;
 
 	u32 intrstatus;
 	u32 sc_flags; /* SC_OP_* */
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index edbeffb..c41185b 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -1948,12 +1948,10 @@ static void ath_tx_complete_buf(struct ath_softc *sc, struct ath_buf *bf,
 	if (bf->bf_state.bfs_paprd) {
 		if (time_after(jiffies,
 			       bf->bf_state.bfs_paprd_timestamp +
-			       msecs_to_jiffies(ATH_PAPRD_TIMEOUT))) {
+			       msecs_to_jiffies(ATH_PAPRD_TIMEOUT)))
 			dev_kfree_skb_any(skb);
-		} else {
-			sc->paprd_txok = txok;
+		else
 			complete(&sc->paprd_complete);
-		}
 	} else {
 		ath_tx_complete(sc, skb, bf->aphy, tx_flags);
 		ath_debug_stat_tx(sc, txq, bf, ts);
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH 2/3] ath9k: Restore ps state in ath_paprd_calibrate() upon failure to allocate skb
From: Vasanthakumar Thiagarajan @ 2010-06-24 11:01 UTC (permalink / raw)
  To: linville@tuxdriver.com; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1277372566-22883-2-git-send-email-vasanth@atheros.com>

On Thu, Jun 24, 2010 at 03:12:45PM +0530, Vasanthakumar Thiagarajan wrote:
> Signed-off-by: Vasanthakumar Thiagarajan <vasanth@atheros.com>
> ---
>  drivers/net/wireless/ath/ath9k/main.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index 6d57877..484d113 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -271,7 +271,7 @@ void ath_paprd_calibrate(struct work_struct *work)
>  	ath9k_ps_wakeup(sc);
>  	skb = alloc_skb(len, GFP_KERNEL);
>  	if (!skb)
> -		return;
> +		goto fail_paprd;
>  
>  	tx_info = IEEE80211_SKB_CB(skb);

Please drop this particular patch, ath9k_ps_wakeup() does not need
to be there before alloc_skb() in the first place. I'll send a cleaner one.  

Vasanth

^ permalink raw reply

* [PATCH] ath9k: Wakeup the chip in an appropriate place in ath_paprd_calibrate()
From: Vasanthakumar Thiagarajan @ 2010-06-24 11:09 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless

Move ath9k_ps_wakeup() down just before accessing hw registers.

Signed-off-by: Vasanthakumar Thiagarajan <vasanth@atheros.com>
---
 drivers/net/wireless/ath/ath9k/main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 6d57877..efbf535 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -268,7 +268,6 @@ void ath_paprd_calibrate(struct work_struct *work)
 	int time_left;
 	int i;
 
-	ath9k_ps_wakeup(sc);
 	skb = alloc_skb(len, GFP_KERNEL);
 	if (!skb)
 		return;
@@ -289,6 +288,7 @@ void ath_paprd_calibrate(struct work_struct *work)
 	qnum = sc->tx.hwq_map[WME_AC_BE];
 	txctl.txq = &sc->tx.txq[qnum];
 
+	ath9k_ps_wakeup(sc);
 	ar9003_paprd_init_table(ah);
 	for (chain = 0; chain < AR9300_MAX_CHAINS; chain++) {
 		if (!(ah->caps.tx_chainmask & BIT(chain)))
-- 
1.7.0.4


^ permalink raw reply related

* rt2x00 & mac80211: correct usage of ieee80211_beacon_get_tim?
From: Helmut Schaa @ 2010-06-24 13:21 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg, Ivo van Doorn, Gertjan van Wingerde

Hi,

I've just reviewed the beacon handling in rt2x00 in AP mode and experienced
some inconsistencies. The DTIM count is not correctly updated: sometimes
multiple beacons are sent out using the same DTIM count.

rt2x00 calls ieee80211_beacon_get_tim right after the current beacon was
sent out to fetch the next one. However, rt2x00 also implements the set_tim
callback and updates the beacon in each call to set_tim. As far as I
understood the code in mac80211 the set_tim callback is called when the
first frame for a powersaving station gets queued.

Since every call to ieee80211_beacon_get_tim updates the DTIM count the
following can happen (assuming a DTIM period of 2):

- the hw sends out the current beacon (DTIM count == 0)
- call to ieee80211_beacon_get_tim fetches the next beacon (DTIM count == 1)
- the first frame for a PS STA gets queued -> set_tim
- again call ieee80211_beacon_get_tim (DTIM count == 0)
- hw sends out the beacon with incorrect DTIM count

A proper way of fixing this issue would be not to use the set_tim callback but
just fetch the next beacon right before it gets send out (like ath* does).
However, that's not easily possible with rt2x00 devices older then rt2800 as
they only generate beacon_done interrupts (which is obviously too late for
fetching the current beacon ;) ).

So, is the current implementation in rt2x00 supposed to work and mac80211
needs fixing?

Could we add a parameter to ieee80211_beacon_get_tim that indicates if a _new_
beacon should be generated or if the _current_ beacon should be updated in
response to the set_tim callback?

Any other ideas?

Thanks,
Helmut

^ permalink raw reply

* Re: rt2x00 & mac80211: correct usage of ieee80211_beacon_get_tim?
From: John W. Linville @ 2010-06-24 14:32 UTC (permalink / raw)
  To: Helmut Schaa
  Cc: linux-wireless, Johannes Berg, Ivo van Doorn,
	Gertjan van Wingerde
In-Reply-To: <201006241521.47623.helmut.schaa@googlemail.com>

On Thu, Jun 24, 2010 at 03:21:47PM +0200, Helmut Schaa wrote:

> So, is the current implementation in rt2x00 supposed to work and mac80211
> needs fixing?
> 
> Could we add a parameter to ieee80211_beacon_get_tim that indicates if a _new_
> beacon should be generated or if the _current_ beacon should be updated in
> response to the set_tim callback?
> 
> Any other ideas?

Since we seem to want to allow ieee80211_beacon_get_tim to be called
from the set_tim callbacks, maybe we could manage bss->dtim_count
somewhere other than ieee80211_beacon_add_tim?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* [PATCH] rtl8180: mark rtl8180_beacon_work static
From: John W. Linville @ 2010-06-24 15:09 UTC (permalink / raw)
  To: linux-wireless; +Cc: John W. Linville

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 drivers/net/wireless/rtl818x/rtl8180_dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/rtl818x/rtl8180_dev.c b/drivers/net/wireless/rtl818x/rtl8180_dev.c
index 515817d..4270502 100644
--- a/drivers/net/wireless/rtl818x/rtl8180_dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8180_dev.c
@@ -671,7 +671,7 @@ static u64 rtl8180_get_tsf(struct ieee80211_hw *dev)
 	       (u64)(rtl818x_ioread32(priv, &priv->map->TSFT[1])) << 32;
 }
 
-void rtl8180_beacon_work(struct work_struct *work)
+static void rtl8180_beacon_work(struct work_struct *work)
 {
 	struct rtl8180_vif *vif_priv =
 		container_of(work, struct rtl8180_vif, beacon_work.work);
-- 
1.7.0.1


^ permalink raw reply related

* [PATCH] mac80211: don't shadow mgmt variable in ieee80211_rx_h_action
From: John W. Linville @ 2010-06-24 15:14 UTC (permalink / raw)
  To: linux-wireless; +Cc: John W. Linville

net/mac80211/rx.c:2059:39: warning: symbol 'mgmt' shadows an earlier one
net/mac80211/rx.c:1916:31: originally declared here

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 net/mac80211/rx.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 1f76352..ab58a5d 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2056,11 +2056,11 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
 	nskb = skb_copy_expand(rx->skb, local->hw.extra_tx_headroom, 0,
 			       GFP_ATOMIC);
 	if (nskb) {
-		struct ieee80211_mgmt *mgmt = (void *)nskb->data;
+		struct ieee80211_mgmt *nmgmt = (void *)nskb->data;
 
-		mgmt->u.action.category |= 0x80;
-		memcpy(mgmt->da, mgmt->sa, ETH_ALEN);
-		memcpy(mgmt->sa, rx->sdata->vif.addr, ETH_ALEN);
+		nmgmt->u.action.category |= 0x80;
+		memcpy(nmgmt->da, nmgmt->sa, ETH_ALEN);
+		memcpy(nmgmt->sa, rx->sdata->vif.addr, ETH_ALEN);
 
 		memset(nskb->cb, 0, sizeof(nskb->cb));
 
-- 
1.7.0.1


^ permalink raw reply related

* [PATCH] wireless: mark reg_mutex as static
From: John W. Linville @ 2010-06-24 15:27 UTC (permalink / raw)
  To: linux-wireless; +Cc: John W. Linville

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 net/wireless/reg.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 994c7c5..1ac2bdd 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -80,7 +80,7 @@ static const struct ieee80211_regdomain *country_ie_regdomain;
  *     - country_ie_regdomain
  *     - last_request
  */
-DEFINE_MUTEX(reg_mutex);
+static DEFINE_MUTEX(reg_mutex);
 #define assert_reg_lock() WARN_ON(!mutex_is_locked(&reg_mutex))
 
 /* Used to queue up regulatory hints */
-- 
1.7.0.1


^ permalink raw reply related

* [PATCH] minstrel_ht: mark minstrel_mcs_groups as static
From: John W. Linville @ 2010-06-24 15:16 UTC (permalink / raw)
  To: linux-wireless; +Cc: John W. Linville

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 net/mac80211/rc80211_minstrel_ht.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 7a04951..a8f658c 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -59,7 +59,7 @@
  * groups, based on the number of streams and flags (HT40, SGI) that they
  * use.
  */
-const struct mcs_group minstrel_mcs_groups[] = {
+static const struct mcs_group minstrel_mcs_groups[] = {
 	MCS_GROUP(1, 0, 0),
 	MCS_GROUP(2, 0, 0),
 #if MINSTREL_MAX_STREAMS >= 3
-- 
1.7.0.1


^ permalink raw reply related

* [PATCH] minstrel_ht: make *idx unsigned in minstrel_downgrade_rate
From: John W. Linville @ 2010-06-24 15:19 UTC (permalink / raw)
  To: linux-wireless; +Cc: John W. Linville

net/mac80211/rc80211_minstrel_ht.c:440:46: warning: incorrect type in argument 2 (different signedness)
net/mac80211/rc80211_minstrel_ht.c:440:46:    expected int *idx
net/mac80211/rc80211_minstrel_ht.c:440:46:    got unsigned int *<noident>
net/mac80211/rc80211_minstrel_ht.c:446:46: warning: incorrect type in argument 2 (different signedness)
net/mac80211/rc80211_minstrel_ht.c:446:46:    expected int *idx
net/mac80211/rc80211_minstrel_ht.c:446:46:    got unsigned int *<noident>

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 net/mac80211/rc80211_minstrel_ht.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index a8f658c..ae91ad9 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -328,7 +328,8 @@ minstrel_next_sample_idx(struct minstrel_ht_sta *mi)
 }
 
 static void
-minstrel_downgrade_rate(struct minstrel_ht_sta *mi, int *idx, bool primary)
+minstrel_downgrade_rate(struct minstrel_ht_sta *mi, unsigned int *idx,
+			bool primary)
 {
 	int group, orig_group;
 
-- 
1.7.0.1


^ permalink raw reply related

* Re: rt2x00 & mac80211: correct usage of ieee80211_beacon_get_tim?
From: Johannes Berg @ 2010-06-24 15:51 UTC (permalink / raw)
  To: Helmut Schaa; +Cc: linux-wireless, Ivo van Doorn, Gertjan van Wingerde
In-Reply-To: <201006241521.47623.helmut.schaa@googlemail.com>

On Thu, 2010-06-24 at 15:21 +0200, Helmut Schaa wrote:

> I've just reviewed the beacon handling in rt2x00 in AP mode and experienced
> some inconsistencies. The DTIM count is not correctly updated: sometimes
> multiple beacons are sent out using the same DTIM count.

Er, so there's your problem already.

> rt2x00 calls ieee80211_beacon_get_tim right after the current beacon was
> sent out to fetch the next one. However, rt2x00 also implements the set_tim
> callback and updates the beacon in each call to set_tim. As far as I
> understood the code in mac80211 the set_tim callback is called when the
> first frame for a powersaving station gets queued.
> 
> Since every call to ieee80211_beacon_get_tim updates the DTIM count the
> following can happen (assuming a DTIM period of 2):
> 
> - the hw sends out the current beacon (DTIM count == 0)
> - call to ieee80211_beacon_get_tim fetches the next beacon (DTIM count == 1)
> - the first frame for a PS STA gets queued -> set_tim
> - again call ieee80211_beacon_get_tim (DTIM count == 0)
> - hw sends out the beacon with incorrect DTIM count
> 
> A proper way of fixing this issue would be not to use the set_tim callback but
> just fetch the next beacon right before it gets send out (like ath* does).
> However, that's not easily possible with rt2x00 devices older then rt2800 as
> they only generate beacon_done interrupts (which is obviously too late for
> fetching the current beacon ;) ).
> 
> So, is the current implementation in rt2x00 supposed to work and mac80211
> needs fixing?
> 
> Could we add a parameter to ieee80211_beacon_get_tim that indicates if a _new_
> beacon should be generated or if the _current_ beacon should be updated in
> response to the set_tim callback?
> 
> Any other ideas?

No, your entire description makes no sense. There are two possibilities:

 a) You get a new beacon from mac80211 each time you send it. Then you
    don't have to worry about the set_tim() callback at all -- don't
    assign it!

 b) You need to get a new beacon frame from mac80211 only when it
    changes. You can do this from set_tim(). HOWEVER: since you're not
    getting a new one from mac80211 all the time anyway, you NEED to
    have the driver or firmware overwrite the DTIM count, like b43's
    firmware for example will do.

Ok so maybe there are more possibilities like the firmware filling the
TIM IE differently and you would use set_tim() differently then.

However, *fundamentally*, any time you don't get a new skb from mac80211
for each transmitted beacon you NEED to overwrite the DTIM count in it.

johannes


^ permalink raw reply

* Re: rt2x00 & mac80211: correct usage of ieee80211_beacon_get_tim?
From: Johannes Berg @ 2010-06-24 15:53 UTC (permalink / raw)
  To: Helmut Schaa; +Cc: linux-wireless, Ivo van Doorn, Gertjan van Wingerde
In-Reply-To: <1277394714.3870.8.camel@jlt3.sipsolutions.net>

On Thu, 2010-06-24 at 17:51 +0200, Johannes Berg wrote:

>  b) You need to get a new beacon frame from mac80211 only when it
>     changes. You can do this from set_tim(). HOWEVER: since you're not
>     getting a new one from mac80211 all the time anyway, you NEED to
>     have the driver or firmware overwrite the DTIM count, like b43's
>     firmware for example will do.

Oh and "when it changes" doesn't include DTIM period changes, obviously.

johannes


^ permalink raw reply

* Re: rt2x00 & mac80211: correct usage of ieee80211_beacon_get_tim?
From: Johannes Berg @ 2010-06-24 15:54 UTC (permalink / raw)
  To: Helmut Schaa; +Cc: linux-wireless, Ivo van Doorn, Gertjan van Wingerde
In-Reply-To: <1277394837.3870.9.camel@jlt3.sipsolutions.net>

On Thu, 2010-06-24 at 17:53 +0200, Johannes Berg wrote:
> On Thu, 2010-06-24 at 17:51 +0200, Johannes Berg wrote:
> 
> >  b) You need to get a new beacon frame from mac80211 only when it
> >     changes. You can do this from set_tim(). HOWEVER: since you're not
> >     getting a new one from mac80211 all the time anyway, you NEED to
> >     have the driver or firmware overwrite the DTIM count, like b43's
> >     firmware for example will do.
> 
> Oh and "when it changes" doesn't include DTIM period changes, obviously.

Er, it does include "DTIM period" changes (shouldn't actually happen
anyway), I meant "DTIM count field changes".

johannes


^ permalink raw reply

* Re: intel 5100/iwlagn bug in 2.6.35-rc2 during large file transfer
From: Richard Farina @ 2010-06-24 16:12 UTC (permalink / raw)
  To: reinette chatre; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1277225293.25793.2257.camel@rchatre-DESK>

reinette chatre wrote:
> On Wed, 2010-06-16 at 19:56 -0700, Richard Farina wrote:
>   
>> The repeated line appears ad infinitum filling my dmesg buffer.  This of 
>> hangcheck timer seem to trigger with every large file transfer on my 
>> intel 5100.  What would you like me to do to provide a more useful 
>> output as this is currently extremely easy to reproduce.  Kernel 2.6.34 
>> using compat-wireless stable 2.6.35-rc2
>>
>> Thanks,
>> Rick Farina
>>
>> phy0: failed to reallocate TX buffer
>> phy0: failed to reallocate TX buffer
>> phy0: failed to reallocate TX buffer
>> phy0: failed to reallocate TX buffer
>> phy0: failed to reallocate TX buffer
>> phy0: failed to reallocate TX buffer
>> phy0: failed to reallocate TX buffer
>>     
>
> First mac80211 runs out of memory ... it cannot even allocate enough
> memory for a skb header.
>
>   
>> net_ratelimit: 22 callbacks suppressed
>> __alloc_pages_slowpath: 3799 callbacks suppressed
>> swapper: page allocation failure. order:1, mode:0x4020
>> Pid: 0, comm: swapper Not tainted 2.6.34-pentoo #5
>> Call Trace:
>>  <IRQ>  [<ffffffff8109cb74>] __alloc_pages_nodemask+0x571/0x5b9
>>  [<ffffffff816732e9>] ? skb_release_data+0xc4/0xc9
>>  [<ffffffffa04701e4>] iwlagn_rx_allocate+0x98/0x25a [iwlagn]
>>     
>
> Next driver runs out of memory.
>
> Note that the above are all atomic allocations that fail and should be
> able to recover. 
>
> Is your system low on memory? Are you running applications that take a
> lot of memory? Does your wifi connection drop or otherwise suffer at the
> time you see these messages?
>
>   
I have 4GB of RAM on this system, I often run a VM which wastes like 
half that but that still leaves 2GB for linux and I'm running XFCE4 so 
not exactly a memory hog. It's possible that firefox leaks ram until I'm 
out but that would be a LOT of leak, much more than I usually see.

Yeah, as you may guess these errors cause my wifi connection to slow 
drastically. If I had to guess, since this happens when I make a large 
file transfer it is likely that something related is leaking RAM.  I'm 
using wget or axel to download and NFS to dump the files on a NAS.  I'll 
try to trigger this again and watch memory usage to see if I can find 
something other than the driver that could be leaking.  Failing that, 
what do I need to enable to find a leak in the driver?

Thanks,
Rick Farina

> Reinette
>
>
>
>
>   


^ permalink raw reply

* Re: rt2x00 & mac80211: correct usage of ieee80211_beacon_get_tim?
From: Helmut Schaa @ 2010-06-24 16:20 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Ivo van Doorn, Gertjan van Wingerde
In-Reply-To: <1277394714.3870.8.camel@jlt3.sipsolutions.net>

Am Donnerstag 24 Juni 2010 schrieb Johannes Berg:
>  a) You get a new beacon from mac80211 each time you send it. Then you
>     don't have to worry about the set_tim() callback at all -- don't
>     assign it!
> 
>  b) You need to get a new beacon frame from mac80211 only when it
>     changes. You can do this from set_tim(). HOWEVER: since you're not
>     getting a new one from mac80211 all the time anyway, you NEED to
>     have the driver or firmware overwrite the DTIM count, like b43's
>     firmware for example will do.
> 
> Ok so maybe there are more possibilities like the firmware filling the
> TIM IE differently and you would use set_tim() differently then.
> 
> However, *fundamentally*, any time you don't get a new skb from mac80211
> for each transmitted beacon you NEED to overwrite the DTIM count in it.

So, to answer my own question:

mac80211 behaves as designed and we need to fix rt2x00 to correctly update
the DTIM parameters when we stick to the set_tim callback for updating the
beacon template (option b).

Or we would implement option a) to get a new beacon prior to transmission
and would get a correct DTIM count from mac80211.

Thanks for the clarification,
Helmut

^ permalink raw reply

* Re: [PATCH] minstrel_ht: mark minstrel_mcs_groups as static
From: Felix Fietkau @ 2010-06-24 16:47 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless
In-Reply-To: <1277392602-10578-1-git-send-email-linville@tuxdriver.com>

On 2010-06-24 5:16 PM, John W. Linville wrote:
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> ---
>  net/mac80211/rc80211_minstrel_ht.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
> index 7a04951..a8f658c 100644
> --- a/net/mac80211/rc80211_minstrel_ht.c
> +++ b/net/mac80211/rc80211_minstrel_ht.c
> @@ -59,7 +59,7 @@
>   * groups, based on the number of streams and flags (HT40, SGI) that they
>   * use.
>   */
> -const struct mcs_group minstrel_mcs_groups[] = {
> +static const struct mcs_group minstrel_mcs_groups[] = {
NACK. minstrel_ht debugfs needs this. The proper fix for the warning is
to add this one to a header file instead.

- Felix

^ permalink raw reply

* Re: intel 5100/iwlagn bug in 2.6.35-rc2 during large file transfer
From: reinette chatre @ 2010-06-24 17:13 UTC (permalink / raw)
  To: Richard Farina; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <4C2383E2.8000909@gmail.com>

Hi Richard,

On Thu, 2010-06-24 at 09:12 -0700, Richard Farina wrote:
> reinette chatre wrote:
> > On Wed, 2010-06-16 at 19:56 -0700, Richard Farina wrote:
> >   
> >> The repeated line appears ad infinitum filling my dmesg buffer.  This of 
> >> hangcheck timer seem to trigger with every large file transfer on my 
> >> intel 5100.  What would you like me to do to provide a more useful 
> >> output as this is currently extremely easy to reproduce.  Kernel 2.6.34 
> >> using compat-wireless stable 2.6.35-rc2
> >>
> >> Thanks,
> >> Rick Farina
> >>
> >> phy0: failed to reallocate TX buffer
> >> phy0: failed to reallocate TX buffer
> >> phy0: failed to reallocate TX buffer
> >> phy0: failed to reallocate TX buffer
> >> phy0: failed to reallocate TX buffer
> >> phy0: failed to reallocate TX buffer
> >> phy0: failed to reallocate TX buffer
> >>     
> >
> > First mac80211 runs out of memory ... it cannot even allocate enough
> > memory for a skb header.
> >
> >   
> >> net_ratelimit: 22 callbacks suppressed
> >> __alloc_pages_slowpath: 3799 callbacks suppressed
> >> swapper: page allocation failure. order:1, mode:0x4020
> >> Pid: 0, comm: swapper Not tainted 2.6.34-pentoo #5
> >> Call Trace:
> >>  <IRQ>  [<ffffffff8109cb74>] __alloc_pages_nodemask+0x571/0x5b9
> >>  [<ffffffff816732e9>] ? skb_release_data+0xc4/0xc9
> >>  [<ffffffffa04701e4>] iwlagn_rx_allocate+0x98/0x25a [iwlagn]
> >>     
> >
> > Next driver runs out of memory.
> >
> > Note that the above are all atomic allocations that fail and should be
> > able to recover. 
> >
> > Is your system low on memory? Are you running applications that take a
> > lot of memory? Does your wifi connection drop or otherwise suffer at the
> > time you see these messages?
> >
> >   
> I have 4GB of RAM on this system, I often run a VM which wastes like 
> half that but that still leaves 2GB for linux and I'm running XFCE4 so 
> not exactly a memory hog. It's possible that firefox leaks ram until I'm 
> out but that would be a LOT of leak, much more than I usually see.

There has been an issue with atomic memory allocations ever since
2.6.31. This used to be easy to trigger with iwlagn, but we fixed a
number of issues. There are still issue with any atomic memory
allocation (not just iwlagn) and this issue is still open. You can find
more information at https://bugzilla.kernel.org/show_bug.cgi?id=14141

> Yeah, as you may guess these errors cause my wifi connection to slow 
> drastically.

The driver, when unable to allocate memory atomically, will reattempt
the allocation later when it can use GFP_KERNEL. I think there may be
ways in which we can try to optimize this since right now it will only
schedule this when there are about 8 buffers remaining. I was looking at
your trace again and even though you state "Kernel 2.6.34 using
compat-wireless stable 2.6.35-rc2" ... the trace you provide does not
seem to match the driver code from 2.6.35-rc2. Could you please confirm
which version of driver you are running so that I can prepare a patch?

>  If I had to guess, since this happens when I make a large 
> file transfer it is likely that something related is leaking RAM.  I'm 
> using wget or axel to download and NFS to dump the files on a NAS.  I'll 
> try to trigger this again

Does this happen every time you run this test? I would like to get an
idea whether we will get a clear indication whether our changes will
help or not.

>  and watch memory usage to see if I can find 
> something other than the driver that could be leaking.  Failing that, 
> what do I need to enable to find a leak in the driver?

Perhaps kmemleak?

Reinette




^ permalink raw reply

* Re: [PATCH] minstrel_ht: mark minstrel_mcs_groups as static
From: John W. Linville @ 2010-06-24 17:07 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless
In-Reply-To: <4C238C3A.6090905@openwrt.org>

On Thu, Jun 24, 2010 at 06:47:54PM +0200, Felix Fietkau wrote:
> On 2010-06-24 5:16 PM, John W. Linville wrote:
> > Signed-off-by: John W. Linville <linville@tuxdriver.com>
> > ---
> >  net/mac80211/rc80211_minstrel_ht.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
> > index 7a04951..a8f658c 100644
> > --- a/net/mac80211/rc80211_minstrel_ht.c
> > +++ b/net/mac80211/rc80211_minstrel_ht.c
> > @@ -59,7 +59,7 @@
> >   * groups, based on the number of streams and flags (HT40, SGI) that they
> >   * use.
> >   */
> > -const struct mcs_group minstrel_mcs_groups[] = {
> > +static const struct mcs_group minstrel_mcs_groups[] = {
> NACK. minstrel_ht debugfs needs this. The proper fix for the warning is
> to add this one to a header file instead.

Yup, fair enough.  Replacement patch to follow.

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* [PATCH] minstrel_ht: move minstrel_mcs_groups declaration to header file
From: John W. Linville @ 2010-06-24 17:06 UTC (permalink / raw)
  To: linux-wireless; +Cc: Felix Fietkau, John W. Linville
In-Reply-To: <1277392602-10578-1-git-send-email-linville@tuxdriver.com>

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 net/mac80211/rc80211_minstrel_ht.h         |    2 ++
 net/mac80211/rc80211_minstrel_ht_debugfs.c |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.h b/net/mac80211/rc80211_minstrel_ht.h
index 696c0fc..462d2b2 100644
--- a/net/mac80211/rc80211_minstrel_ht.h
+++ b/net/mac80211/rc80211_minstrel_ht.h
@@ -29,6 +29,8 @@ struct mcs_group {
 	unsigned int duration[MCS_GROUP_RATES];
 };
 
+extern const struct mcs_group minstrel_mcs_groups[];
+
 struct minstrel_rate_stats {
 	/* current / last sampling period attempts/success counters */
 	unsigned int attempts, last_attempts;
diff --git a/net/mac80211/rc80211_minstrel_ht_debugfs.c b/net/mac80211/rc80211_minstrel_ht_debugfs.c
index 4fb3ccb..4a5a4b3 100644
--- a/net/mac80211/rc80211_minstrel_ht_debugfs.c
+++ b/net/mac80211/rc80211_minstrel_ht_debugfs.c
@@ -14,8 +14,6 @@
 #include "rc80211_minstrel.h"
 #include "rc80211_minstrel_ht.h"
 
-extern const struct mcs_group minstrel_mcs_groups[];
-
 static int
 minstrel_ht_stats_open(struct inode *inode, struct file *file)
 {
-- 
1.7.0.1


^ permalink raw reply related

* Re: [PATCH] minstrel_ht: move minstrel_mcs_groups declaration to header file
From: Felix Fietkau @ 2010-06-24 17:26 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless
In-Reply-To: <1277399207-17462-1-git-send-email-linville@tuxdriver.com>

On 2010-06-24 7:06 PM, John W. Linville wrote:
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
Acked-by: Felix Fietkau <nbd@openwrt.org>
(for the other minstrel_ht patch as well)

- Felix

^ permalink raw reply

* Re: intel 5100/iwlagn bug in 2.6.35-rc2 during large file transfer
From: Richard Farina @ 2010-06-24 17:30 UTC (permalink / raw)
  To: reinette chatre; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1277399636.25793.2389.camel@rchatre-DESK>

reinette chatre wrote:
> Hi Richard,
>
> On Thu, 2010-06-24 at 09:12 -0700, Richard Farina wrote:
>   
>> reinette chatre wrote:
>>     
>>> On Wed, 2010-06-16 at 19:56 -0700, Richard Farina wrote:
>>>   
>>>       
>>>> The repeated line appears ad infinitum filling my dmesg buffer.  This of 
>>>> hangcheck timer seem to trigger with every large file transfer on my 
>>>> intel 5100.  What would you like me to do to provide a more useful 
>>>> output as this is currently extremely easy to reproduce.  Kernel 2.6.34 
>>>> using compat-wireless stable 2.6.35-rc2
>>>>
>>>> Thanks,
>>>> Rick Farina
>>>>
>>>> phy0: failed to reallocate TX buffer
>>>> phy0: failed to reallocate TX buffer
>>>> phy0: failed to reallocate TX buffer
>>>> phy0: failed to reallocate TX buffer
>>>> phy0: failed to reallocate TX buffer
>>>> phy0: failed to reallocate TX buffer
>>>> phy0: failed to reallocate TX buffer
>>>>     
>>>>         
>>> First mac80211 runs out of memory ... it cannot even allocate enough
>>> memory for a skb header.
>>>
>>>   
>>>       
>>>> net_ratelimit: 22 callbacks suppressed
>>>> __alloc_pages_slowpath: 3799 callbacks suppressed
>>>> swapper: page allocation failure. order:1, mode:0x4020
>>>> Pid: 0, comm: swapper Not tainted 2.6.34-pentoo #5
>>>> Call Trace:
>>>>  <IRQ>  [<ffffffff8109cb74>] __alloc_pages_nodemask+0x571/0x5b9
>>>>  [<ffffffff816732e9>] ? skb_release_data+0xc4/0xc9
>>>>  [<ffffffffa04701e4>] iwlagn_rx_allocate+0x98/0x25a [iwlagn]
>>>>     
>>>>         
>>> Next driver runs out of memory.
>>>
>>> Note that the above are all atomic allocations that fail and should be
>>> able to recover. 
>>>
>>> Is your system low on memory? Are you running applications that take a
>>> lot of memory? Does your wifi connection drop or otherwise suffer at the
>>> time you see these messages?
>>>
>>>   
>>>       
>> I have 4GB of RAM on this system, I often run a VM which wastes like 
>> half that but that still leaves 2GB for linux and I'm running XFCE4 so 
>> not exactly a memory hog. It's possible that firefox leaks ram until I'm 
>> out but that would be a LOT of leak, much more than I usually see.
>>     
>
> There has been an issue with atomic memory allocations ever since
> 2.6.31. This used to be easy to trigger with iwlagn, but we fixed a
> number of issues. There are still issue with any atomic memory
> allocation (not just iwlagn) and this issue is still open. You can find
> more information at https://bugzilla.kernel.org/show_bug.cgi?id=14141
>
>   
>> Yeah, as you may guess these errors cause my wifi connection to slow 
>> drastically.
>>     
>
> The driver, when unable to allocate memory atomically, will reattempt
> the allocation later when it can use GFP_KERNEL. I think there may be
> ways in which we can try to optimize this since right now it will only
> schedule this when there are about 8 buffers remaining. I was looking at
> your trace again and even though you state "Kernel 2.6.34 using
> compat-wireless stable 2.6.35-rc2" ... the trace you provide does not
> seem to match the driver code from 2.6.35-rc2. Could you please confirm
> which version of driver you are running so that I can prepare a patch?
>
>   
There were two compat-wireless releases for 2.6.35_rc2 because Luis had 
asked me to test and then he changed it for the official release.  I'll 
use the official 2.6.35_rc2 release for the current testing so if there 
are any patches you wish to toss my way please base them on that.  The 
other option is you tell me what to do, I can run any kernel, any git 
snapshot, whatever you say.  Like I said, all I have to do is download 
something or transfer something large so it is pretty easily 
reproducible here so I'll test whatever you like.

Thanks,
Rick Farina
>>  If I had to guess, since this happens when I make a large 
>> file transfer it is likely that something related is leaking RAM.  I'm 
>> using wget or axel to download and NFS to dump the files on a NAS.  I'll 
>> try to trigger this again
>>     
>
> Does this happen every time you run this test? I would like to get an
> idea whether we will get a clear indication whether our changes will
> help or not.
>
>   
>>  and watch memory usage to see if I can find 
>> something other than the driver that could be leaking.  Failing that, 
>> what do I need to enable to find a leak in the driver?
>>     
>
> Perhaps kmemleak?
>
> Reinette
>
>
>
>
>   


^ permalink raw reply

* Re: intel 5100/iwlagn bug in 2.6.35-rc2 during large file transfer
From: reinette chatre @ 2010-06-24 17:48 UTC (permalink / raw)
  To: Richard Farina; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <4C23961D.7050500@gmail.com>

On Thu, 2010-06-24 at 10:30 -0700, Richard Farina wrote:
> There were two compat-wireless releases for 2.6.35_rc2 because Luis had 
> asked me to test and then he changed it for the official release.  I'll 
> use the official 2.6.35_rc2 release for the current testing so if there 
> are any patches you wish to toss my way please base them on that.  The 
> other option is you tell me what to do, I can run any kernel, any git 
> snapshot, whatever you say.  Like I said, all I have to do is download 
> something or transfer something large so it is pretty easily 
> reproducible here so I'll test whatever you like.


Could you please try out this patch? It is based on the latest linux-2.6
so should apply cleanly to your latest 2.6.35 stable compat.

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
index 0f292a2..028db74 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
@@ -613,7 +613,7 @@ void iwlagn_rx_allocate(struct iwl_priv *priv, gfp_t priority)
 		}
 		spin_unlock_irqrestore(&rxq->lock, flags);
 
-		if (rxq->free_count > RX_LOW_WATERMARK)
+		if (rxq->free_count > 8)
 			gfp_mask |= __GFP_NOWARN;
 
 		if (priv->hw_params.rx_page_order > 0)
diff --git a/drivers/net/wireless/iwlwifi/iwl-fh.h b/drivers/net/wireless/iwlwifi/iwl-fh.h
index 113c366..396a16c 100644
--- a/drivers/net/wireless/iwlwifi/iwl-fh.h
+++ b/drivers/net/wireless/iwlwifi/iwl-fh.h
@@ -426,7 +426,7 @@
  * RX related structures and functions
  */
 #define RX_FREE_BUFFERS 64
-#define RX_LOW_WATERMARK 8
+#define RX_LOW_WATERMARK 64
 
 /* Size of one Rx buffer in host DRAM */
 #define IWL_RX_BUF_SIZE_3K (3 * 1000) /* 3945 only */



^ permalink raw reply related

* [PATCH 2/2] mac80211: use netif_receive_skb in ieee80211_tx_status callpath
From: John W. Linville @ 2010-06-24 18:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, John W. Linville

This avoids the extra queueing from calling netif_rx.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 net/mac80211/status.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 34da679..10caec5 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -377,7 +377,7 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 				skb2 = skb_clone(skb, GFP_ATOMIC);
 				if (skb2) {
 					skb2->dev = prev_dev;
-					netif_rx(skb2);
+					netif_receive_skb(skb2);
 				}
 			}
 
@@ -386,7 +386,7 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 	}
 	if (prev_dev) {
 		skb->dev = prev_dev;
-		netif_rx(skb);
+		netif_receive_skb(skb);
 		skb = NULL;
 	}
 	rcu_read_unlock();
-- 
1.7.0.1


^ permalink raw reply related

* [PATCH 1/2] mac80211: use netif_receive_skb in ieee80211_rx callpath
From: John W. Linville @ 2010-06-24 18:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, John W. Linville

This avoids the extra queueing from calling netif_rx.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 net/mac80211/rx.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index ab58a5d..d70e1a9 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -293,7 +293,7 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
 			skb2 = skb_clone(skb, GFP_ATOMIC);
 			if (skb2) {
 				skb2->dev = prev_dev;
-				netif_rx(skb2);
+				netif_receive_skb(skb2);
 			}
 		}
 
@@ -304,7 +304,7 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
 
 	if (prev_dev) {
 		skb->dev = prev_dev;
-		netif_rx(skb);
+		netif_receive_skb(skb);
 	} else
 		dev_kfree_skb(skb);
 
@@ -1578,7 +1578,7 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
 			/* deliver to local stack */
 			skb->protocol = eth_type_trans(skb, dev);
 			memset(skb->cb, 0, sizeof(skb->cb));
-			netif_rx(skb);
+			netif_receive_skb(skb);
 		}
 	}
 
@@ -2244,7 +2244,7 @@ static void ieee80211_rx_cooked_monitor(struct ieee80211_rx_data *rx,
 			skb2 = skb_clone(skb, GFP_ATOMIC);
 			if (skb2) {
 				skb2->dev = prev_dev;
-				netif_rx(skb2);
+				netif_receive_skb(skb2);
 			}
 		}
 
@@ -2255,7 +2255,7 @@ static void ieee80211_rx_cooked_monitor(struct ieee80211_rx_data *rx,
 
 	if (prev_dev) {
 		skb->dev = prev_dev;
-		netif_rx(skb);
+		netif_receive_skb(skb);
 		skb = NULL;
 	} else
 		goto out_free_skb;
-- 
1.7.0.1


^ permalink raw reply related

* Re: [PATCH 0/3] b43: logging cleanups
From: John W. Linville @ 2010-06-24 18:53 UTC (permalink / raw)
  To: Joe Perches
  Cc: Stefano Brivio, linux-wireless, netdev, linux-kernel,
	Larry.Finger, mb, zajec5
In-Reply-To: <cover.1276988387.git.joe@perches.com>

On Sat, Jun 19, 2010 at 04:30:08PM -0700, Joe Perches wrote:
> Just some small cleanups
> 
> Joe Perches (3):
>   drivers/net/wireless/b43: Use local ratelimit_state
>   drivers/net/wireless/b43: Logging cleanups
>   drivers/net/wireless/b43: Rename b43_debug to b43_debugging

Any of the b43 guys want to express an opinion on these?

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox