From 4d98a8169d3319547873e45a0b712ebee992d2a1 Mon Sep 17 00:00:00 2001 From: "Claude (noether)" Date: Mon, 18 May 2026 18:01:41 +0200 Subject: [PATCH] fleet/ohm + patches/driver/bes2600/queue-pending-record-lock-bh-danctnix: bundle besser#18 fix into the migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pulls the besser#18 lockdep fix (originally on noether/bes2600-pending-record-lock-bh / PR #30) into this PR so the ohm migration ships a single self-consistent pkgrel that contains all three goal components: kernel-agent flow + Patch I + besser#18 fix (plus the GCC 15 SCS Makefile workaround, no-op while SCS=n). ohm.yaml includes now resolve to 4 patches: 1. driver/bes2600/cumulative-c5x-danctnix/ (148 149 B) 2. driver/bes2600/scan-filter-5ghz-danctnix/ ( 7 735 B) 3. arch/arm64/xor-neon-ffixed-x18-scs-build-fix-danctnix/ (1 562 B) 4. driver/bes2600/queue-pending-record-lock-bh-danctnix/ (5 258 B) ---- cumulative.patch (162 704 B) b2sum 0eb091ddaba4a8f1c3c2a78eb8c621cdc6e6dfed6c43f7dac03e508a05b... Trailer-strip applied to the besser#18 patch source for the same reason as the SCS patch — it's now the last in the concatenated cumulative, and patch(1) errors on the orphan '-- \n2.54.0\n' EOF sentinel. Same gotcha documented in 84734ba. PR #30 (the standalone besser#18 mirror PR) becomes superfluous once this lands; close it as 'bundled into #28'. --- fleet/ohm.yaml | 3 + ...600-take-pending-record-lock-with-bh.patch | 118 ++++++++++++++++++ .../README.md | 19 +++ 3 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/fleet/ohm.yaml b/fleet/ohm.yaml index edb0cd6..2495aa1 100644 --- a/fleet/ohm.yaml +++ b/fleet/ohm.yaml @@ -49,6 +49,9 @@ includes: # (current ohm setting). Kept in the manifest for the day SCS gets # re-enabled. See reference_arm64_scs_arm_neon_gcc15 memory. - arch/arm64/xor-neon-ffixed-x18-scs-build-fix-danctnix/ + # close besser#18 — pending_record_lock SOFTIRQ-safe -> -unsafe inversion. + # Mirror of marfrit/bes2600-dkms#11 (d95453c). 5-site spin_lock -> _bh. + - driver/bes2600/queue-pending-record-lock-bh-danctnix/ # Explicitly NOT included (decision logged): # - debian-copyright-fsf-address: Debian packaging metadata, not kernel 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..a1d763d --- /dev/null +++ b/patches/driver/bes2600/queue-pending-record-lock-bh-danctnix/0001-bes2600-take-pending-record-lock-with-bh.patch @@ -0,0 +1,118 @@ +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); 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.