2009-05-27 05:48:10

by Jeff Hansen

[permalink] [raw]
Subject: [PATCH 1/4] ath9k: Reset SC_OP_TSF_RESET flag after stuck beacon

I have a TrendNet 652-BRP running OpenWRT + ath9k very well. The only
problem is that the beacon gets stuck maybe once a day. After
Vasanthakumar Thiagarajan's "ath9k: cleanup beacon parameters
configuration" patch, ath9k would nearly re-configure the beacons after it
detected the stuck beacon, and did a reset. But it would fail the
SC_OP_TSF_RESET check in ath_beacon_config_ap. This patch gets the beacon
fully reconfigured after the reset.

Signed-off-by: Jeff Hansen <[email protected]>
---
drivers/net/wireless/ath/ath9k/beacon.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index a21b213..0c67ed2 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -411,6 +411,7 @@ void ath_beacon_tasklet(unsigned long data)
} else if (sc->beacon.bmisscnt >= BSTUCK_THRESH) {
DPRINTF(sc, ATH_DBG_BEACON,
"beacon is officially stuck\n");
+ sc->sc_flags |= SC_OP_TSF_RESET;
ath_reset(sc, false);
}

--
1.6.0.4




2009-05-27 05:48:10

by Jeff Hansen

[permalink] [raw]
Subject: [PATCH 4/4] mac80211: Fix typos in "set default QoS values according to spec"

Some of the acwmin and acwmax values were backwards in the original patch,
which cut TX performance (on my device at least) down from ~25-30Mbps to
2Mbps. Performance is back up after this commit.

Signed-off-by: Jeff Hansen <[email protected]>
---
net/mac80211/util.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 949d857..d096528 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -657,20 +657,20 @@ void ieee80211_set_wmm_default(struct ieee80211_sub_if_data *sdata)

switch (queue) {
case 3: /* AC_BK */
- qparam.cw_max = aCWmin;
- qparam.cw_min = aCWmax;
+ qparam.cw_max = aCWmax;
+ qparam.cw_min = aCWmin;
qparam.txop = 0;
qparam.aifs = 7;
break;
default: /* never happens but let's not leave undefined */
case 2: /* AC_BE */
- qparam.cw_max = aCWmin;
- qparam.cw_min = aCWmax;
+ qparam.cw_max = aCWmax;
+ qparam.cw_min = aCWmin;
qparam.txop = 0;
qparam.aifs = 3;
break;
case 1: /* AC_VI */
- qparam.cw_max = aCWmin;
+ qparam.cw_max = aCWmax;
qparam.cw_min = (aCWmin + 1) / 2 - 1;
if (use_11b)
qparam.txop = 6016/32;
@@ -679,7 +679,7 @@ void ieee80211_set_wmm_default(struct ieee80211_sub_if_data *sdata)
qparam.aifs = 2;
break;
case 0: /* AC_VO */
- qparam.cw_max = (aCWmin + 1) / 2 - 1;
+ qparam.cw_max = (aCWmax + 1) / 2 - 1;
qparam.cw_min = (aCWmin + 1) / 4 - 1;
if (use_11b)
qparam.txop = 3264/32;
--
1.6.0.4



2009-05-27 05:48:10

by Jeff Hansen

[permalink] [raw]
Subject: [PATCH 3/4] ath9k: Add "debug" file to debugfs

This patch adds the debug file to the ath9k debugfs, which lets you modify
the debug_mask at runtime, without having to reload the ath9k module.

Signed-off-by: Jeff Hansen <[email protected]>
---
drivers/net/wireless/ath/ath9k/debug.c | 38 ++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath9k/debug.h | 1 +
2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index d50821c..e2ff4a2 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -45,6 +45,38 @@ static int ath9k_debugfs_open(struct inode *inode, struct file *file)
return 0;
}

+static ssize_t read_file_debug(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ char buf[32];
+ unsigned int len = 0;
+ len += snprintf(buf, sizeof(buf), "0x%08x\n", sc->debug.debug_mask);
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t write_file_debug(struct file *file, const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ unsigned long mask;
+ char buf[32];
+ if (copy_from_user(buf, user_buf, (sizeof(buf) - 1) < count ?
+ (sizeof(buf) - 1) : count))
+ return 0;
+ buf[sizeof(buf)-1] = 0;
+ if (strict_strtoul(buf, 0, &mask) == 0)
+ sc->debug.debug_mask = mask;
+ return count;
+}
+
+static const struct file_operations fops_debug = {
+ .read = read_file_debug,
+ .write = write_file_debug,
+ .open = ath9k_debugfs_open,
+ .owner = THIS_MODULE
+};
+
static ssize_t read_file_dma(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
@@ -462,6 +494,11 @@ int ath9k_init_debug(struct ath_softc *sc)
if (!sc->debug.debugfs_phy)
goto err;

+ sc->debug.debugfs_debug = debugfs_create_file("debug",
+ S_IRUGO | S_IWUSR, sc->debug.debugfs_phy, sc, &fops_debug);
+ if (!sc->debug.debugfs_debug)
+ goto err;
+
sc->debug.debugfs_dma = debugfs_create_file("dma", S_IRUGO,
sc->debug.debugfs_phy, sc, &fops_dma);
if (!sc->debug.debugfs_dma)
@@ -499,6 +536,7 @@ void ath9k_exit_debug(struct ath_softc *sc)
debugfs_remove(sc->debug.debugfs_rcstat);
debugfs_remove(sc->debug.debugfs_interrupt);
debugfs_remove(sc->debug.debugfs_dma);
+ debugfs_remove(sc->debug.debugfs_debug);
debugfs_remove(sc->debug.debugfs_phy);
}

diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index cf9146a..edda15b 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -95,6 +95,7 @@ struct ath_stats {
struct ath9k_debug {
int debug_mask;
struct dentry *debugfs_phy;
+ struct dentry *debugfs_debug;
struct dentry *debugfs_dma;
struct dentry *debugfs_interrupt;
struct dentry *debugfs_rcstat;
--
1.6.0.4



Subject: Re: [PATCH 2/4] ath9k: Combine legacy and 11n rc statistics

On Wed, May 27, 2009 at 10:35:13AM +0530, Jeff Hansen wrote:
> + max = 80 + sc->cur_rate_table->rate_cnt * 64;
> + buf = vmalloc(max + 1);

I think kmalloc() is the better option here, you are not even
allocating a page, you are allocating 2952B at max.

Vasanth


2009-05-27 05:48:10

by Jeff Hansen

[permalink] [raw]
Subject: [PATCH 2/4] ath9k: Combine legacy and 11n rc statistics

This patch combines the legacy and 11n rcstats into one, using the normal
rate table indices instead of two separate indices for each mode. Legacy
rates also get all of the PER and retry information, now, too.

Signed-off-by: Jeff Hansen <[email protected]>
---
drivers/net/wireless/ath/ath9k/debug.c | 112 ++++++++++----------------------
drivers/net/wireless/ath/ath9k/debug.h | 9 +--
2 files changed, 36 insertions(+), 85 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 97df20c..d50821c 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -15,6 +15,7 @@
*/

#include <asm/unaligned.h>
+#include <linux/vmalloc.h>

#include "ath9k.h"

@@ -224,111 +225,66 @@ static const struct file_operations fops_interrupt = {
.owner = THIS_MODULE
};

-static void ath_debug_stat_11n_rc(struct ath_softc *sc, struct sk_buff *skb)
-{
- struct ath_tx_info_priv *tx_info_priv = NULL;
- struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
- struct ieee80211_tx_rate *rates = tx_info->status.rates;
- int final_ts_idx, idx;
-
- tx_info_priv = ATH_TX_INFO_PRIV(tx_info);
- final_ts_idx = tx_info_priv->tx.ts_rateindex;
- idx = sc->cur_rate_table->info[rates[final_ts_idx].idx].dot11rate;
-
- sc->debug.stats.n_rcstats[idx].success++;
-}
-
-static void ath_debug_stat_legacy_rc(struct ath_softc *sc, struct sk_buff *skb)
+void ath_debug_stat_rc(struct ath_softc *sc, struct sk_buff *skb)
{
struct ath_tx_info_priv *tx_info_priv = NULL;
struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
struct ieee80211_tx_rate *rates = tx_info->status.rates;
int final_ts_idx, idx;
+ struct ath_rc_stats *stats;

tx_info_priv = ATH_TX_INFO_PRIV(tx_info);
final_ts_idx = tx_info_priv->tx.ts_rateindex;
idx = rates[final_ts_idx].idx;
-
- sc->debug.stats.legacy_rcstats[idx].success++;
+ stats = &sc->debug.stats.rcstats[idx];
+ stats->success++;
}

-void ath_debug_stat_rc(struct ath_softc *sc, struct sk_buff *skb)
-{
- if (conf_is_ht(&sc->hw->conf))
- ath_debug_stat_11n_rc(sc, skb);
- else
- ath_debug_stat_legacy_rc(sc, skb);
-}
-
-/* FIXME: legacy rates, later on .. */
void ath_debug_stat_retries(struct ath_softc *sc, int rix,
int xretries, int retries, u8 per)
{
- if (conf_is_ht(&sc->hw->conf)) {
- int idx = sc->cur_rate_table->info[rix].dot11rate;
+ struct ath_rc_stats *stats = &sc->debug.stats.rcstats[rix];

- sc->debug.stats.n_rcstats[idx].xretries += xretries;
- sc->debug.stats.n_rcstats[idx].retries += retries;
- sc->debug.stats.n_rcstats[idx].per = per;
- }
+ stats->xretries += xretries;
+ stats->retries += retries;
+ stats->per = per;
}

-static ssize_t ath_read_file_stat_11n_rc(struct file *file,
- char __user *user_buf,
- size_t count, loff_t *ppos)
+static ssize_t read_file_rcstat(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
{
struct ath_softc *sc = file->private_data;
- char buf[1024];
- unsigned int len = 0;
+ char *buf;
+ unsigned int len = 0, max;
int i = 0;
+ ssize_t retval;

- len += sprintf(buf, "%7s %13s %8s %8s %6s\n\n", "Rate", "Success",
- "Retries", "XRetries", "PER");
-
- for (i = 0; i <= 15; i++) {
- len += snprintf(buf + len, sizeof(buf) - len,
- "%5s%3d: %8u %8u %8u %8u\n", "MCS", i,
- sc->debug.stats.n_rcstats[i].success,
- sc->debug.stats.n_rcstats[i].retries,
- sc->debug.stats.n_rcstats[i].xretries,
- sc->debug.stats.n_rcstats[i].per);
- }
-
- return simple_read_from_buffer(user_buf, count, ppos, buf, len);
-}
+ if (sc->cur_rate_table == NULL)
+ return 0;

-static ssize_t ath_read_file_stat_legacy_rc(struct file *file,
- char __user *user_buf,
- size_t count, loff_t *ppos)
-{
- struct ath_softc *sc = file->private_data;
- char buf[512];
- unsigned int len = 0;
- int i = 0;
+ max = 80 + sc->cur_rate_table->rate_cnt * 64;
+ buf = vmalloc(max + 1);
+ if (buf == NULL)
+ return 0;
+ buf[max] = 0;

- len += sprintf(buf, "%7s %13s\n\n", "Rate", "Success");
+ len += sprintf(buf, "%5s %15s %8s %9s %3s\n\n", "Rate", "Success",
+ "Retries", "XRetries", "PER");

for (i = 0; i < sc->cur_rate_table->rate_cnt; i++) {
- len += snprintf(buf + len, sizeof(buf) - len, "%5u: %12u\n",
- sc->cur_rate_table->info[i].ratekbps / 1000,
- sc->debug.stats.legacy_rcstats[i].success);
+ u32 ratekbps = sc->cur_rate_table->info[i].ratekbps;
+ struct ath_rc_stats *stats = &sc->debug.stats.rcstats[i];
+
+ len += snprintf(buf + len, max - len,
+ "%3u.%d: %8u %8u %8u %8u\n", ratekbps / 1000,
+ (ratekbps % 1000) / 100, stats->success,
+ stats->retries, stats->xretries,
+ stats->per);
}

- return simple_read_from_buffer(user_buf, count, ppos, buf, len);
-}
-
-static ssize_t read_file_rcstat(struct file *file, char __user *user_buf,
- size_t count, loff_t *ppos)
-{
- struct ath_softc *sc = file->private_data;
-
- if (sc->cur_rate_table == NULL)
- return 0;
-
- if (conf_is_ht(&sc->hw->conf))
- return ath_read_file_stat_11n_rc(file, user_buf, count, ppos);
- else
- return ath_read_file_stat_legacy_rc(file, user_buf, count ,ppos);
+ retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+ vfree(buf);
+ return retval;
}

static const struct file_operations fops_rcstat = {
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index db845cf..cf9146a 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -80,11 +80,7 @@ struct ath_interrupt_stats {
u32 dtim;
};

-struct ath_legacy_rc_stats {
- u32 success;
-};
-
-struct ath_11n_rc_stats {
+struct ath_rc_stats {
u32 success;
u32 retries;
u32 xretries;
@@ -93,8 +89,7 @@ struct ath_11n_rc_stats {

struct ath_stats {
struct ath_interrupt_stats istats;
- struct ath_legacy_rc_stats legacy_rcstats[12]; /* max(11a,11b,11g) */
- struct ath_11n_rc_stats n_rcstats[16]; /* 0..15 MCS rates */
+ struct ath_rc_stats rcstats[RATE_TABLE_SIZE];
};

struct ath9k_debug {
--
1.6.0.4



2009-05-27 05:12:54

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH 4/4] mac80211: Fix typos in "set default QoS values according to spec"

On Tue, 2009-05-26 at 22:05 -0700, Jeff Hansen wrote:
> Some of the acwmin and acwmax values were backwards in the original patch,
> which cut TX performance (on my device at least) down from ~25-30Mbps to
> 2Mbps. Performance is back up after this commit.

Thanks for reporting these and verifying that this fixes the issue.
However, not all of these are typos.. In addition,
[email protected] should be cc'ed.

> diff --git a/net/mac80211/util.c b/net/mac80211/util.c

> @@ -657,20 +657,20 @@ void ieee80211_set_wmm_default(struct ieee80211_sub_if_data *sdata)
>
> switch (queue) {
> case 3: /* AC_BK */
> - qparam.cw_max = aCWmin;
> - qparam.cw_min = aCWmax;
> + qparam.cw_max = aCWmax;
> + qparam.cw_min = aCWmin;
> qparam.txop = 0;
> qparam.aifs = 7;
> break;
> default: /* never happens but let's not leave undefined */
> case 2: /* AC_BE */
> - qparam.cw_max = aCWmin;
> - qparam.cw_min = aCWmax;
> + qparam.cw_max = aCWmax;
> + qparam.cw_min = aCWmin;
> qparam.txop = 0;
> qparam.aifs = 3;
> break;

These are correct fixes.

> case 1: /* AC_VI */
> - qparam.cw_max = aCWmin;
> + qparam.cw_max = aCWmax;
> qparam.cw_min = (aCWmin + 1) / 2 - 1;
> if (use_11b)
> qparam.txop = 6016/32;
> @@ -679,7 +679,7 @@ void ieee80211_set_wmm_default(struct ieee80211_sub_if_data *sdata)
> qparam.aifs = 2;
> break;
> case 0: /* AC_VO */
> - qparam.cw_max = (aCWmin + 1) / 2 - 1;
> + qparam.cw_max = (aCWmax + 1) / 2 - 1;
> qparam.cw_min = (aCWmin + 1) / 4 - 1;
> if (use_11b)
> qparam.txop = 3264/32;

These were actually already correct (yes, it may look like a typo, but
it is not, both of VI and VO are indeed using aCWmin in calculation for
both min and max values) and should not be changed.

- Jouni