Date: Sun, 18 Sep 2011 08:26:57 +0000 (UTC) From: Adrian Chadd <adrian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r225646 - user/adrian/if_ath_tx/sys/dev/ath Message-ID: <201109180826.p8I8QvXH020468@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: adrian Date: Sun Sep 18 08:26:57 2011 New Revision: 225646 URL: http://svn.freebsd.org/changeset/base/225646 Log: Part 2 of the locking/completion overhaul. Update the rest of the flush, error and cleanup functions to delay calling the completion functions until the TXQ lock has been released. The TXQ locking in some instances is quite inefficient and should be tidied up before this is merged into -HEAD. Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath.c user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath.c ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_ath.c Sun Sep 18 07:20:59 2011 (r225645) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath.c Sun Sep 18 08:26:57 2011 (r225646) @@ -4681,9 +4681,7 @@ ath_tx_draintxq(struct ath_softc *sc, st * Drain software queued frames which are on * active TIDs. */ - ATH_TXQ_LOCK(txq); ath_tx_txq_drain(sc, txq); - ATH_TXQ_UNLOCK(txq); } static void 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 Sep 18 07:20:59 2011 (r225645) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Sun Sep 18 08:26:57 2011 (r225646) @@ -2225,7 +2225,8 @@ ath_tx_tid_txq_unmark(struct ath_softc * * forward. */ static void -ath_tx_tid_drain(struct ath_softc *sc, struct ath_node *an, struct ath_tid *tid) +ath_tx_tid_drain(struct ath_softc *sc, struct ath_node *an, struct ath_tid *tid, + ath_bufhead *bf_cq) { struct ath_buf *bf; struct ieee80211_tx_ampdu *tap; @@ -2284,7 +2285,7 @@ ath_tx_tid_drain(struct ath_softc *sc, s __func__, SEQNO(bf->bf_state.bfs_seqno)); } ATH_TXQ_REMOVE(tid, bf, bf_list); - ath_tx_default_comp(sc, bf, 0); + TAILQ_INSERT_TAIL(bf_cq, bf, bf_list); } /* @@ -2323,6 +2324,10 @@ void ath_tx_node_flush(struct ath_softc *sc, struct ath_node *an) { int tid; + ath_bufhead bf_cq; + struct ath_buf *bf; + + TAILQ_INIT(&bf_cq); for (tid = 0; tid < IEEE80211_TID_SIZE; tid++) { struct ath_tid *atid = &an->an_tid[tid]; @@ -2333,9 +2338,15 @@ ath_tx_node_flush(struct ath_softc *sc, ath_tx_tid_unsched(sc, an, atid); /* Free packets */ - ath_tx_tid_drain(sc, an, atid); + ath_tx_tid_drain(sc, an, atid, &bf_cq); ATH_TXQ_UNLOCK(txq); } + + /* Handle completed frames */ + while ((bf = TAILQ_FIRST(&bf_cq)) != NULL) { + TAILQ_REMOVE(&bf_cq, bf, bf_list); + ath_tx_default_comp(sc, bf, 0); + } } /* @@ -2345,8 +2356,11 @@ void ath_tx_txq_drain(struct ath_softc *sc, struct ath_txq *txq) { struct ath_tid *tid; + ath_bufhead bf_cq; + struct ath_buf *bf; - ATH_TXQ_LOCK_ASSERT(txq); + TAILQ_INIT(&bf_cq); + ATH_TXQ_LOCK(txq); /* * Iterate over all active tids for the given txq, @@ -2354,10 +2368,16 @@ ath_tx_txq_drain(struct ath_softc *sc, s */ while (! TAILQ_EMPTY(&txq->axq_tidq)) { tid = TAILQ_FIRST(&txq->axq_tidq); - ath_tx_tid_drain(sc, tid->an, tid); + ath_tx_tid_drain(sc, tid->an, tid, &bf_cq); ath_tx_tid_unsched(sc, tid->an, tid); } + ATH_TXQ_UNLOCK(txq); + + while ((bf = TAILQ_FIRST(&bf_cq)) != NULL) { + TAILQ_REMOVE(&bf_cq, bf, bf_list); + ath_tx_default_comp(sc, bf, 0); + } } /* @@ -2433,13 +2453,10 @@ ath_tx_comp_cleanup_unaggr(struct ath_so int tid = bf->bf_state.bfs_tid; struct ath_tid *atid = &an->an_tid[tid]; - ATH_TXQ_LOCK_ASSERT(sc->sc_ac2q[atid->ac]); - DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: TID %d: incomp=%d\n", __func__, tid, atid->incomp); - ath_tx_default_comp(sc, bf, 0); - + ATH_TXQ_LOCK(sc->sc_ac2q[atid->ac]); atid->incomp--; if (atid->incomp == 0) { DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, @@ -2448,7 +2465,9 @@ ath_tx_comp_cleanup_unaggr(struct ath_so atid->cleanup_inprogress = 0; ath_tx_tid_resume(sc, atid); } + ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); + ath_tx_default_comp(sc, bf, 0); } /* @@ -2468,11 +2487,13 @@ ath_tx_cleanup(struct ath_softc *sc, str struct ath_tid *atid = &an->an_tid[tid]; struct ieee80211_tx_ampdu *tap; struct ath_buf *bf, *bf_next; + ath_bufhead bf_cq; DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: TID %d: called\n", __func__, tid); - ATH_TXQ_LOCK_ASSERT(sc->sc_ac2q[atid->ac]); + TAILQ_INIT(&bf_cq); + ATH_TXQ_LOCK(sc->sc_ac2q[atid->ac]); /* * Update the frames in the software TX queue: @@ -2499,7 +2520,7 @@ ath_tx_cleanup(struct ath_softc *sc, str * Call the default completion handler with "fail" just * so upper levels are suitably notified about this. */ - ath_tx_default_comp(sc, bf, 1); + TAILQ_INSERT_TAIL(&bf_cq, bf, bf_list); bf = bf_next; continue; } @@ -2544,6 +2565,13 @@ ath_tx_cleanup(struct ath_softc *sc, str DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: TID %d: cleanup needed: %d packets\n", __func__, tid, atid->incomp); + ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); + + /* Handle completing frames and fail them */ + while ((bf = TAILQ_FIRST(&bf_cq)) != NULL) { + TAILQ_REMOVE(&bf_cq, bf, bf_list); + ath_tx_default_comp(sc, bf, 1); + } } static void @@ -2621,6 +2649,8 @@ ath_tx_aggr_retry_unaggr(struct ath_soft struct ath_tid *atid = &an->an_tid[tid]; struct ieee80211_tx_ampdu *tap; + ATH_TXQ_LOCK(sc->sc_ac2q[atid->ac]); + tap = ath_tx_get_tx_tid(an, tid); /* @@ -2683,8 +2713,8 @@ ath_tx_aggr_retry_unaggr(struct ath_soft #endif /* Free buffer, bf is free after this call */ + ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); ath_tx_default_comp(sc, bf, 0); - return; } @@ -2701,6 +2731,8 @@ ath_tx_aggr_retry_unaggr(struct ath_soft */ ATH_TXQ_INSERT_HEAD(atid, bf, bf_list); ath_tx_tid_sched(sc, an, atid); + + ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); } /* @@ -2755,8 +2787,6 @@ ath_tx_retry_subframe(struct ath_softc * "%s: wasn't added: seqno %d\n", __func__, SEQNO(bf->bf_state.bfs_seqno)); bf->bf_state.bfs_dobaw = 0; - /* XXX subframe completion status? is that valid here? */ - ath_tx_default_comp(sc, bf, 0); return 1; } @@ -2780,12 +2810,14 @@ ath_tx_comp_aggr_error(struct ath_softc ath_bufhead bf_q; int drops = 0; struct ieee80211_tx_ampdu *tap; + ath_bufhead bf_cq; - ATH_TXQ_LOCK_ASSERT(sc->sc_ac2q[tid->ac]); + ATH_TXQ_LOCK(sc->sc_ac2q[tid->ac]); tap = ath_tx_get_tx_tid(an, tid->tid); TAILQ_INIT(&bf_q); + TAILQ_INIT(&bf_cq); sc->sc_stats.ast_tx_aggrfail++; /* @@ -2804,7 +2836,11 @@ ath_tx_comp_aggr_error(struct ath_softc while (bf) { bf_next = bf->bf_next; bf->bf_next = NULL; /* Remove it from the aggr list */ - drops += ath_tx_retry_subframe(sc, bf, &bf_q); + if (ath_tx_retry_subframe(sc, bf, &bf_q)) { + drops++; + bf->bf_next = NULL; + TAILQ_INSERT_TAIL(&bf_cq, bf, bf_list); + } bf = bf_next; } @@ -2836,6 +2872,13 @@ ath_tx_comp_aggr_error(struct ath_softc } ath_tx_tid_sched(sc, an, tid); + ATH_TXQ_UNLOCK(sc->sc_ac2q[tid->ac]); + + /* Complete frames which errored out */ + while ((bf = TAILQ_FIRST(&bf_cq)) != NULL) { + TAILQ_REMOVE(&bf_cq, bf, bf_list); + ath_tx_default_comp(sc, bf, 0); + } } /* @@ -2853,12 +2896,12 @@ ath_tx_comp_cleanup_aggr(struct ath_soft bf = bf_first; + ATH_TXQ_LOCK(sc->sc_ac2q[atid->ac]); + + /* update incomp */ while (bf) { atid->incomp--; - bf_next = bf->bf_next; - bf->bf_next = NULL; /* Remove it from the aggr list */ - ath_tx_default_comp(sc, bf, 1); - bf = bf_next; + bf = bf->bf_next; } if (atid->incomp == 0) { @@ -2868,7 +2911,14 @@ ath_tx_comp_cleanup_aggr(struct ath_soft atid->cleanup_inprogress = 0; ath_tx_tid_resume(sc, atid); } + ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); + /* Handle frame completion */ + while (bf) { + bf_next = bf->bf_next; + ath_tx_default_comp(sc, bf, 1); + bf = bf_next; + } } /* @@ -2917,8 +2967,8 @@ ath_tx_aggr_comp_aggr(struct ath_softc * * Punt cleanup to the relevant function, not our problem now */ if (0 && atid->cleanup_inprogress) { - ath_tx_comp_cleanup_aggr(sc, bf_first); ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); + ath_tx_comp_cleanup_aggr(sc, bf_first); return; } @@ -2937,8 +2987,8 @@ ath_tx_aggr_comp_aggr(struct ath_softc * * handle errors first */ if (ts.ts_status & HAL_TXERR_XRETRY) { - ath_tx_comp_aggr_error(sc, bf_first, atid); ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); + ath_tx_comp_aggr_error(sc, bf_first, atid); return; } @@ -3023,9 +3073,14 @@ ath_tx_aggr_comp_aggr(struct ath_softc * device_printf(sc->sc_dev, "%s: wasn't added: seqno %d\n", __func__, SEQNO(bf->bf_state.bfs_seqno)); + bf->bf_next = NULL; TAILQ_INSERT_TAIL(&bf_cq, bf, bf_list); } else { - drops += ath_tx_retry_subframe(sc, bf, &bf_q); + if (ath_tx_retry_subframe(sc, bf, &bf_q)) { + drops++; + bf->bf_next = NULL; + TAILQ_INSERT_TAIL(&bf_cq, bf, bf_list); + } nbad++; } bf = bf_next; @@ -3103,6 +3158,23 @@ ath_tx_aggr_comp_unaggr(struct ath_softc struct ath_tid *atid = &an->an_tid[tid]; struct ath_tx_status *ts = &bf->bf_status.ds_txstat; + /* + * Update rate control status here, before we possibly + * punt to retry or cleanup. + * + * Do it outside of the TXQ lock. + */ + if (fail == 0 && ((bf->bf_txflags & HAL_TXDESC_NOACK) == 0)) + ath_tx_update_ratectrl(sc, ni, bf->bf_state.bfs_rc, + &bf->bf_status.ds_txstat, + bf->bf_state.bfs_pktlen, + 1, (ts->ts_status == 0) ? 0 : 1); + + /* + * This is called early so atid->hwq_depth can be tracked. + * This unfortunately means that it's released and regrabbed + * during retry and cleanup. That's rather inefficient. + */ ATH_TXQ_LOCK(sc->sc_ac2q[atid->ac]); if (tid == IEEE80211_NONQOS_TID) @@ -3117,24 +3189,14 @@ ath_tx_aggr_comp_unaggr(struct ath_softc __func__, atid->hwq_depth); /* - * Update rate control status here, before we possibly - * punt to retry or cleanup. - */ - if (fail == 0 && ((bf->bf_txflags & HAL_TXDESC_NOACK) == 0)) - ath_tx_update_ratectrl(sc, ni, bf->bf_state.bfs_rc, - &bf->bf_status.ds_txstat, - bf->bf_state.bfs_pktlen, - 1, (ts->ts_status == 0) ? 0 : 1); - - /* * If a cleanup is in progress, punt to comp_cleanup; * rather than handling it here. It's thus their * responsibility to clean up, call the completion * function in net80211, etc. */ if (atid->cleanup_inprogress) { - ath_tx_comp_cleanup_unaggr(sc, bf); ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); + ath_tx_comp_cleanup_unaggr(sc, bf); return; } @@ -3143,8 +3205,8 @@ ath_tx_aggr_comp_unaggr(struct ath_softc * are being failed (eg during queue deletion.) */ if (fail == 0 && ts->ts_status & HAL_TXERR_XRETRY) { - ath_tx_aggr_retry_unaggr(sc, bf); ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); + ath_tx_aggr_retry_unaggr(sc, bf); return; } @@ -3668,14 +3730,12 @@ ath_addba_stop(struct ieee80211_node *ni /* There's no need to hold the TXQ lock here */ sc->sc_addba_stop(ni, tap); - ATH_TXQ_LOCK(sc->sc_ac2q[atid->ac]); /* * ath_tx_cleanup will resume the TID if possible, otherwise * it'll set the cleanup flag, and it'll be unpaused once * things have been cleaned up. */ ath_tx_cleanup(sc, an, tid); - ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); } /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201109180826.p8I8QvXH020468>