Johannes Berg <[email protected]> writes:
> From: Johannes Berg <[email protected]>
>
> Drivers typically expect this, as it's the case for almost all cases
> where this is called (i.e. from the TX path). Also, the code in mac80211
> itself (if the driver calls ieee80211_tx_dequeue()) expects this as it
> uses this_cpu_ptr() without additional protection.
>
> This should fix various reports of the problem:
> https://bugzilla.kernel.org/show_bug.cgi?id=204127
> https://lore.kernel.org/linux-wireless/can5hydrwb3o_fe6a1xdnp1e+xs66d5kieuhhfigkklnqokx...@mail.gmail.com/
> https://lore.kernel.org/lkml/[email protected]/
>
> Reported-by: Jiri Kosina <[email protected]>
> Reported-by: Aaron Hill <[email protected]>
> Reported-by: Lukas Redlinger <[email protected]>
> Reported-by: Oleksii Shevchuk <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> net/mac80211/util.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index 051a02ddcb85..ad1e88958da2 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -273,9 +273,9 @@ static void __ieee80211_wake_txqs(struct
> ieee80211_sub_if_data *sdata, int ac)
> &txqi->flags))
> continue;
>
> - spin_unlock_bh(&fq->lock);
> + spin_unlock(&fq->lock);
> drv_wake_tx_queue(local, txqi);
> - spin_lock_bh(&fq->lock);
> + spin_lock(&fq->lock);
Okay, so this will mean that the drv_wake_tx_queue() entry point will be
called with bhs disabled. But there are lots of uses of
spin_{,un}lock_bh() in tx.c:
$ git grep lock_bh tx.c
tx.c: spin_lock_bh(&fq->lock);
tx.c: spin_unlock_bh(&fq->lock);
tx.c: spin_lock_bh(&fq->lock);
tx.c: spin_unlock_bh(&fq->lock);
tx.c: spin_lock_bh(&fq->lock);
tx.c: spin_unlock_bh(&fq->lock);
tx.c: spin_lock_bh(&local->active_txq_lock[txqi->txq.ac]);
tx.c: spin_unlock_bh(&local->active_txq_lock[txqi->txq.ac]);
tx.c: spin_lock_bh(&fq->lock);
tx.c: spin_unlock_bh(&fq->lock);
tx.c: spin_lock_bh(&fq->lock);
tx.c: spin_unlock_bh(&fq->lock);
tx.c: spin_lock_bh(&sta->lock);
tx.c: spin_unlock_bh(&sta->lock);
tx.c: spin_lock_bh(&sta->lock);
tx.c: spin_unlock_bh(&sta->lock);
tx.c: spin_lock_bh(&fq->lock);
tx.c: spin_unlock_bh(&fq->lock);
tx.c: spin_lock_bh(&fq->lock);
tx.c: spin_unlock_bh(&fq->lock);
tx.c: spin_lock_bh(&fq->lock);
tx.c: spin_unlock_bh(&fq->lock);
tx.c: spin_unlock_bh(&fq->lock);
tx.c: spin_lock_bh(&local->active_txq_lock[ac]);
tx.c: spin_unlock_bh(&local->active_txq_lock[ac]);
tx.c: spin_lock_bh(&local->active_txq_lock[txq->ac]);
tx.c: spin_unlock_bh(&local->active_txq_lock[txq->ac]);
tx.c: spin_lock_bh(&local->active_txq_lock[ac]);
tx.c: spin_unlock_bh(&local->active_txq_lock[ac]);
tx.c: spin_unlock_bh(&local->active_txq_lock[ac]);
tx.c: spin_lock_bh(&local->active_txq_lock[ac]);
tx.c: spin_unlock_bh(&local->active_txq_lock[ac]);
tx.c: spin_lock_bh(&local->tim_lock);
tx.c: spin_unlock_bh(&local->tim_lock);
so won't that mean that the driver still gets bhs re-enabled after (for
instance) the first call to ieee80211_tx_dequeue()?
I must admit that I'm a bit fuzzy on this whole bh/not-bh thing; I've
mostly been cargo-culting the _bh variant of the locking... :)
-Toke
