From d95453c98e31d7a47bc227aef5d0b426ac9e334b Mon Sep 17 00:00:00 2001 From: Markus Fritsche Date: Mon, 18 May 2026 16:58:49 +0200 Subject: [PATCH] =?UTF-8?q?bes2600:=20take=20pending=5Frecord=5Flock=20wit?= =?UTF-8?q?h=20=5Fbh()=20to=20fix=20SOFTIRQ-safe=20=E2=86=92=20-unsafe=20i?= =?UTF-8?q?nversion=20(besser#18)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PROVE_LOCKING reports: WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected kworker/u16:1 is trying to acquire: &hw_priv->tx_loop.pending_record_lock at bes2600_queue_clear+0x80 and this task is already holding: &queue->lock at bes2600_queue_clear+0x60 which would create a new lock dependency: (&queue->lock){+.-.} -> (&hw_priv->tx_loop.pending_record_lock){+.+.} but this new dependency connects a SOFTIRQ-irq-safe lock: (&queue->lock){+.-.} ... which became SOFTIRQ-irq-safe at: bes2600_tx -> ieee80211_handle_wake_tx_queue -> tasklet_action to a SOFTIRQ-irq-unsafe lock: (&hw_priv->tx_loop.pending_record_lock){+.+.} ... which became SOFTIRQ-irq-unsafe at: bes2600_queue_get_skb -> bes2600_join_work -> process_one_work queue->lock is taken consistently with spin_lock_bh() at 22 sites; the nested acquisition of pending_record_lock at queue.c:289 (inside the outer queue->lock_bh held at line 285) had it implicitly BH-safe via the outer scope. But pending_record_lock is ALSO taken from non-BH-disabled contexts: bes2600_queue_get_skb (queue.c:832) — process context via bes2600_join_work (workqueue), no outer queue->lock held bes2600_tx_loop_item_pending_check (tx_loop.c:112) — TX-loop context, no outer queue->lock held When CPU0 holds pending_record_lock from one of those non-BH paths and a softirq fires that wants queue->lock, and CPU1 in softirq has queue->lock and is about to acquire pending_record_lock — classic AB-BA SOFTIRQ deadlock. The fix is the conservative one: take pending_record_lock with _bh() at every site that's not already inside a queue->lock_bh-held scope. That makes the lock consistently SOFTIRQ-safe, eliminating the inversion. queue.c:289/295 stays as plain spin_lock because BH is already disabled by the outer queue->lock_bh acquired at queue.c:285. Five sites converted: bes2600/queue.c:832 -- spin_lock -> spin_lock_bh bes2600/queue.c:839 -- spin_unlock -> spin_unlock_bh bes2600/queue.c:844 -- spin_unlock -> spin_unlock_bh bes2600/tx_loop.c:112 -- spin_lock -> spin_lock_bh bes2600/tx_loop.c:114 -- spin_unlock -> spin_unlock_bh Contract: - Documentation/locking/locktypes.rst spelling: spin_lock_bh() is the canonical way to make a non-IRQ spinlock safe against softirq preemption that might re-enter the same lock. - Same shape as queue->lock in this driver and as is_drv->lock in the cw1200 ancestor. Closes: besser#18 Fixes: Signed-off-by: Markus Fritsche --- bes2600/queue.c | 6 +++--- bes2600/tx_loop.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/bes2600/queue.c b/drivers/staging/bes2600/queue.c index cc606c1..4016b76 100644 --- a/drivers/staging/bes2600/queue.c +++ b/drivers/staging/bes2600/queue.c @@ -829,19 +829,19 @@ int bes2600_queue_get_skb(struct bes2600_queue *queue, u32 packetID, bes2600_queue_parse_id(packetID, &queue_generation, &queue_id, &item_generation, &item_id, &if_id, &link_id); - spin_lock(&queue->stats->hw_priv->tx_loop.pending_record_lock); + spin_lock_bh(&queue->stats->hw_priv->tx_loop.pending_record_lock); if (!list_empty(&queue->stats->hw_priv->tx_loop.pending_record_list)) { list_for_each_entry_safe(record_item, temp_record_item, &queue->stats->hw_priv->tx_loop.pending_record_list, head) { if (record_item->packetID == packetID) { list_del(&record_item->head); dev_kfree_skb(record_item->skb); kfree(record_item); - spin_unlock(&queue->stats->hw_priv->tx_loop.pending_record_lock); + spin_unlock_bh(&queue->stats->hw_priv->tx_loop.pending_record_lock); return -EINVAL; } } } - spin_unlock(&queue->stats->hw_priv->tx_loop.pending_record_lock); + spin_unlock_bh(&queue->stats->hw_priv->tx_loop.pending_record_lock); item = &queue->pool[item_id]; diff --git a/drivers/staging/bes2600/tx_loop.c b/drivers/staging/bes2600/tx_loop.c index e6cf072..0cf7ce1 100644 --- a/drivers/staging/bes2600/tx_loop.c +++ b/drivers/staging/bes2600/tx_loop.c @@ -109,9 +109,9 @@ void bes2600_tx_loop_set_enable(struct bes2600_common *hw_priv, bool need_warn) bes2600_queue_iterate_pending_packet(&hw_priv->tx_queue[i], bes2600_tx_loop_item_pending_item); } - spin_lock(&hw_priv->tx_loop.pending_record_lock); + spin_lock_bh(&hw_priv->tx_loop.pending_record_lock); bes2600_queue_iterate_record_pending_packet(hw_priv, bes2600_tx_loop_item_pending_item); - spin_unlock(&hw_priv->tx_loop.pending_record_lock); + spin_unlock_bh(&hw_priv->tx_loop.pending_record_lock); if (atomic_read(&hw_priv->bh_rx) > 0) wake_up(&hw_priv->bh_wq); -- 2.54.0