Luca Coelho <luca@xxxxxxxxx> writes: > On Mon, 2021-11-29 at 14:54 +0100, Toke Høiland-Jørgensen wrote: >> Luca Coelho <luca@xxxxxxxxx> writes: >> >> > From: Johannes Berg <johannes.berg@xxxxxxxxx> >> > >> > When we call ieee80211_agg_start_txq(), that will in turn call >> > schedule_and_wake_txq(). Called from ieee80211_stop_tx_ba_cb() >> > this is done under sta->lock, which leads to certain circular >> > lock dependencies, as reported by Chris Murphy: >> > https://lore.kernel.org/r/CAJCQCtSXJ5qA4bqSPY=oLRMbv-irihVvP7A2uGutEbXQVkoNaw@xxxxxxxxxxxxxx >> > >> > In general, ieee80211_agg_start_txq() is usually not called >> > with sta->lock held, only in this one place. But it's always >> > called with sta->ampdu_mlme.mtx held, and that's therefore >> > clearly sufficient. >> > >> > Change ieee80211_stop_tx_ba_cb() to also call it without the >> > sta->lock held, by factoring it out of ieee80211_remove_tid_tx() >> > (which is only called in this one place). >> > >> > This breaks the locking chain and makes it less likely that >> > we'll have similar locking chain problems in the future. >> > >> > Reported-by: Chris Murphy <lists@xxxxxxxxxxxxxxxxx> >> > Signed-off-by: Johannes Berg <johannes.berg@xxxxxxxxx> >> > Signed-off-by: Luca Coelho <luciano.coelho@xxxxxxxxx> >> >> Does this need a fixes: tag? > > Hi Toke, > > Neither Johannes nor Chris pointed to any specific patch that this > commit is fixing. If you know the exact commit that broke this, I can > add the tag and send v2. Well, it looks like the code you're changing comes all the way back from: ba8c3d6f16a1 ("mac80211: add an intermediate software queue implementation") Maybe Johannes can comment on whether it's appropriate to include this, or if the code changed too much in the meantime... -Toke
