Commit 06c41bda authored by Johannes Berg's avatar Johannes Berg

mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock

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@mail.gmail.com

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.

Fixes: ba8c3d6f ("mac80211: add an intermediate software queue implementation")
Reported-by: default avatarChris Murphy <lists@colorremedies.com>
Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
Signed-off-by: default avatarLuca Coelho <luciano.coelho@intel.com>
Link: https://lore.kernel.org/r/iwlwifi.20211202152554.f519884c8784.I555fef8e67d93fff3d9a304886c4a9f8b322e591@changeidSigned-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
parent 37d33114
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
* Copyright 2007, Michael Wu <flamingice@sourmilk.net> * Copyright 2007, Michael Wu <flamingice@sourmilk.net>
* Copyright 2007-2010, Intel Corporation * Copyright 2007-2010, Intel Corporation
* Copyright(c) 2015-2017 Intel Deutschland GmbH * Copyright(c) 2015-2017 Intel Deutschland GmbH
* Copyright (C) 2018 - 2020 Intel Corporation * Copyright (C) 2018 - 2021 Intel Corporation
*/ */
#include <linux/ieee80211.h> #include <linux/ieee80211.h>
...@@ -213,6 +213,8 @@ ieee80211_agg_start_txq(struct sta_info *sta, int tid, bool enable) ...@@ -213,6 +213,8 @@ ieee80211_agg_start_txq(struct sta_info *sta, int tid, bool enable)
struct ieee80211_txq *txq = sta->sta.txq[tid]; struct ieee80211_txq *txq = sta->sta.txq[tid];
struct txq_info *txqi; struct txq_info *txqi;
lockdep_assert_held(&sta->ampdu_mlme.mtx);
if (!txq) if (!txq)
return; return;
...@@ -290,7 +292,6 @@ static void ieee80211_remove_tid_tx(struct sta_info *sta, int tid) ...@@ -290,7 +292,6 @@ static void ieee80211_remove_tid_tx(struct sta_info *sta, int tid)
ieee80211_assign_tid_tx(sta, tid, NULL); ieee80211_assign_tid_tx(sta, tid, NULL);
ieee80211_agg_splice_finish(sta->sdata, tid); ieee80211_agg_splice_finish(sta->sdata, tid);
ieee80211_agg_start_txq(sta, tid, false);
kfree_rcu(tid_tx, rcu_head); kfree_rcu(tid_tx, rcu_head);
} }
...@@ -889,6 +890,7 @@ void ieee80211_stop_tx_ba_cb(struct sta_info *sta, int tid, ...@@ -889,6 +890,7 @@ void ieee80211_stop_tx_ba_cb(struct sta_info *sta, int tid,
{ {
struct ieee80211_sub_if_data *sdata = sta->sdata; struct ieee80211_sub_if_data *sdata = sta->sdata;
bool send_delba = false; bool send_delba = false;
bool start_txq = false;
ht_dbg(sdata, "Stopping Tx BA session for %pM tid %d\n", ht_dbg(sdata, "Stopping Tx BA session for %pM tid %d\n",
sta->sta.addr, tid); sta->sta.addr, tid);
...@@ -906,10 +908,14 @@ void ieee80211_stop_tx_ba_cb(struct sta_info *sta, int tid, ...@@ -906,10 +908,14 @@ void ieee80211_stop_tx_ba_cb(struct sta_info *sta, int tid,
send_delba = true; send_delba = true;
ieee80211_remove_tid_tx(sta, tid); ieee80211_remove_tid_tx(sta, tid);
start_txq = true;
unlock_sta: unlock_sta:
spin_unlock_bh(&sta->lock); spin_unlock_bh(&sta->lock);
if (start_txq)
ieee80211_agg_start_txq(sta, tid, false);
if (send_delba) if (send_delba)
ieee80211_send_delba(sdata, sta->sta.addr, tid, ieee80211_send_delba(sdata, sta->sta.addr, tid,
WLAN_BACK_INITIATOR, WLAN_REASON_QSTA_NOT_USE); WLAN_BACK_INITIATOR, WLAN_REASON_QSTA_NOT_USE);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment