ath9k: Keep track of stations for debugfs.

From: Ben Greear <[email protected]>

The stations hold the ath_node, which holds the tid and other xmit logic structures. In order to debug stuck xmit logic, we need a way to print out the tid state for the stations.

---

v1 -> v2: Use linked list instead of array. Protect with spinlock.

:100644 100644 deda815... 3f5c513... M drivers/net/wireless/ath/ath9k/ath9k.h
:100644 100644 faf84e4... 650f00f... M drivers/net/wireless/ath/ath9k/debug.c
:100644 100644 23b2998... 59c01ca... M drivers/net/wireless/ath/ath9k/init.c
:100644 100644 b716ffb... 8684afa... M drivers/net/wireless/ath/ath9k/main.c

 drivers/net/wireless/ath/ath9k/ath9k.h |    7 ++-
 drivers/net/wireless/ath/ath9k/debug.c |   94 +++++++++++++++++++++++++++++++-
 drivers/net/wireless/ath/ath9k/init.c  |    4 ++
 drivers/net/wireless/ath/ath9k/main.c  |   13 +++++
 4 files changed, 115 insertions(+), 3 deletions(-)

index deda815..3f5c513 100644
@@ -254,7 +254,10 @@ struct ath_atx_tid {
 };
 
 struct ath_node {
-	struct ath_common *common;
+#ifdef CONFIG_ATH9K_DEBUGFS
+	struct list_head list; /* for sc->nodes */
+	struct ieee80211_sta *sta; /* station struct we're part of */
+#endif
 	struct ath_atx_tid tid[WME_NUM_TID];
 	struct ath_atx_ac ac[WME_NUM_AC];
 	u16 maxampdu;

@@ -624,6 +627,8 @@ struct ath_softc {
 
 #ifdef CONFIG_ATH9K_DEBUGFS
 	struct ath9k_debug debug;
+	spinlock_t nodes_lock;
+	struct list_head nodes; /* basically, stations */
 #endif
 	struct ath_beacon_config cur_beacon_conf;
 	struct delayed_work tx_complete_work;

index 23b2998..59c01ca 100644
@@ -559,6 +559,10 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc, u16 subsysid,
 	spin_lock_init(&sc->sc_serial_rw);
 	spin_lock_init(&sc->sc_pm_lock);
 	mutex_init(&sc->mutex);
+#ifdef CONFIG_ATH9K_DEBUGFS
+	spin_lock_init(&sc->nodes_lock);
+	INIT_LIST_HEAD(&sc->nodes);
+#endif
 	tasklet_init(&sc->intr_tq, ath9k_tasklet, (unsigned long)sc);
 	tasklet_init(&sc->bcon_tasklet, ath_beacon_tasklet,
 		     (unsigned long)sc);

index b716ffb..8684afa 100644
@@ -545,6 +545,12 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta)
 	struct ath_hw *ah = sc->sc_ah;
 	an = (struct ath_node *)sta->drv_priv;
 
+#ifdef CONFIG_ATH9K_DEBUGFS
+	spin_lock(&sc->nodes_lock);
+	list_add(&an->list, &sc->nodes);
+	spin_unlock(&sc->nodes_lock);
+	an->sta = sta;
+#endif
 	if ((ah->caps.hw_caps) & ATH9K_HW_CAP_APM)
 		sc->sc_flags |= SC_OP_ENABLE_APM;
 

@@ -560,6 +566,13 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta)
 {
 	struct ath_node *an = (struct ath_node *)sta->drv_priv;
 
+#ifdef CONFIG_ATH9K_DEBUGFS
+	spin_lock(&sc->nodes_lock);
+	list_del(&an->list);
+	spin_unlock(&sc->nodes_lock);
+	an->sta = NULL;
+#endif
+
 	if (sc->sc_flags & SC_OP_TXAGGR)
 		ath_tx_node_cleanup(sc, an);
 }

1.7.2.3

Again, see my comments about the # STAs limit. I think this can go in as a cfg80211 driver limitation which can be exposed. If you want to go over the supported number (known to work, safe, call it what you want) then a kconfig option can be used.

Luis

Either way, it's a separate patch.

The last thing I want to do is to make it harder to reproduce bugs, so if 60 causes issues, and we made the default limit to 32, then it just makes it that much harder for someone to reproduce bugs and fix them (twiddle kconfig, re-compile kernel, etc).

I surely can't stop you from putting in a similar patch, but I'm going to focus my own efforts on fixing problems I've already found.

Thanks,
Ben

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com

I disagree with putting in an essentially arbitrary restriction based on what has been tested, unless we can actually point at a *specific* limitation that makes the limit necessary.
If ath9k gets unstable with too many STA interfaces, then we should identify the underlying cause or fix it.

- Felix

There's a todo list item that is more generic -- exposing the possible interface combinations including their counts. So if anything, we should start working on that, although I agree that if the limit is just due to bugs, it shouldn't be advertised.

johannes