Johannes Berg <[email protected]> writes: > On Tue, 2019-10-01 at 12:53 +0200, Toke Høiland-Jørgensen wrote: >> >> > - 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. > > Right. > >> But there are lots of uses of >> spin_{,un}lock_bh() in tx.c: > > [snip] > >> so won't that mean that the driver still gets bhs re-enabled after (for >> instance) the first call to ieee80211_tx_dequeue()? > > No, local_bh_disable()/local_bh_enable() is re-entrant and nests fine.
Ah, right, gotcha. Hmm, in that case, would it be more clear to just add an outer local_bh_disable()/local_bh_enable() to ___ieee80211_wake_txqs(). With this patch we're relying on the mismatched use of _bh/non-_bh variants of the locking to ensure the bhs stay off. Isn't that prone to breaking in the future? Oh, and also, with just this patch, the additional drv_wake_tx_queue() call for the vif TXQ at the bottom of __ieee80211_wake_txqs() will still happen without bhs disabled, won't it? -Toke
