Re: [PATCH v5 4/4] wifi: ath12k: implement custom wake_tx_queue with flow control
Jeff Johnson <[email protected]>
| Newsgroups | org.kernel.vger.linux-wireless,org.infradead.lists.ath11k,org.infradead.lists.ath12k,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 8/11/2026 4:14 AM, Jose Ignacio Tornos Martinez wrote: > Keeping rcu_read_lock() for the entire function is intentional. > ath12k_wifi7_mac_op_tx() calls rcu_dereference() internally (on ahvif->link[] > and ahsta->link[]), and with CONFIG_DEBUG_LOCK_ALLOC, rcu_dereference() checks > lock_is_held(&rcu_lock_map) via rcu_read_lock_held(). > Only an explicit rcu_read_lock() sets rcu_lock_map and spin_lock_bh() does not, > so releasing early would trigger lockdep warnings. hmm, so some of the following commits are broken? https://lore.kernel.org/all/[email protected]/ In particular the one that I merged into ath9k: https://lore.kernel.org/all/[email protected]/ diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index 0ac9212e42f7..4a0f465aa2fe 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -1993,7 +1993,6 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq) ieee80211_txq_schedule_start(hw, txq->mac80211_qnum); spin_lock_bh(&sc->chan_lock); - rcu_read_lock(); if (sc->cur_chan->stopped) goto out; @@ -2011,7 +2010,6 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq) } out: - rcu_read_unlock(); spin_unlock_bh(&sc->chan_lock); ieee80211_txq_schedule_end(hw, txq->mac80211_qnum); } I fed that into my review agent and it confirms your observation: The change works correctly on production kernels without CONFIG_DEBUG_LOCK_ALLOC. It will produce a false lockdep splat on debug kernels with CONFIG_PROVE_RCU because rcu_dereference() checks rcu_read_lock_held() which requires an explicit rcu_read_lock(), not just a spinlock. I have a fixup for ath9k which does: 171 - ratetbl = rcu_dereference(sta->rates); 171 + ratetbl = rcu_dereference_bh(sta->rates); Should ath12k also use rcu_dereference_bh()? /jeff