Date: Mon, 14 Feb 2011 08:20:11 GMT From: dfilter@FreeBSD.ORG (dfilter service) To: freebsd-net@FreeBSD.org Subject: Re: kern/153938: commit references a PR Message-ID: <201102140820.p1E8KBlB093432@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/153938; it has been noted by GNATS. From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/153938: commit references a PR Date: Mon, 14 Feb 2011 08:14:15 +0000 (UTC) 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 *); _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201102140820.p1E8KBlB093432>