Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 14 Feb 2011 08:14:07 +0000 (UTC)
From:      Hans Petter Selasky <hselasky@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r218676 - head/sys/dev/usb/wlan
Message-ID:  <201102140814.p1E8E7RI084575@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: hselasky
Date: Mon Feb 14 08:14:06 2011
New Revision: 218676
URL: http://svn.freebsd.org/changeset/base/218676

Log:
  * Fix page fault caused by referring freed node.
  
  While updating Tx stats, already freed node could be referred and cause
  page fault. To avoid such panic, spool Tx stats in driver's softc. Then,
  on every ratectl interval, grab node though ieee80211_iterate_nodes() and
  update ratectl stats.
  
  * Simplify some code in run_iter_func().
  
  * Fix typo
  
  * Use memset instead of bzero (hselasky @)
  
  PR:		kern/153938
  Submitted by:	PseudoCylon <moonlightakkiy@yahoo.ca>
  Approved by:	thompsa (mentor)

Modified:
  head/sys/dev/usb/wlan/if_run.c
  head/sys/dev/usb/wlan/if_runreg.h
  head/sys/dev/usb/wlan/if_runvar.h

Modified: head/sys/dev/usb/wlan/if_run.c
==============================================================================
--- head/sys/dev/usb/wlan/if_run.c	Mon Feb 14 08:09:02 2011	(r218675)
+++ head/sys/dev/usb/wlan/if_run.c	Mon Feb 14 08:14:06 2011	(r218676)
@@ -1684,7 +1684,7 @@ run_read_eeprom(struct run_softc *sc)
 	return (0);
 }
 
-struct ieee80211_node *
+static struct ieee80211_node *
 run_node_alloc(struct ieee80211vap *vap, const uint8_t mac[IEEE80211_ADDR_LEN])
 {
 	return malloc(sizeof (struct run_node), M_DEVBUF, M_NOWAIT | M_ZERO);
@@ -1787,7 +1787,6 @@ run_newstate(struct ieee80211vap *vap, e
 		}
 		break;
 
-
 	case IEEE80211_S_RUN:
 		if (!(sc->runbmap & bid)) {
 			if(sc->running++)
@@ -2229,10 +2228,10 @@ run_drain_fifo(void *arg)
 {
 	struct run_softc *sc = arg;
 	struct ifnet *ifp = sc->sc_ifp;
-	struct ieee80211_node *ni = sc->sc_ni[0];	/* make compiler happy */
 	uint32_t stat;
-	int retrycnt = 0;
+	uint16_t (*wstat)[3];
 	uint8_t wcid, mcs, pid;
+	int8_t retry;
 
 	RUN_LOCK_ASSERT(sc, MA_OWNED);
 
@@ -2250,31 +2249,32 @@ run_drain_fifo(void *arg)
 		    wcid == 0)
 			continue;
 
-		ni = sc->sc_ni[wcid];
-		if (ni->ni_rctls == NULL)
-			continue;
-
-		/* update per-STA AMRR stats */
-		if (stat & RT2860_TXQ_OK) {
-			/*
-			 * Check if there were retries, ie if the Tx
-			 * success rate is different from the requested
-			 * rate. Note that it works only because we do
-			 * not allow rate fallback from OFDM to CCK.
-			 */
-			mcs = (stat >> RT2860_TXQ_MCS_SHIFT) & 0x7f;
-			pid = (stat >> RT2860_TXQ_PID_SHIFT) & 0xf;
-			if (mcs + 1 != pid)
-				retrycnt = 1;
-			ieee80211_ratectl_tx_complete(ni->ni_vap, ni,
-			    IEEE80211_RATECTL_TX_SUCCESS,
-			    &retrycnt, NULL);
-		} else {
-			retrycnt = 1;
-			ieee80211_ratectl_tx_complete(ni->ni_vap, ni,
-			    IEEE80211_RATECTL_TX_FAILURE,
-			    &retrycnt, NULL);
+		/*
+		 * Even though each stat is Tx-complete-status like format,
+		 * the device can poll stats. Because there is no guarantee
+		 * that the referring node is still around when read the stats.
+		 * So that, if we use ieee80211_ratectl_tx_update(), we will
+		 * have hard time not to refer already freed node.
+		 *
+		 * To eliminate such page faults, we poll stats in softc.
+		 * Then, update the rates later with ieee80211_ratectl_tx_update().
+		 */
+		wstat = &(sc->wcid_stats[wcid]);
+		(*wstat)[RUN_TXCNT]++;
+		if (stat & RT2860_TXQ_OK)
+			(*wstat)[RUN_SUCCESS]++;
+		else
 			ifp->if_oerrors++;
+		/*
+		 * Check if there were retries, ie if the Tx success rate is
+		 * different from the requested rate. Note that it works only
+		 * because we do not allow rate fallback from OFDM to CCK.
+		 */
+		mcs = (stat >> RT2860_TXQ_MCS_SHIFT) & 0x7f;
+		pid = (stat >> RT2860_TXQ_PID_SHIFT) & 0xf;
+		if ((retry = pid -1 - mcs) > 0) {
+			(*wstat)[RUN_TXCNT] += retry;
+			(*wstat)[RUN_RETRY] += retry;
 		}
 	}
 	DPRINTFN(3, "count=%d\n", sc->fifo_cnt);
@@ -2290,46 +2290,51 @@ run_iter_func(void *arg, struct ieee8021
 	struct ieee80211com *ic = ni->ni_ic;
 	struct ifnet *ifp = ic->ic_ifp;
 	struct run_node *rn = (void *)ni;
-	uint32_t sta[3];
-	int txcnt = 0, success = 0, retrycnt = 0;
-	int error;
+	union run_stats sta[2];
+	uint16_t (*wstat)[3];
+	int txcnt, success, retrycnt, error;
+
+	RUN_LOCK(sc);
 
 	if (sc->rvp_cnt <= 1 && (vap->iv_opmode == IEEE80211_M_IBSS ||
 	    vap->iv_opmode == IEEE80211_M_STA)) {
-		RUN_LOCK(sc);
-
 		/* read statistic counters (clear on read) and update AMRR state */
 		error = run_read_region_1(sc, RT2860_TX_STA_CNT0, (uint8_t *)sta,
 		    sizeof sta);
 		if (error != 0)
-			return;
-
-		DPRINTFN(3, "retrycnt=%d txcnt=%d failcnt=%d\n",
-		    le32toh(sta[1]) >> 16, le32toh(sta[1]) & 0xffff,
-		    le32toh(sta[0]) & 0xffff);
+			goto fail;
 
 		/* count failed TX as errors */
-		ifp->if_oerrors += le32toh(sta[0]) & 0xffff;
+		ifp->if_oerrors += le16toh(sta[0].error.fail);
 
-		retrycnt =
-		    (le32toh(sta[0]) & 0xffff) +	/* failed TX count */
-		    (le32toh(sta[1]) >> 16);		/* TX retransmission count */
-
-		txcnt =
-		    retrycnt +
-		    (le32toh(sta[1]) & 0xffff);		/* successful TX count */
-
-		success =
-		    (le32toh(sta[1]) >> 16) +
-		    (le32toh(sta[1]) & 0xffff);
+		retrycnt = le16toh(sta[1].tx.retry);
+		success = le16toh(sta[1].tx.success);
+		txcnt = retrycnt + success + le16toh(sta[0].error.fail);
 
-		ieee80211_ratectl_tx_update(vap, ni, &txcnt, &success,
-		    &retrycnt);
+		DPRINTFN(3, "retrycnt=%d success=%d failcnt=%d\n",
+			retrycnt, success, le16toh(sta[0].error.fail));
+	} else {
+		wstat = &(sc->wcid_stats[RUN_AID2WCID(ni->ni_associd)]);
 
-		RUN_UNLOCK(sc);
+		if (wstat == &(sc->wcid_stats[0]) ||
+		    wstat > &(sc->wcid_stats[RT2870_WCID_MAX]))
+			goto fail;
+
+		txcnt = (*wstat)[RUN_TXCNT];
+		success = (*wstat)[RUN_SUCCESS];
+		retrycnt = (*wstat)[RUN_RETRY];
+		DPRINTFN(3, "retrycnt=%d txcnt=%d success=%d\n",
+		    retrycnt, txcnt, success);
+
+		memset(wstat, 0, sizeof(*wstat));
 	}
 
+	ieee80211_ratectl_tx_update(vap, ni, &txcnt, &success, &retrycnt);
 	rn->amrr_ridx = ieee80211_ratectl_rate(ni, NULL, 0);
+
+fail:
+	RUN_UNLOCK(sc);
+
 	DPRINTFN(3, "ridx=%d\n", rn->amrr_ridx);
 }
 
@@ -2345,6 +2350,8 @@ run_newassoc_cb(void *arg)
 
 	run_write_region_1(sc, RT2860_WCID_ENTRY(wcid),
 	    ni->ni_macaddr, IEEE80211_ADDR_LEN);
+
+	memset(&(sc->wcid_stats[wcid]), 0, sizeof(sc->wcid_stats[wcid]));
 }
 
 static void
@@ -2384,8 +2391,6 @@ run_newassoc(struct ieee80211_node *ni, 
 	DPRINTF("new assoc isnew=%d associd=%x addr=%s\n",
 	    isnew, ni->ni_associd, ether_sprintf(ni->ni_macaddr));
 
-	sc->sc_ni[wcid] = ni;
-
 	for (i = 0; i < rs->rs_nrates; i++) {
 		rate = rs->rs_rates[i] & IEEE80211_RATE_VAL;
 		/* convert 802.11 rate to hardware rate index */

Modified: head/sys/dev/usb/wlan/if_runreg.h
==============================================================================
--- head/sys/dev/usb/wlan/if_runreg.h	Mon Feb 14 08:09:02 2011	(r218675)
+++ head/sys/dev/usb/wlan/if_runreg.h	Mon Feb 14 08:14:06 2011	(r218676)
@@ -1208,4 +1208,17 @@ static const struct rt2860_rate {
 	{ 30, 0x09 },	\
 	{ 31, 0x10 }
 
+
+union run_stats {
+	uint32_t	raw;
+	struct {
+		uint16_t	fail;
+		uint16_t	pad;
+	} error;
+	struct {
+		uint16_t	success;
+		uint16_t	retry;
+	} tx;
+} __aligned(4);
+
 #endif	/* _IF_RUNREG_H_ */

Modified: head/sys/dev/usb/wlan/if_runvar.h
==============================================================================
--- head/sys/dev/usb/wlan/if_runvar.h	Mon Feb 14 08:09:02 2011	(r218675)
+++ head/sys/dev/usb/wlan/if_runvar.h	Mon Feb 14 08:14:06 2011	(r218676)
@@ -160,7 +160,10 @@ struct run_softc {
 	device_t			sc_dev;
 	struct usb_device		*sc_udev;
 	struct ifnet			*sc_ifp;
-	struct ieee80211_node		*sc_ni[RT2870_WCID_MAX + 1];
+	uint16_t			wcid_stats[RT2870_WCID_MAX + 1][3];
+#define	RUN_TXCNT	0
+#define	RUN_SUCCESS	1
+#define	RUN_RETRY	2
 
 	int				(*sc_srom_read)(struct run_softc *,
 					    uint16_t, uint16_t *);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201102140814.p1E8E7RI084575>