From owner-svn-src-user@FreeBSD.ORG Mon Jun 13 00:55:29 2011 Return-Path: Delivered-To: svn-src-user@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A1A63106566B; Mon, 13 Jun 2011 00:55:29 +0000 (UTC) (envelope-from adrian@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 919588FC16; Mon, 13 Jun 2011 00:55:29 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id p5D0tTNh048460; Mon, 13 Jun 2011 00:55:29 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p5D0tT1T048456; Mon, 13 Jun 2011 00:55:29 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201106130055.p5D0tT1T048456@svn.freebsd.org> From: Adrian Chadd Date: Mon, 13 Jun 2011 00:55:29 +0000 (UTC) To: src-committers@freebsd.org, svn-src-user@freebsd.org X-SVN-Group: user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r223028 - user/adrian/if_ath_tx/sys/dev/ath X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Jun 2011 00:55:29 -0000 Author: adrian Date: Mon Jun 13 00:55:29 2011 New Revision: 223028 URL: http://svn.freebsd.org/changeset/base/223028 Log: Add new mutexes - one protecting the sc txqueue slist and one for ath_node Since ath_node now contains useful data that will be accessed concurrently, protect it with a mutex. Migrate the sc->sc_txnodeq out of ATH_LOCK and behind its own lock. Since the rate control logic uses state in ath_node (ie, "stuff" attached after it), add some mutexes to the rate control calls. It's likely the locking isn't completely correct yet. Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath.c user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath.c ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_ath.c Sun Jun 12 23:45:46 2011 (r223027) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath.c Mon Jun 13 00:55:29 2011 (r223028) @@ -741,6 +741,9 @@ ath_attach(u_int16_t devid, struct ath_s /* Setup software TX queue related bits */ STAILQ_INIT(&sc->sc_txnodeq); + snprintf(sc->sc_txnodeq_name, sizeof(sc->sc_txnodeq_name), + "%s: txnodeq\n", device_get_nameunit(sc->sc_dev)); + ATH_TXNODE_LOCK_INIT(sc); if (bootverbose) ieee80211_announce(ic); @@ -793,6 +796,7 @@ ath_detach(struct ath_softc *sc) ath_desc_free(sc); ath_tx_cleanup(sc); ath_hal_detach(sc->sc_ah); /* NB: sets chip in full sleep */ + ATH_TXNODE_LOCK_DESTROY(sc); if_free(ifp); return 0; @@ -3120,6 +3124,11 @@ ath_node_alloc(struct ieee80211vap *vap, } ath_rate_node_init(sc, an); + /* Setup the mutex - there's no associd yet so set the name to NULL */ + snprintf(an->an_name, sizeof(an->an_name), "%s: node %p", + device_get_nameunit(sc->sc_dev), an); + mtx_init(&an->an_mtx, an->an_name, NULL, MTX_DEF); + /* XXX setup ath_tid */ ath_tx_tid_init(sc, an); @@ -3139,6 +3148,7 @@ ath_node_free(struct ieee80211_node *ni) ath_tx_tid_cleanup(sc, ATH_NODE(ni)); ath_rate_node_cleanup(sc, ATH_NODE(ni)); + mtx_destroy(&ATH_NODE(ni)->an_mtx); sc->sc_node_free(ni); } Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Sun Jun 12 23:45:46 2011 (r223027) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Mon Jun 13 00:55:29 2011 (r223028) @@ -649,8 +649,10 @@ ath_tx_normal_setup(struct ath_softc *sc txrate |= rt->info[rix].shortPreamble; try0 = ATH_TXMAXTRY; /* XXX?too many? */ } else { + ATH_NODE_LOCK(an); ath_rate_findrate(sc, an, shortPreamble, pktlen, &rix, &try0, &txrate); + ATH_NODE_UNLOCK(an); sc->sc_txrix = rix; /* for LED blinking */ sc->sc_lastdatarix = rix; /* for fast frames */ if (try0 != ATH_TXMAXTRY) @@ -885,10 +887,12 @@ ath_tx_normal_setup(struct ath_softc *sc * we don't use it. */ if (ismrr) { + ATH_NODE_LOCK(an); if (ath_tx_is_11n(sc)) ath_rate_getxtxrates(sc, an, rix, rate, try); else ath_rate_setupxtxdesc(sc, an, ds, shortPreamble, rix); + ATH_NODE_UNLOCK(an); } if (ath_tx_is_11n(sc)) { @@ -1222,15 +1226,14 @@ bad: * This is done to make it easy for the software scheduler to * find which nodes have data to send. * - * This must be called with the ATH lock held. + * The node and txnode locks should be held. */ static void ath_tx_node_sched(struct ath_softc *sc, struct ath_node *an) { - /* - * XXX sched is serialised behind the ATH lock; not - * XXX a per-node lock. - */ + ATH_NODE_LOCK_ASSERT(an); + ATH_TXNODE_LOCK_ASSERT(sc); + if (an->sched) return; /* already scheduled */ @@ -1243,20 +1246,19 @@ ath_tx_node_sched(struct ath_softc *sc, * Mark the current node as no longer needing to be polled for * TX packets. * - * This must be called with the ATH lock held. + * The node and txnode locks should be held. */ static void ath_tx_node_unsched(struct ath_softc *sc, struct ath_node *an) { - /* - * XXX sched is serialised behind the ATH lock; not - * XXX a per-node lock. - */ + ATH_NODE_LOCK_ASSERT(an); + ATH_TXNODE_LOCK_ASSERT(sc); + if (an->sched == 0) return; - STAILQ_REMOVE(&sc->sc_txnodeq, an, ath_node, an_list); an->sched = 0; + STAILQ_REMOVE(&sc->sc_txnodeq, an, ath_node, an_list); } /* @@ -1302,13 +1304,18 @@ ath_tx_swq(struct ath_softc *sc, struct ATH_TXQ_UNLOCK(atid); - /* Mark the given node/tid as having packets to dequeue */ - ATH_LOCK(sc); + /* + * ATH_TXNODE must be acquired before ATH_NODE is acquired + * if they're both needed. + */ + ATH_TXNODE_LOCK(sc); + ATH_NODE_LOCK(an); /* Bump queued packet counter */ - /* XXX for now, an_qdepth is behind the ATH lock */ - atomic_add_int(&an->an_qdepth, 1); + an->an_qdepth++; + /* Mark the given node as having packets to dequeue */ ath_tx_node_sched(sc, an); - ATH_UNLOCK(sc); + ATH_NODE_UNLOCK(an); + ATH_TXNODE_UNLOCK(sc); } /* @@ -1375,6 +1382,8 @@ ath_tx_tid_txq_unmark(struct ath_softc * * * It can also be called on an active node during an interface * reset or state transition. + * + * This doesn't update an->an_qdepth! */ static void ath_tx_tid_free_pkts(struct ath_softc *sc, struct ath_node *an, @@ -1383,7 +1392,6 @@ ath_tx_tid_free_pkts(struct ath_softc *s struct ath_tid *atid = &an->an_tid[tid]; struct ath_buf *bf; - /* Walk the queue, free frames */ for (;;) { ATH_TXQ_LOCK(atid); @@ -1400,18 +1408,17 @@ ath_tx_tid_free_pkts(struct ath_softc *s /* * Flush all software queued packets for the given node. - * - * This protects ath_node behind ATH_LOCK for now. */ void ath_tx_node_flush(struct ath_softc *sc, struct ath_node *an) { int tid; - ATH_LOCK(sc); + ATH_NODE_LOCK(an); for (tid = 0; tid < IEEE80211_TID_SIZE; tid++) ath_tx_tid_free_pkts(sc, an, tid); - ATH_UNLOCK(sc); + an->an_qdepth = 0; + ATH_NODE_UNLOCK(an); } /* @@ -1444,7 +1451,11 @@ ath_tx_tid_cleanup(struct ath_softc *sc, ath_tx_tid_txq_unmark(sc, an, i); /* Remove any pending hardware TXQ scheduling */ + ATH_NODE_LOCK(an); + ATH_TXNODE_LOCK(sc); ath_tx_node_unsched(sc, an); + ATH_TXNODE_UNLOCK(sc); + ATH_NODE_UNLOCK(an); /* Free mutex */ mtx_destroy(&atid->axq_lock); @@ -1477,7 +1488,9 @@ ath_tx_tid_hw_queue(struct ath_softc *sc ATH_TXQ_UNLOCK(atid); /* Remove from queue */ - atomic_subtract_int(&an->an_qdepth, 1); + ATH_NODE_LOCK(an); + an->an_qdepth--; + ATH_NODE_UNLOCK(an); txq = bf->bf_state.bfs_txq; /* Sanity check! */ @@ -1519,6 +1532,7 @@ ath_tx_hw_queue(struct ath_softc *sc, st static int ath_txq_node_qlen(struct ath_softc *sc, struct ath_node *an) { + ATH_NODE_LOCK_ASSERT(an); return an->an_qdepth; } @@ -1531,17 +1545,17 @@ ath_txq_sched(struct ath_softc *sc) { struct ath_node *an, *next; - /* XXX I'm not happy the ATH lock is held for so long here */ - ATH_LOCK(sc); - + ATH_TXNODE_LOCK(sc); /* Iterate over the list of active nodes, queuing packets */ STAILQ_FOREACH_SAFE(an, &sc->sc_txnodeq, an_list, next) { /* Try dequeueing packets from the current node */ ath_tx_hw_queue(sc, an); /* Are any packets left on the node software queue? Remove */ + ATH_NODE_LOCK(an); if (! ath_txq_node_qlen(sc, an)) ath_tx_node_unsched(sc, an); + ATH_NODE_UNLOCK(an); } - ATH_UNLOCK(sc); + ATH_TXNODE_UNLOCK(sc); } Modified: user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h Sun Jun 12 23:45:46 2011 (r223027) +++ user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h Mon Jun 13 00:55:29 2011 (r223028) @@ -94,7 +94,7 @@ struct ath_tid { STAILQ_HEAD(,ath_buf) axq_q; /* pending buffers */ u_int axq_depth; /* queue depth (stat only) */ struct mtx axq_lock; /* lock on queue, tx_buf */ - char axq_name[24]; /* e.g. "ath0_a1_t5" */ + char axq_name[24]; /* e.g. "wlan0_a1_t5" */ struct ath_buf *tx_buf[ATH_TID_MAX_BUFS]; /* active tx buffers, beginning at current BAW */ }; @@ -110,6 +110,8 @@ struct ath_node { struct ath_buf *an_ff_buf[WME_NUM_AC]; /* ff staging area */ struct ath_tid an_tid[IEEE80211_TID_SIZE]; /* per-TID state */ u_int an_qdepth; /* Current queue depth of all TIDs */ + char an_name[32]; /* eg "wlan0_a1" */ + struct mtx an_mtx; /* protecting the ath_node state */ /* variable-length rate control state follows */ }; #define ATH_NODE(ni) ((struct ath_node *)(ni)) @@ -204,6 +206,10 @@ struct ath_txq { char axq_name[12]; /* e.g. "ath0_txq4" */ }; +#define ATH_NODE_LOCK(_an) mtx_lock(&(_an)->an_mtx) +#define ATH_NODE_UNLOCK(_an) mtx_unlock(&(_an)->an_mtx) +#define ATH_NODE_LOCK_ASSERT(_an) mtx_assert(&(_an)->an_mtx, MA_OWNED) + #define ATH_TXQ_LOCK_INIT(_sc, _tq) do { \ snprintf((_tq)->axq_name, sizeof((_tq)->axq_name), "%s_txq%u", \ device_get_nameunit((_sc)->sc_dev), (_tq)->axq_qnum); \ @@ -403,7 +409,9 @@ struct ath_softc { struct task sc_dfstask; /* DFS processing task */ /* Software TX queue related state */ + struct mtx sc_txnodeq_mtx; /* mutex protecting the below */ STAILQ_HEAD(, ath_node) sc_txnodeq; /* Nodes which have traffic to send */ + char sc_txnodeq_name[16]; /* mutex name */ }; #define ATH_LOCK_INIT(_sc) \ @@ -414,6 +422,14 @@ struct ath_softc { #define ATH_UNLOCK(_sc) mtx_unlock(&(_sc)->sc_mtx) #define ATH_LOCK_ASSERT(_sc) mtx_assert(&(_sc)->sc_mtx, MA_OWNED) +#define ATH_TXNODE_LOCK_INIT(_sc) \ + mtx_init(&(_sc)->sc_txnodeq_mtx, (sc)->sc_txnodeq_name, \ + NULL, MTX_DEF | MTX_RECURSE) +#define ATH_TXNODE_LOCK_DESTROY(_sc) mtx_destroy(&(_sc)->sc_txnodeq_mtx) +#define ATH_TXNODE_LOCK(_sc) mtx_lock(&(_sc)->sc_txnodeq_mtx) +#define ATH_TXNODE_UNLOCK(_sc) mtx_unlock(&(_sc)->sc_txnodeq_mtx) +#define ATH_TXNODE_LOCK_ASSERT(_sc) mtx_assert(&(_sc)->sc_txnodeq_mtx, MA_OWNED) + #define ATH_TXQ_SETUP(sc, i) ((sc)->sc_txqsetup & (1<