From c9e9ad973cfe0909ff547598f68c915ca6b53e44 Mon Sep 17 00:00:00 2001 From: "Claude (noether)" Date: Mon, 18 May 2026 16:59:28 +0200 Subject: [PATCH] patches/driver/bes2600/queue-pending-record-lock-bh-danctnix: mirror besser#18 fix from bes2600-dkms Single-patch series-dir, mirror of the Markus-authored commit d95453c on marfrit/bes2600-dkms branch bes2600/queue-pending-record-lock-bh-fix (PR #11). Paths rewritten from DKMS-style (bes2600/foo.c) to in-tree staging (drivers/staging/bes2600/foo.c) via sed -- this is the in-tree variant. Fix: convert plain spin_lock(&pending_record_lock) to spin_lock_bh() at the 5 sites where it's taken in non-BH-disabled contexts (queue.c:832/839/844, tx_loop.c:112/114). 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. Eliminates the SOFTIRQ-safe -> SOFTIRQ-unsafe lockdep warning reported in besser#18 (PROVE_LOCKING-only -- non-fatal on production builds where lockdep is off, but real AB-BA window between bes2600_join_work workqueue context and bes2600_tx softirq context). This commit does NOT add the include to fleet/ohm.yaml. The patch will be wired into ohm's manifest in a follow-up commit (or this branch's PR can extend with the ohm.yaml change once the migration PR #28 lands and the bes2600-dkms PR #11 is reviewed). Closes: besser#18 Refs: marfrit/bes2600-dkms #11 (source-of-truth PR) --- ...600-take-pending-record-lock-with-bh.patch | 121 ++++++++++++++++++ .../README.md | 19 +++ 2 files changed, 140 insertions(+) create mode 100644 patches/driver/bes2600/queue-pending-record-lock-bh-danctnix/0001-bes2600-take-pending-record-lock-with-bh.patch create mode 100644 patches/driver/bes2600/queue-pending-record-lock-bh-danctnix/README.md diff --git a/patches/driver/bes2600/queue-pending-record-lock-bh-danctnix/0001-bes2600-take-pending-record-lock-with-bh.patch b/patches/driver/bes2600/queue-pending-record-lock-bh-danctnix/0001-bes2600-take-pending-record-lock-with-bh.patch new file mode 100644 index 0000000..ff82b10 --- /dev/null +++ b/patches/driver/bes2600/queue-pending-record-lock-bh-danctnix/0001-bes2600-take-pending-record-lock-with-bh.patch @@ -0,0 +1,121 @@ +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 + diff --git a/patches/driver/bes2600/queue-pending-record-lock-bh-danctnix/README.md b/patches/driver/bes2600/queue-pending-record-lock-bh-danctnix/README.md new file mode 100644 index 0000000..28f809f --- /dev/null +++ b/patches/driver/bes2600/queue-pending-record-lock-bh-danctnix/README.md @@ -0,0 +1,19 @@ +# queue-pending-record-lock-bh-danctnix — close besser#18 + +Converts `pending_record_lock` to `spin_lock_bh()` at the 5 sites +where it is taken in non-BH-disabled contexts (`bes2600_queue_get_skb` +called from `bes2600_join_work`, and `bes2600_tx_loop_item_pending_check`). + +Eliminates the PROVE_LOCKING SOFTIRQ-safe → SOFTIRQ-unsafe warning +reported in besser#18: `&queue->lock` (taken with `_bh` everywhere, +including the nested acquisition at `queue.c:289` that holds +`pending_record_lock` as inner) was registered SOFTIRQ-irq-safe by +lockdep, but `pending_record_lock` was sometimes taken without BH +disable, creating an AB-BA deadlock window. + +Provenance: +- Source-of-truth commit on `marfrit/bes2600-dkms` branch + `bes2600/queue-pending-record-lock-bh-fix`, commit `d95453c`. +- This file is the same commit's `git format-patch` output with + the DKMS-style `bes2600/foo.c` paths rewritten to in-tree + `drivers/staging/bes2600/foo.c` paths via sed.