Date: Mon, 19 Feb 2024 16:10:27 GMT From: "Bjoern A. Zeeb" <bz@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 401dbf9cfc0c - releng/13.3 - LinuxKPI: 802.11: lsta txq locking cleanup Message-ID: <202402191610.41JGAROx036167@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch releng/13.3 has been updated by bz: URL: https://cgit.FreeBSD.org/src/commit/?id=401dbf9cfc0cfb2a02355d41bf73e146bd3dfb74 commit 401dbf9cfc0cfb2a02355d41bf73e146bd3dfb74 Author: Bjoern A. Zeeb <bz@FreeBSD.org> AuthorDate: 2024-02-14 21:56:48 +0000 Commit: Bjoern A. Zeeb <bz@FreeBSD.org> CommitDate: 2024-02-19 16:09:22 +0000 LinuxKPI: 802.11: lsta txq locking cleanup Rename the LSTA lock to LSTA_TXQ lock as that is really what it is and put down the full set of macros. Replace the init and destroy with the macro invocation rather than direct code. Put locking around the txq_ready unset and check. Move the taskq_enqueue call under lock to be sure we do not call it anymore after txq_ready got unset. Leave a comment related to the node reference which is passed into the TX path on the recvif mbuf pointer. Approved by: re (cperciva) Fixes: 0936c648ad0ee PR: 274382 (possibly) Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D43909 (cherry picked from commit fa4e4257943650c0b5f58c01bb0bdfadea61dfb2) (cherry picked from commit 763b10806cd4ebdcbd2b6753d4f50ec088dc57fb) --- sys/compat/linuxkpi/common/src/linux_80211.c | 31 ++++++++++++++++++---------- sys/compat/linuxkpi/common/src/linux_80211.h | 14 +++++++++++-- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/sys/compat/linuxkpi/common/src/linux_80211.c b/sys/compat/linuxkpi/common/src/linux_80211.c index d6431e7fc1d5..adddb0fc3500 100644 --- a/sys/compat/linuxkpi/common/src/linux_80211.c +++ b/sys/compat/linuxkpi/common/src/linux_80211.c @@ -272,7 +272,7 @@ lkpi_lsta_alloc(struct ieee80211vap *vap, const uint8_t mac[IEEE80211_ADDR_LEN], } /* Deferred TX path. */ - mtx_init(&lsta->txq_mtx, "lsta_txq", NULL, MTX_DEF); + LKPI_80211_LSTA_TXQ_LOCK_INIT(lsta); TASK_INIT(&lsta->txq_task, 0, lkpi_80211_txq_task, lsta); mbufq_init(&lsta->txq, IFQ_MAXLEN); lsta->txq_ready = true; @@ -304,8 +304,11 @@ lkpi_lsta_free(struct lkpi_sta *lsta, struct ieee80211_node *ni) /* XXX-BZ free resources, ... */ IMPROVE(); - /* XXX locking */ + /* Drain sta->txq[] */ + + LKPI_80211_LSTA_TXQ_LOCK(lsta); lsta->txq_ready = false; + LKPI_80211_LSTA_TXQ_UNLOCK(lsta); /* Drain taskq, won't be restarted until added_to_drv is set again. */ while (taskqueue_cancel(taskqueue_thread, &lsta->txq_task, NULL) != 0) @@ -324,9 +327,7 @@ lkpi_lsta_free(struct lkpi_sta *lsta, struct ieee80211_node *ni) } KASSERT(mbufq_empty(&lsta->txq), ("%s: lsta %p has txq len %d != 0\n", __func__, lsta, mbufq_len(&lsta->txq))); - - /* Drain sta->txq[] */ - mtx_destroy(&lsta->txq_mtx); + LKPI_80211_LSTA_TXQ_LOCK_DESTROY(lsta); /* Remove lsta from vif; that is done by the state machine. Should assert it? */ @@ -3398,16 +3399,21 @@ lkpi_ic_raw_xmit(struct ieee80211_node *ni, struct mbuf *m, struct lkpi_sta *lsta; lsta = ni->ni_drv_data; - /* XXX locking */ + LKPI_80211_LSTA_TXQ_LOCK(lsta); if (!lsta->txq_ready) { + LKPI_80211_LSTA_TXQ_UNLOCK(lsta); + /* + * Free the mbuf (do NOT release ni ref for the m_pkthdr.rcvif! + * ieee80211_raw_output() does that in case of error). + */ m_free(m); return (ENETDOWN); } /* Queue the packet and enqueue the task to handle it. */ - LKPI_80211_LSTA_LOCK(lsta); mbufq_enqueue(&lsta->txq, m); - LKPI_80211_LSTA_UNLOCK(lsta); + taskqueue_enqueue(taskqueue_thread, &lsta->txq_task); + LKPI_80211_LSTA_TXQ_UNLOCK(lsta); #ifdef LINUXKPI_DEBUG_80211 if (linuxkpi_debug_80211 & D80211_TRACE_TX) @@ -3416,7 +3422,6 @@ lkpi_ic_raw_xmit(struct ieee80211_node *ni, struct mbuf *m, mbufq_len(&lsta->txq)); #endif - taskqueue_enqueue(taskqueue_thread, &lsta->txq_task); return (0); } @@ -3624,9 +3629,13 @@ lkpi_80211_txq_task(void *ctx, int pending) mbufq_init(&mq, IFQ_MAXLEN); - LKPI_80211_LSTA_LOCK(lsta); + LKPI_80211_LSTA_TXQ_LOCK(lsta); + /* + * Do not re-check lsta->txq_ready here; we may have a pending + * disassoc frame still. + */ mbufq_concat(&mq, &lsta->txq); - LKPI_80211_LSTA_UNLOCK(lsta); + LKPI_80211_LSTA_TXQ_UNLOCK(lsta); m = mbufq_dequeue(&mq); while (m != NULL) { diff --git a/sys/compat/linuxkpi/common/src/linux_80211.h b/sys/compat/linuxkpi/common/src/linux_80211.h index d9cb1a538f91..7a1d5877b874 100644 --- a/sys/compat/linuxkpi/common/src/linux_80211.h +++ b/sys/compat/linuxkpi/common/src/linux_80211.h @@ -278,8 +278,18 @@ struct lkpi_wiphy { #define LKPI_80211_LVIF_LOCK(_lvif) mtx_lock(&(_lvif)->mtx) #define LKPI_80211_LVIF_UNLOCK(_lvif) mtx_unlock(&(_lvif)->mtx) -#define LKPI_80211_LSTA_LOCK(_lsta) mtx_lock(&(_lsta)->txq_mtx) -#define LKPI_80211_LSTA_UNLOCK(_lsta) mtx_unlock(&(_lsta)->txq_mtx) +#define LKPI_80211_LSTA_TXQ_LOCK_INIT(_lsta) \ + mtx_init(&(_lsta)->txq_mtx, "lsta-txq", NULL, MTX_DEF); +#define LKPI_80211_LSTA_TXQ_LOCK_DESTROY(_lsta) \ + mtx_destroy(&(_lsta)->txq_mtx); +#define LKPI_80211_LSTA_TXQ_LOCK(_lsta) \ + mtx_lock(&(_lsta)->txq_mtx) +#define LKPI_80211_LSTA_TXQ_UNLOCK(_lsta) \ + mtx_unlock(&(_lsta)->txq_mtx) +#define LKPI_80211_LSTA_TXQ_LOCK_ASSERT(_lsta) \ + mtx_assert(&(_lsta)->txq_mtx, MA_OWNED) +#define LKPI_80211_LSTA_TXQ_UNLOCK_ASSERT(_lsta) \ + mtx_assert(&(_lsta)->txq_mtx, MA_NOTOWNED) #define LKPI_80211_LTXQ_LOCK_INIT(_ltxq) \ mtx_init(&(_ltxq)->ltxq_mtx, "ltxq", NULL, MTX_DEF);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202402191610.41JGAROx036167>