Compare commits
36 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| f8986a4a18 | |||
| 122582e270 | |||
| ae175f9745 | |||
| 693e9b42aa | |||
| 0f783a1e69 | |||
| 843d40231f | |||
| 6ab61b9a06 | |||
| 216c7c59b1 | |||
| 02d3f4b222 | |||
| 3d63ec0a35 | |||
| 722434414a | |||
| fc88ff41c3 | |||
| fde41fcdd4 | |||
| 6bae531917 | |||
| 3a38286e6f | |||
| 1e408c9d33 | |||
| d01400140b | |||
| 993117a108 | |||
| 0b63ca3c24 | |||
| 4666e03254 | |||
| f232476240 | |||
| 08c7aafb48 | |||
| 809e3cce84 | |||
| 4344873f2d | |||
| 679083d1aa | |||
| 594f73c6b4 | |||
| 928268f477 | |||
| 425eb92456 | |||
| 1830c17891 | |||
| 69a1d0f8b1 | |||
| 458ad36f8b | |||
| ea509e810f | |||
| e53aad5013 | |||
| 4acba3e707 | |||
| f6a25d811f | |||
| 07a7d4b3af |
@@ -0,0 +1,172 @@
|
||||
# linux-pinetab2-danctnix-besser
|
||||
|
||||
Soft-upstream fork of `linux-pinetab2` (DanctNIX kernel for PineTab2) carrying the **BESser** bes2600 staging-driver patchset.
|
||||
|
||||
Drop-in replacement for `linux-pinetab2`. Same kernel version, same config (one toggle aside — see SCS caveat below), same modules — only the `drivers/staging/bes2600/` driver differs.
|
||||
|
||||
---
|
||||
|
||||
## TL;DR
|
||||
|
||||
| | |
|
||||
|---|---|
|
||||
| **Current package** | `linux-pinetab2-danctnix-besser-7.0.danctnix1-3-aarch64.pkg.tar.zst` |
|
||||
| **Module srcversion** | `BEB625FA7443171EA8D55F7` (`bes2600.ko`) |
|
||||
| **Kernel base** | DanctNIX [`linux-pinetab2`](https://codeberg.org/DanctNIX/linux-pinetab2) tag `v7.0-danctnix1` |
|
||||
| **What it fixes vs upstream** | +73 % TX throughput, the `wsm_generic_confirm 0x0007` dmesg storm (besser#1 closed), the firmware-PSM-not-honored hang, the multi-function SDIO LMAC-wedge recovery |
|
||||
| **What it adds today vs pkgrel=1** | **Patch I**: 5 GHz scan filter — `iw scan freq <single-5ghz-channel>` works, multi-channel per-band sweep refused at driver boundary to dodge firmware reject cascade. NM `band=a` profiles associate to 5 GHz cleanly. **Sustained 11.32 MB/s** download (2.54 GB factory image) on `newton` 5 GHz ch.48 — **3.6× the 2.4 GHz baseline of 3.12 MB/s** on the same source. |
|
||||
| **Source-of-truth** | `git.reauktion.de/marfrit/bes2600-dkms` — branch `cleanups` for c-stack+A+B, branch `bes2600/scan-filter-5ghz` for Patch I |
|
||||
| **This PKGBUILD** | `git.reauktion.de/marfrit/besser` `claude-noether-14` `danctnix-besser-pkgbuild/kernel/` |
|
||||
| **Kernel-agent mirror** | `git.reauktion.de/marfrit/kernel-agent` `fleet/ohm.yaml` (manifest) + `patches/driver/bes2600/scan-filter-5ghz-danctnix/` |
|
||||
| **Caveat** | `CONFIG_SHADOW_CALL_STACK=n` (security-hardening regression, workaround for a GCC 15.2.1 + arm_neon.h pragma issue — tracked in [besser#20](https://git.reauktion.de/marfrit/besser/issues/20), restore to `=y` when GCC is fixed) |
|
||||
|
||||
---
|
||||
|
||||
## What's in the patchset
|
||||
|
||||
A 17-commit cumulative diff over `v7.0-danctnix1`'s in-tree `drivers/staging/bes2600/`, plus the standalone Patch I (5 GHz scan filter) and an arm64 build-environment workaround for GCC 15.
|
||||
|
||||
Individual commits with full rationale + Phase-7 verification logs live on the **`cleanups` branch** of [`marfrit/bes2600-dkms`](https://git.reauktion.de/marfrit/bes2600-dkms/commits/branch/cleanups) and the **`bes2600/scan-filter-5ghz` branch** for Patch I. This PKGBUILD ships them squashed into separate patch files for build atomicity.
|
||||
|
||||
| group | what it does |
|
||||
|---|---|
|
||||
| **c-stack (c5.1–c5.2.1, c6.1, c6.2, c7)** | wifi-stability fixes: scan-defer-on-firmware-reject, scan-defer-backoff-tune, LMAC recover via `mmc_hw_reset`, PM state resync, wake-state consume, firmware-doesn't-honour-PSM self-detect, multi-function SDIO `mmc_hw_reset` rescan |
|
||||
| **Patch A** | decrypt-storm fast-recover at `bes2600_rx_cb`: ≥5 `WSM_STATUS_DECRYPTFAILURE` in 5 s → `ieee80211_connection_loss(vif)`. Phase-7 confirmed N=2 (2026-05-07), storms recover ~1 s vs 109 s baseline. |
|
||||
| **Patch B** | connection-loss bus-reset: ≥3 driver-side connection-loss decisions in 60 s on the same vif → `mmc_hw_reset` instead of mac80211 reauth. Installed dormant; never tripped in production yet. |
|
||||
| **Patch C v3** | structural: drop `sdio_rx_work` workqueue relay; IRQ → bh-direct architecture (matches mainline cw1200). +73 % sustained RX. |
|
||||
| **Patch D** | `ba_lock` removed; `ba_acc/ba_cnt/ba_acc_rx/ba_cnt_rx/ba_ena` → `atomic_t`; per-RX-frame spinlock eliminated. |
|
||||
| **Patch E** | per-RX-frame `ps_state_lock` skipped when c7's `pm_unsupported` latch is on (steady-state on production firmware). |
|
||||
| **Patch F** | cw1200 mainline backports: hw_scan SKB-lifecycle UAF, `init_common` `destroy_workqueue` on error, `atomic_add(1, x) → atomic_inc(x)` cosmetic. |
|
||||
| **Patch G** | GPL-2.0 §1 attribution restoration: SPDX-License-Identifier on every file, Tarnyagin/ST-Ericsson copyright restored on cw1200-derived files. |
|
||||
| **Patch C2** | `ieee80211_rx_irqsafe → ieee80211_rx_ni` at all 6 sites (kernel.org-clean process-context API; tasklet hop removed). |
|
||||
| **Patch H** | `bh.c` hygiene cleanup: 76- and 468-line `#if 0` cw1200-ancestor fossil blocks removed; `__bes2600_irq_enable` stub removed; per-iteration `BUG_ON` → `WARN_ON_ONCE`. |
|
||||
| **Patch I** ([besser#1](https://git.reauktion.de/marfrit/besser/issues/1)) | **5 GHz scan filter.** Refuses only **multi-channel** 5 GHz scans (the per-band-sweep mac80211 issues internally) at the driver boundary with `-EOPNOTSUPP`, dodging the firmware's status-2 reject cascade. Single-channel 5 GHz scans pass through so NM/`wpa_supplicant` per-freq BSS discovery (when `802-11-wireless.band=a`) still finds and associates to 5 GHz APs. Net effect: dmesg storm gone, 5 GHz attachment works, 3.6× sustained throughput on 5 GHz HT40 vs 2.4 GHz HT20. |
|
||||
| **arm64 SCS Makefile workaround** | Adds `-ffixed-x18` explicitly for `arch/arm64/lib/xor-neon.o` when `CONFIG_SHADOW_CALL_STACK=y`. Dead code in this pkgrel (SCS is off), in place for the day SCS re-enable becomes possible. See [besser#20](https://git.reauktion.de/marfrit/besser/issues/20). |
|
||||
|
||||
## Measured outcome
|
||||
|
||||
- **Phase 7 (Patch I, 2026-05-18):** Pattern A `wsm_generic_confirm failed for request 0x0007` storm: 14.3/h → **0/h** over 30-min observation. 5 GHz `newton` BSSID `c0:25:06:e6:5b:33` @ 5240 MHz (ch.48), TX bitrate 150 Mbit/s MCS 7 HT40 short-GI. Internet download throughput **11.32 MB/s** (sustained 90.5 Mbit/s, ~60 % of PHY) vs 3.12 MB/s on 2.4 GHz HT20 same source.
|
||||
- **Phase 7 (Patch C v3 + F + G + D + E + C2 + H, Mobian-flavor):** N=3 stress @ 4 MB/s sender on RK3566/PineTab2 — Patch B baseline 1.36 MB/s → +73 % sustained 2.28 MB/s. Race-fix verified under stress (no `wsm_release_tx_buffer` WARN storm under load).
|
||||
- Module loads + associates cleanly; `pm_unsupported` latch fires on boot as expected.
|
||||
|
||||
## Building
|
||||
|
||||
```sh
|
||||
makepkg -s
|
||||
```
|
||||
|
||||
Identical workflow to upstream `linux-pinetab2`. Produces `linux-pinetab2-danctnix-besser-<ver>-aarch64.pkg.tar.zst` plus a matching `-headers` package. Build host can be aarch64 native (recommended — no cross-toolchain setup) or x86 with an aarch64 cross-compiler.
|
||||
|
||||
Build time: ~45–55 min on an 8-core aarch64 host (boltzmann/RPi5-class), most of it the kernel modules phase.
|
||||
|
||||
**GCC 15.2.1 note:** This pkgrel ships with `CONFIG_SHADOW_CALL_STACK=n` because GCC 15.2.1's strict pragma validator chokes on `arm_neon.h`'s push/`target("+nothing+aes")`/pop sequences when SCS is on. The `0003-arm64-xor-neon-ffixed-x18-build-fix.patch` is a defensive Makefile-side workaround that's a no-op while SCS is off; it'll silently unblock SCS=y once GCC upstream is fixed. See [besser#20](https://git.reauktion.de/marfrit/besser/issues/20) for the re-enable plan.
|
||||
|
||||
## Installing
|
||||
|
||||
The package declares `provides=("linux-pinetab2=$pkgver-$pkgrel")` and `conflicts=(linux-pinetab2)`, so `pacman` will cleanly take over from upstream `linux-pinetab2`:
|
||||
|
||||
```sh
|
||||
sudo pacman -U linux-pinetab2-danctnix-besser-7.0.danctnix1-3-aarch64.pkg.tar.zst
|
||||
```
|
||||
|
||||
That removes the upstream `linux-pinetab2` package (if installed) and registers the BESser-flavored kernel under the same provides slot. Headers package is optional; install it if you build out-of-tree modules.
|
||||
|
||||
The pacman `mkinitcpio` hook auto-generates `/boot/initramfs-linux-pinetab2-danctnix-besser.img`. Modules land in `/usr/lib/modules/<release>-pinetab2-danctnix-besser/`, vmlinuz at `/boot/vmlinuz-linux-pinetab2-danctnix-besser`, DTBs at `/boot/dtbs/rockchip/rk3566-pinetab2-{v0.1,v2.0}.dtb`.
|
||||
|
||||
### Bootloader (PineTab2-specific)
|
||||
|
||||
PineTab2 boots via U-Boot loading a script `boot.scr` (compiled from `/boot/boot.txt` via `mkscr`). After install, point the script at the new kernel + initramfs:
|
||||
|
||||
```sh
|
||||
sudo cp /boot/boot.txt /boot/boot.txt.pre-besser
|
||||
sudo cp /boot/boot.scr /boot/boot.scr.pre-besser
|
||||
sudo sed -i \
|
||||
-e 's|/vmlinuz-linux-pinetab2$|/vmlinuz-linux-pinetab2-danctnix-besser|' \
|
||||
-e 's|/initramfs-linux-pinetab2\.img|/initramfs-linux-pinetab2-danctnix-besser.img|' \
|
||||
/boot/boot.txt
|
||||
cd /boot && sudo ./mkscr
|
||||
sudo systemctl reboot
|
||||
```
|
||||
|
||||
Backups (`*.pre-besser`) let you revert without touching the U-Boot console: `sudo cp /boot/boot.scr.pre-besser /boot/boot.scr` and reboot.
|
||||
|
||||
## Verifying
|
||||
|
||||
After reboot:
|
||||
|
||||
```sh
|
||||
uname -r
|
||||
# expected: <kver>-pinetab2-danctnix-besser
|
||||
|
||||
lsmod | grep -i bes2600
|
||||
# expected: bes2600 (loaded), bes2600_btuart (loaded if Bluetooth in use)
|
||||
|
||||
cat /sys/module/bes2600/srcversion
|
||||
# expected: BEB625FA7443171EA8D55F7 for pkgrel=3
|
||||
```
|
||||
|
||||
`dmesg | grep bes2600` should show clean firmware load, no SDIO TX panic, no `wsm_release_tx_buffer` WARN storm under load, no `wsm_generic_confirm failed for request 0x0007` storm.
|
||||
|
||||
For the 5 GHz fix specifically:
|
||||
```sh
|
||||
sudo iw dev wlan0 scan freq 5180
|
||||
# expected: completes, no "Operation not supported"
|
||||
|
||||
sudo iw dev wlan0 scan freq 5180 5200 5220 5240
|
||||
# expected: "Operation not supported (-95)" — multi-channel 5 GHz refused
|
||||
```
|
||||
|
||||
## Rolling back
|
||||
|
||||
If the new kernel misbehaves:
|
||||
|
||||
```sh
|
||||
sudo cp /boot/boot.scr.pre-besser /boot/boot.scr
|
||||
sudo systemctl reboot
|
||||
```
|
||||
|
||||
That returns you to whatever kernel `boot.scr` was pointing at before the install (typically upstream `linux-pinetab2` or the previous `linux-pinetab2-danctnix-besser`). The package itself can be removed with `sudo pacman -R linux-pinetab2-danctnix-besser` and the original `linux-pinetab2` re-installed via `sudo pacman -S linux-pinetab2`.
|
||||
|
||||
## Provenance
|
||||
|
||||
- Mobian-flavor source-of-truth: <https://git.reauktion.de/marfrit/bes2600-dkms> (`cleanups` branch for c-stack + Patches A/B, `bes2600/scan-filter-5ghz` for Patch I)
|
||||
- Per-patch breakdown, Phase 0–7 logs, follow-up issues: <https://git.reauktion.de/marfrit/besser>
|
||||
- Upstream cw1200 mainline (architectural reference): `drivers/net/wireless/st/cw1200/` in linux-rockchip
|
||||
- Kernel base: <https://codeberg.org/DanctNIX/linux-pinetab2> tag `v7.0-danctnix1`
|
||||
- Kernel-agent mirror of the patch tree + per-host manifest: <https://git.reauktion.de/marfrit/kernel-agent>
|
||||
|
||||
## Why it's "BESser"
|
||||
|
||||
"Besser" = German for "better." Patch series ID across both DKMS (Mobian) and in-tree (Danctnix) trees. Single source-of-truth lives in `marfrit/bes2600-dkms`; this PKGBUILD is the danctnix-flavor consumption surface.
|
||||
|
||||
## Soft-upstream intent
|
||||
|
||||
Submitting this PKGBUILD to DanctNIX for review. If accepted as a replacement for `linux-pinetab2` (or sidegrade), the BESser patchset ships to all PineTab2 users via the regular danctnix package update channel. The bes2600 driver gets:
|
||||
|
||||
- ~2× sustained RX throughput on 2.4 GHz
|
||||
- ~3.6× sustained RX throughput on 5 GHz (via Patch I + correctly using HT40)
|
||||
- Race-correctness on the hot path
|
||||
- GPL-2.0 §1 attribution compliance
|
||||
- Modern kernel API (no deprecated `from_timer`, no `_irqsafe` from process context, no `BUG_ON` in steady-state)
|
||||
|
||||
Drop-in compatibility: same kernel version, same module names, no userspace ABI change. SCS off is the one config caveat, tracked in [besser#20](https://git.reauktion.de/marfrit/besser/issues/20).
|
||||
|
||||
## Maintenance plan
|
||||
|
||||
- New danctnix kernel release → rebase BESser patches onto the new tag, regenerate cumulative diff, bump pkgver.
|
||||
- New BESser patch on Mobian DKMS → re-overlay + re-flavor + regenerate cumulative diff.
|
||||
- Both flavors continue to be maintained in lockstep via `marfrit/bes2600-dkms` source-of-truth.
|
||||
- GCC 15 SCS issue → periodically re-test build with `CONFIG_SHADOW_CALL_STACK=y` against current Arch ARM GCC. When the build succeeds, flip the config and re-deploy.
|
||||
|
||||
## Known gaps
|
||||
|
||||
- Cumulative diff (squashed) for the c-stack + Patches A/B; Patch I as a separate `0002-` file. Per-patch series can be regenerated if danctnix maintainers prefer.
|
||||
- Bluetooth-side `bes2600_btuart` is independent and untouched by this patchset.
|
||||
- `bes2600_switch_bt` orchestration removed (Mobian-only entry point; not used in danctnix tree).
|
||||
- Multi-band `iw scan` (no `freq` filter) still reports aborted scan because mac80211 aggregates per-band results and marks the whole scan aborted when any leg returns negative (mac80211 contract, not bes2600). Single-band scans (`iw scan freq 2462` or `iw scan freq 5180`) work normally; `nmcli connection up` with `band=bg` or `band=a` profile works normally. This is the Phase 5 reviewer's predicted residual limitation; userspace tools that need full multi-band BSS discovery should issue per-band scans.
|
||||
|
||||
## Author
|
||||
|
||||
Markus Fritsche <fritsche.markus@gmail.com>
|
||||
|
||||
Built collaboratively with Claude Opus 4.7 (1M context).
|
||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,168 @@
|
||||
From 093a5038b8b68f316d976b7cb69609ca7f24f322 Mon Sep 17 00:00:00 2001
|
||||
From: Markus Fritsche <fritsche.markus@gmail.com>
|
||||
Date: Mon, 18 May 2026 11:27:40 +0200
|
||||
Subject: [PATCH 1/2] bes2600: filter 5 GHz scans at the driver boundary
|
||||
(besser#1)
|
||||
|
||||
The BES2600 firmware refuses WSM start-scan for 5 GHz with status 2
|
||||
("rejected by policy"). This shows up in dmesg as the recurring
|
||||
|
||||
wsm_generic_confirm failed for request 0x0007.
|
||||
[SCAN] Scan failed (-22).
|
||||
|
||||
pattern (besser issue #1, ~14-16/h on ohm/PineTab2 baseline).
|
||||
|
||||
Trace shows every reject is the second of a back-to-back pair: mac80211
|
||||
splits multi-band hw_scan requests per band when the driver does not
|
||||
set IEEE80211_HW_SINGLE_SCAN_ON_ALL_BANDS (we don't), then re-invokes
|
||||
drv_hw_scan from __ieee80211_scan_completed for each subsequent band.
|
||||
The 2.4 GHz iteration succeeds; the 5 GHz iteration is what the
|
||||
firmware rejects. See ieee80211_prep_hw_scan in net/mac80211/scan.c
|
||||
for the loop, and the existing memory reference_bes2600_5ghz_scan_reject
|
||||
for the firmware behaviour.
|
||||
|
||||
The 056a71a defer-on-reject patch already in this tree handles the
|
||||
BT-A2DP-coex branch and the consecutive-reject backoff, but it cannot
|
||||
prevent the per-band-loop reject: by the time defer_should_scan is
|
||||
consulted, the per-band call is already in flight, and the reject_count
|
||||
gets reset on every successful 2.4 GHz scan in between (which is
|
||||
~36% of attempts), so the threshold never trips.
|
||||
|
||||
The fix: refuse the 5 GHz iteration upfront in bes2600_hw_scan. The
|
||||
2.4 GHz scan still runs normally. The 5 GHz portion is reported as
|
||||
aborted to userspace -- same outcome as today, minus the dmesg storm
|
||||
and the wsm_generic_confirm WARN cascade.
|
||||
|
||||
5 GHz band registration is intentionally left in place: direct-BSSID
|
||||
association to a known 5 GHz AP still works (no scan is needed for
|
||||
that path), and a future firmware update that fixes the scan behaviour
|
||||
should not be foreclosed by changing band advertisement.
|
||||
|
||||
Contract: per include/net/mac80211.h ieee80211_ops.hw_scan, a negative
|
||||
return aborts the scan without requiring ieee80211_scan_completed().
|
||||
-EOPNOTSUPP is the semantically accurate code (operation is legal,
|
||||
driver can't service it on this band today).
|
||||
|
||||
Phase 3 evidence:
|
||||
- baseline N=3: rate ~14.3-23.6/h converged at 14.3/h (matches OP)
|
||||
- back-to-back scan gap: 6/6 rejected pairs <200us, 1/1 successful
|
||||
pair was 114ms (single-band-only, no 5 GHz leg)
|
||||
- defer log fires: 0/9 in 30-min window (056a71a structurally bypassed)
|
||||
|
||||
Predicted Phase 7 delta: Pattern A 14/h -> 0/h.
|
||||
---
|
||||
bes2600/scan.c | 22 ++++++++++++++++++++++
|
||||
1 file changed, 22 insertions(+)
|
||||
|
||||
diff --git a/drivers/staging/bes2600/scan.c b/drivers/staging/bes2600/scan.c
|
||||
index fb1d298..a81afb6 100644
|
||||
--- a/drivers/staging/bes2600/scan.c
|
||||
+++ b/drivers/staging/bes2600/scan.c
|
||||
@@ -238,6 +238,28 @@ int bes2600_hw_scan(struct ieee80211_hw *hw,
|
||||
/* Scan when P2P_GO corrupt firmware MiniAP mode */
|
||||
if (priv->join_status == BES2600_JOIN_STATUS_AP)
|
||||
return -EOPNOTSUPP;
|
||||
+
|
||||
+ /*
|
||||
+ * Firmware refuses WSM start-scan for 5 GHz with status 2 ("rejected
|
||||
+ * by policy"); see besser issue #1. mac80211 splits multi-band
|
||||
+ * hw_scan requests per-band when the driver does not set
|
||||
+ * IEEE80211_HW_SINGLE_SCAN_ON_ALL_BANDS (we don't -- see
|
||||
+ * ieee80211_hw_set() calls in bes2600_main.c), so each per-band call
|
||||
+ * has req->channels[] from one band only (see ieee80211_prep_hw_scan
|
||||
+ * in net/mac80211/scan.c). Refuse the 5 GHz iteration at the driver
|
||||
+ * boundary so userspace gets a clean aborted-scan for that portion
|
||||
+ * rather than waiting for the firmware reject to cascade up. 5 GHz
|
||||
+ * band registration stays intact so direct-BSSID association to a
|
||||
+ * known 5 GHz AP still works (no scan needed for that path).
|
||||
+ *
|
||||
+ * Contract: per include/net/mac80211.h struct ieee80211_ops.hw_scan
|
||||
+ * documentation, a negative return aborts the scan without requiring
|
||||
+ * ieee80211_scan_completed().
|
||||
+ */
|
||||
+ if (req->n_channels > 0 &&
|
||||
+ req->channels[0]->band == NL80211_BAND_5GHZ)
|
||||
+ return -EOPNOTSUPP;
|
||||
+
|
||||
#if 0
|
||||
if (work_pending(&priv->offchannel_work) ||
|
||||
(hw_priv->roc_if_id != -1)) {
|
||||
--
|
||||
2.54.0
|
||||
|
||||
|
||||
From 8cd10f487c8144d462a510812ba0fa717b3e24df Mon Sep 17 00:00:00 2001
|
||||
From: Markus Fritsche <fritsche.markus@gmail.com>
|
||||
Date: Mon, 18 May 2026 15:56:34 +0200
|
||||
Subject: [PATCH 2/2] bes2600: scan-filter-5ghz: allow targeted single-channel
|
||||
scans (besser#1 follow-up)
|
||||
|
||||
The original Patch I refused EVERY 5 GHz scan request unconditionally
|
||||
(req->n_channels > 0 && band == NL80211_BAND_5GHZ). This eliminated
|
||||
the Pattern A storm but also broke 5 GHz association entirely:
|
||||
NM / wpa_supplicant iterates a freq_list when a connection profile
|
||||
specifies 802-11-wireless.band=a, issuing per-frequency single-channel
|
||||
scans to find the BSS before associating. Those single-channel scans
|
||||
were also refused by our guard, so the BSS was never seen and
|
||||
'Wi-Fi network could not be found' was the only outcome.
|
||||
|
||||
Tighten the guard: refuse only multi-channel 5 GHz scans (n_channels
|
||||
> 1), which is the per-band-sweep pattern mac80211 issues internally
|
||||
and the only one that triggers the firmware storm at the per-band
|
||||
loop boundary. Single-channel 5 GHz scans pass through to firmware,
|
||||
which generally accepts them -- and when they happen to be rejected,
|
||||
the failure is isolated and doesn't cascade.
|
||||
|
||||
Verified on ohm with pkgrel=3 (srcversion BEB625FA7443171EA8D55F7):
|
||||
- Pattern A count since boot: 0 (Phase 7 prediction still holds)
|
||||
- iw dev wlan0 scan freq 5180 -> allowed
|
||||
- iw dev wlan0 scan freq 5180 5200 ... -> refused -EOPNOTSUPP
|
||||
- NM 'nmcli connection up' with band=a -> associated to BSSID
|
||||
c0:25:06:e6:5b:33 on 5240 MHz / ch.48 in ~1 second
|
||||
- TX bitrate 150 Mbit/s MCS 7 40MHz short-GI (vs 72.2 Mbit/s
|
||||
HT20 on 2.4 GHz) -- ~2x throughput recovered
|
||||
|
||||
The change is a single byte (> 0 -> > 1) plus comment update; the
|
||||
test confirmation above is what motivates it.
|
||||
|
||||
Refs: besser#1 (closed but tracked for follow-up like this), original
|
||||
Patch I sha 093a503.
|
||||
---
|
||||
bes2600/scan.c | 16 ++++++++++++----
|
||||
1 file changed, 12 insertions(+), 4 deletions(-)
|
||||
|
||||
diff --git a/drivers/staging/bes2600/scan.c b/drivers/staging/bes2600/scan.c
|
||||
index a81afb6..497523b 100644
|
||||
--- a/drivers/staging/bes2600/scan.c
|
||||
+++ b/drivers/staging/bes2600/scan.c
|
||||
@@ -248,15 +248,23 @@ int bes2600_hw_scan(struct ieee80211_hw *hw,
|
||||
* has req->channels[] from one band only (see ieee80211_prep_hw_scan
|
||||
* in net/mac80211/scan.c). Refuse the 5 GHz iteration at the driver
|
||||
* boundary so userspace gets a clean aborted-scan for that portion
|
||||
- * rather than waiting for the firmware reject to cascade up. 5 GHz
|
||||
- * band registration stays intact so direct-BSSID association to a
|
||||
- * known 5 GHz AP still works (no scan needed for that path).
|
||||
+ * rather than waiting for the firmware reject to cascade up.
|
||||
+ *
|
||||
+ * Only the multi-channel case is refused (n_channels > 1): that's
|
||||
+ * the per-band-sweep pattern mac80211 issues internally and the
|
||||
+ * one that triggers the firmware storm at the per-band loop
|
||||
+ * boundary. Single-channel 5 GHz scans (BSS verification, NM's
|
||||
+ * per-freq iteration when 802-11-wireless.band=a is set) pass
|
||||
+ * through to firmware, which generally accepts them since the
|
||||
+ * storm is the back-to-back per-band issue, not a blanket 5 GHz
|
||||
+ * reject. This preserves 5 GHz association via the
|
||||
+ * "wpa_supplicant iterates freq_list per channel" path.
|
||||
*
|
||||
* Contract: per include/net/mac80211.h struct ieee80211_ops.hw_scan
|
||||
* documentation, a negative return aborts the scan without requiring
|
||||
* ieee80211_scan_completed().
|
||||
*/
|
||||
- if (req->n_channels > 0 &&
|
||||
+ if (req->n_channels > 1 &&
|
||||
req->channels[0]->band == NL80211_BAND_5GHZ)
|
||||
return -EOPNOTSUPP;
|
||||
|
||||
--
|
||||
2.54.0
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
From: Markus Fritsche <fritsche.markus@gmail.com>
|
||||
Date: Mon, 18 May 2026 11:42:00 +0200
|
||||
Subject: [PATCH] arm64: xor-neon: restore -ffixed-x18 when SHADOW_CALL_STACK=y
|
||||
(GCC 15+ build fix)
|
||||
|
||||
GCC 15.2.1 enforces that -fsanitize=shadow-call-stack requires
|
||||
-ffixed-x18 inside arm_neon.h's #pragma GCC target() blocks. The
|
||||
existing CFLAGS_REMOVE_xor-neon.o line strips the kernel-wide
|
||||
-ffixed-x18 (it's part of CC_FLAGS_NO_FPU) and CC_FLAGS_FPU does not
|
||||
restore it, so xor-neon.c fails to build on stricter GCC versions
|
||||
when CONFIG_SHADOW_CALL_STACK=y.
|
||||
|
||||
Add an explicit -ffixed-x18 just for this object, gated on the
|
||||
SCS config so non-SCS builds are unaffected.
|
||||
|
||||
Build environment workaround; not a kernel-runtime bug.
|
||||
---
|
||||
arch/arm64/lib/Makefile | 4 ++++
|
||||
1 file changed, 4 insertions(+)
|
||||
|
||||
diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
|
||||
index 1234567..2345678 100644
|
||||
--- a/arch/arm64/lib/Makefile
|
||||
+++ b/arch/arm64/lib/Makefile
|
||||
@@ -9,6 +9,10 @@ ifeq ($(CONFIG_KERNEL_MODE_NEON), y)
|
||||
obj-$(CONFIG_XOR_BLOCKS) += xor-neon.o
|
||||
CFLAGS_xor-neon.o += $(CC_FLAGS_FPU)
|
||||
CFLAGS_REMOVE_xor-neon.o += $(CC_FLAGS_NO_FPU)
|
||||
+# GCC 15+ enforces that -fsanitize=shadow-call-stack requires -ffixed-x18
|
||||
+# even after a #pragma GCC pop_options inside arm_neon.h. CC_FLAGS_REMOVE
|
||||
+# above strips the kernel-wide -ffixed-x18 (part of CC_FLAGS_NO_FPU); add
|
||||
+# it back here so xor-neon.c still compiles when SHADOW_CALL_STACK=y.
|
||||
+CFLAGS_xor-neon.o += $(if $(CONFIG_SHADOW_CALL_STACK),-ffixed-x18)
|
||||
endif
|
||||
|
||||
lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o
|
||||
@@ -0,0 +1,236 @@
|
||||
# Maintainer: Markus Fritsche <fritsche.markus@gmail.com>
|
||||
# Forked from: linux-pinetab2 by Danct12 <danct12@disroot.org>
|
||||
# Original Contributor: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
|
||||
#
|
||||
# linux-pinetab2-danctnix-besser: linux-pinetab2 + the BESser
|
||||
# bes2600 driver patchset (race-fix, lock-removal, attribution-restore,
|
||||
# fossil-cleanup; +73% throughput vs the in-tree baseline). Soft-upstream
|
||||
# fork of linux-pinetab2 — drop-in replacement, same kernel version, only
|
||||
# the bes2600 staging driver differs. See git.reauktion.de/marfrit/besser
|
||||
# and git.reauktion.de/marfrit/bes2600-dkms for full provenance.
|
||||
|
||||
pkgbase=linux-pinetab2-danctnix-besser
|
||||
pkgver=7.0.danctnix1
|
||||
pkgrel=3
|
||||
pkgdesc='PineTab2 (BESser bes2600 driver patchset)'
|
||||
_srcname=linux-pinetab2
|
||||
_srctag=v${pkgver%.*}-${pkgver##*.}
|
||||
arch=(aarch64)
|
||||
_url_git="https://codeberg.org/DanctNIX/${_srcname}"
|
||||
url="${_url_git}/commits/tag/$_srctag"
|
||||
license=(GPL-2.0-only)
|
||||
makedepends=(
|
||||
bc
|
||||
cpio
|
||||
gettext
|
||||
git
|
||||
libelf
|
||||
pahole
|
||||
perl
|
||||
python
|
||||
tar
|
||||
xz
|
||||
)
|
||||
options=(
|
||||
!debug
|
||||
!strip
|
||||
)
|
||||
source=(
|
||||
https://cdn.kernel.org/pub/linux/kernel/v${pkgver%%.*}.x/linux-${pkgver%.*}.tar.{xz,sign}
|
||||
${_url_git}/releases/download/${_srctag}/${_srctag}.patch.zst{,.sig}
|
||||
0001-bes2600-besser-cumulative-series.patch
|
||||
0002-bes2600-filter-5ghz-scan.patch
|
||||
0003-arm64-xor-neon-ffixed-x18-build-fix.patch
|
||||
0003-arm64-xor-neon-ffixed-x18-build-fix.patch
|
||||
config # the main kernel config file
|
||||
)
|
||||
validpgpkeys=(
|
||||
ABAF11C65A2970B130ABE3C479BE3E4300411886 # Linus Torvalds
|
||||
647F28654894E3BD457199BE38DBBDC86092693E # Greg Kroah-Hartman
|
||||
F09A933C0FE0331E558CA4E166CAB7EAA45DD781 # Danct12
|
||||
)
|
||||
b2sums=('3d9795083c8938f80f480de0d10bfd9c525640e59d5c7f22983de3f12ee42c84c31be902cafb05579ddb1c32bac5ed06b0d4953f9705450be185bd2d9ab08f89'
|
||||
'SKIP'
|
||||
'71fe98221e802b315e54b4b10d3e8c8f376695a36bae3541d876e5776a37f3fa33c8f8dfa6e51fcbd6f5396add02e5166634165f2351836a0ea0453c172fe56c'
|
||||
'SKIP'
|
||||
'fca0a5badf762d5dbc085261cccc07ddeef96384d2ae0a426fb0412acd7a180e068cabd59f01342b7575d41889afc0f47dfbc9256801ab809f746278e6dab510'
|
||||
'396acbdcf570eada62533c0b8f505ed18077e8432249bab5b8ac8d1107cabc9489bdb91a5780446237ec4fd9ba5fc57a49dff34c16ddab60dc30513fc535f00f'
|
||||
'2714e3c0cd8ec978ce9431418f44f578220886fcabb738c9a0c43fc3c043753960b7c47ae96e1780154d8b266a2add6098407de4ffa7aee40d77ce17e8c70df9'
|
||||
'2714e3c0cd8ec978ce9431418f44f578220886fcabb738c9a0c43fc3c043753960b7c47ae96e1780154d8b266a2add6098407de4ffa7aee40d77ce17e8c70df9'
|
||||
'656a998ab40cb85ee4c00f087b071a91632a6c091da2c84b0f74236b51d2dea6e9db6886625f80ad81dc249d8494ec47cd79d6dd9ea4f5e44f3cde857f861e10')
|
||||
|
||||
export KBUILD_BUILD_HOST=archlinux
|
||||
export KBUILD_BUILD_USER=$pkgbase
|
||||
export KBUILD_BUILD_TIMESTAMP="$(date -Ru${SOURCE_DATE_EPOCH:+d @$SOURCE_DATE_EPOCH})"
|
||||
|
||||
prepare() {
|
||||
cd linux-${pkgver%.*}
|
||||
|
||||
echo "Setting version..."
|
||||
echo "-$pkgrel" > localversion.10-pkgrel
|
||||
echo "${pkgbase#linux}" > localversion.20-pkgname
|
||||
|
||||
local src
|
||||
for src in "${source[@]}"; do
|
||||
src="${src%%::*}"
|
||||
src="${src##*/}"
|
||||
src="${src%.zst}"
|
||||
[[ $src = *.patch ]] || continue
|
||||
echo "Applying patch: $src..."
|
||||
patch -Np1 < "../$src"
|
||||
done
|
||||
|
||||
echo "Setting config..."
|
||||
cp ../config .config
|
||||
make olddefconfig
|
||||
diff -u ../config .config || :
|
||||
|
||||
make -s kernelrelease > version
|
||||
echo "Prepared $pkgbase version $(<version)"
|
||||
}
|
||||
|
||||
build() {
|
||||
cd linux-${pkgver%.*}
|
||||
make DTC_FLAGS="-@" all
|
||||
make -C tools/bpf/bpftool vmlinux.h feature-clang-bpf-co-re=1
|
||||
}
|
||||
|
||||
_package() {
|
||||
pkgdesc="The $pkgdesc kernel and modules"
|
||||
depends=(
|
||||
coreutils
|
||||
kmod
|
||||
mkinitcpio
|
||||
)
|
||||
optdepends=(
|
||||
'wireless-regdb: to set the correct wireless channels of your country'
|
||||
'linux-firmware: firmware images needed for some devices'
|
||||
)
|
||||
provides=(
|
||||
KSMBD-MODULE
|
||||
WIREGUARD-MODULE
|
||||
"linux-pinetab2=$pkgver-$pkgrel"
|
||||
)
|
||||
conflicts=(linux-pinetab2)
|
||||
replaces=(
|
||||
wireguard-arch
|
||||
)
|
||||
|
||||
cd linux-${pkgver%.*}
|
||||
local modulesdir="$pkgdir/usr/lib/modules/$(<version)"
|
||||
|
||||
echo "Installing boot image..."
|
||||
# systemd expects to find the kernel here to allow hibernation
|
||||
# https://github.com/systemd/systemd/commit/edda44605f06a41fb86b7ab8128dcf99161d2344
|
||||
install -Dm644 "$(make -s image_name)" "$modulesdir/vmlinuz"
|
||||
|
||||
# Used by mkinitcpio to name the kernel
|
||||
echo "$pkgbase" | install -Dm644 /dev/stdin "$modulesdir/pkgbase"
|
||||
|
||||
echo "Installing modules..."
|
||||
ZSTD_CLEVEL=19 make INSTALL_MOD_PATH="$pkgdir/usr" INSTALL_MOD_STRIP=1 \
|
||||
DEPMOD=/doesnt/exist modules_install # Suppress depmod
|
||||
|
||||
echo "Installing device trees..."
|
||||
make INSTALL_DTBS_PATH="$pkgdir/boot/dtbs" dtbs_install
|
||||
|
||||
# Removing unnecessary device trees (keep only pinetab2 variants).
|
||||
# Use find -delete instead of a bash for-loop: the previous for-loop
|
||||
# silently no-op'd in the makepkg environment, leaving 234 unrelated
|
||||
# board DTBs in the package. find is robust to nullglob/cwd quirks.
|
||||
find "$pkgdir"/boot/dtbs/rockchip/ -mindepth 1 -maxdepth 1 -type f \
|
||||
! -name 'rk3566-pinetab2-*' -delete
|
||||
|
||||
# remove build link
|
||||
rm "$modulesdir"/build
|
||||
}
|
||||
|
||||
_package-headers() {
|
||||
pkgdesc="Headers and scripts for building modules for the $pkgdesc kernel"
|
||||
depends=(pahole)
|
||||
|
||||
cd linux-${pkgver%.*}
|
||||
local builddir="$pkgdir/usr/lib/modules/$(<version)/build"
|
||||
|
||||
echo "Installing build files..."
|
||||
install -Dt "$builddir" -m644 .config Makefile Module.symvers System.map \
|
||||
localversion.* version vmlinux tools/bpf/bpftool/vmlinux.h
|
||||
install -Dt "$builddir/kernel" -m644 kernel/Makefile
|
||||
install -Dt "$builddir/arch/arm64" -m644 arch/arm64/Makefile
|
||||
cp -t "$builddir" -a scripts
|
||||
|
||||
# required when DEBUG_INFO_BTF_MODULES is enabled
|
||||
install -Dt "$builddir/tools/bpf/resolve_btfids" tools/bpf/resolve_btfids/resolve_btfids
|
||||
|
||||
echo "Installing headers..."
|
||||
cp -t "$builddir" -a include
|
||||
cp -t "$builddir/arch/arm64" -a arch/arm64/include
|
||||
install -Dt "$builddir/arch/arm64/kernel" -m644 arch/arm64/kernel/asm-offsets.s
|
||||
|
||||
install -Dt "$builddir/drivers/md" -m644 drivers/md/*.h
|
||||
install -Dt "$builddir/net/mac80211" -m644 net/mac80211/*.h
|
||||
|
||||
# https://bugs.archlinux.org/task/13146
|
||||
install -Dt "$builddir/drivers/media/i2c" -m644 drivers/media/i2c/msp3400-driver.h
|
||||
|
||||
# https://bugs.archlinux.org/task/20402
|
||||
install -Dt "$builddir/drivers/media/usb/dvb-usb" -m644 drivers/media/usb/dvb-usb/*.h
|
||||
install -Dt "$builddir/drivers/media/dvb-frontends" -m644 drivers/media/dvb-frontends/*.h
|
||||
install -Dt "$builddir/drivers/media/tuners" -m644 drivers/media/tuners/*.h
|
||||
|
||||
# https://bugs.archlinux.org/task/71392
|
||||
install -Dt "$builddir/drivers/iio/common/hid-sensors" -m644 drivers/iio/common/hid-sensors/*.h
|
||||
|
||||
echo "Installing KConfig files..."
|
||||
find . -name 'Kconfig*' -exec install -Dm644 {} "$builddir/{}" \;
|
||||
|
||||
echo "Removing unneeded architectures..."
|
||||
local arch
|
||||
for arch in "$builddir"/arch/*/; do
|
||||
[[ $arch = */arm64/ ]] && continue
|
||||
echo "Removing $(basename "$arch")"
|
||||
rm -r "$arch"
|
||||
done
|
||||
|
||||
echo "Removing documentation..."
|
||||
rm -r "$builddir/Documentation"
|
||||
|
||||
echo "Removing broken symlinks..."
|
||||
find -L "$builddir" -type l -printf 'Removing %P\n' -delete
|
||||
|
||||
echo "Removing loose objects..."
|
||||
find "$builddir" -type f -name '*.o' -printf 'Removing %P\n' -delete
|
||||
|
||||
echo "Stripping build tools..."
|
||||
local file
|
||||
while read -rd '' file; do
|
||||
case "$(file -Sib "$file")" in
|
||||
application/x-sharedlib\;*) # Libraries (.so)
|
||||
strip -v $STRIP_SHARED "$file" ;;
|
||||
application/x-archive\;*) # Libraries (.a)
|
||||
strip -v $STRIP_STATIC "$file" ;;
|
||||
application/x-executable\;*) # Binaries
|
||||
strip -v $STRIP_BINARIES "$file" ;;
|
||||
application/x-pie-executable\;*) # Relocatable binaries
|
||||
strip -v $STRIP_SHARED "$file" ;;
|
||||
esac
|
||||
done < <(find "$builddir" -type f -perm -u+x ! -name vmlinux -print0)
|
||||
|
||||
echo "Stripping vmlinux..."
|
||||
strip -v $STRIP_STATIC "$builddir/vmlinux"
|
||||
|
||||
echo "Adding symlink..."
|
||||
mkdir -p "$pkgdir/usr/src"
|
||||
ln -sr "$builddir" "$pkgdir/usr/src/$pkgbase"
|
||||
}
|
||||
|
||||
pkgname=(
|
||||
"$pkgbase"
|
||||
"$pkgbase-headers"
|
||||
)
|
||||
for _p in "${pkgname[@]}"; do
|
||||
eval "package_$_p() {
|
||||
$(declare -f "_package${_p#$pkgbase}")
|
||||
_package${_p#$pkgbase}
|
||||
}"
|
||||
done
|
||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,180 @@
|
||||
# BES2600 architecture review — Bug #5
|
||||
|
||||
Date assembled: 2026-05-07
|
||||
Reviewer: Claude Sonnet (general-purpose subagent, model=sonnet)
|
||||
Driver source: `~/src/besser/bes2600-dkms-mobian/bes2600/` on boltzmann
|
||||
|
||||
This is the architect-review pass requested in `notes/observed-bugs.md` after the Phase 0 measurement showed the throughput floor is set by per-SDIO-transaction workqueue dispatch overhead. The reviewer was given the measurement summary, source location, and a focused brief; output is a ranked restructuring map with file:line citations for every concrete claim.
|
||||
|
||||
---
|
||||
|
||||
## Measurement context (input to the reviewer)
|
||||
|
||||
```
|
||||
Reproduction: pv -L 4M < /dev/zero | nc ohm 12345
|
||||
Module under test: bes2600.ko srcversion 1B3B3ED0... (cleanups + Patch A + Patch B)
|
||||
Hardware: PineTab2, RK3566 Cortex-A55 ARMv8.5, kernel 6.19.10-danctnix1
|
||||
Link rate: 65 Mb/s ≈ 8 MB/s theoretical
|
||||
Observed throughput: 725 KB/s (Phase 0 anchor at N=3)
|
||||
rep 3 cascaded into beacon-loss disconnect at ~9 min in
|
||||
|
||||
Per-second event rates (3-min capture under 4 MB/s pv-cap):
|
||||
workqueue_execute_start: 5,643/sec ← architectural floor
|
||||
bes2600_rx_cb: 611/sec
|
||||
bes2600_bh_wakeup: 267/sec
|
||||
wsm_cmd_send: 13/sec (host-to-chip command rate, surprisingly low)
|
||||
lock contention_begin: 50/sec (modest)
|
||||
mmc_request_start: ~5,800/sec (matches workqueue rate — every SDIO transaction is its own work item)
|
||||
|
||||
perf record top symbol: _raw_spin_unlock_irqrestore (~20 % CPU samples)
|
||||
Dominant callstack: process_one_work → wsm_configuration → wsm_cmd_send → bes2600_bh.isra.0
|
||||
```
|
||||
|
||||
The implication: ~9 workqueue dispatches fire per frame delivered to mac80211. Items below address that ratio in descending order of predicted leverage.
|
||||
|
||||
---
|
||||
|
||||
## Item 1 — Two-hop workqueue relay: SDIO IRQ → `sdio_rx_work` → BH loop → mac80211
|
||||
|
||||
**File:line:** `bes2600_sdio.c:416` (IRQ handler dispatches `rx_work`); `bes2600_sdio.c:829` (`sdio_rx_work` body); `bh.c:1330–1538` (BH main loop, `BES2600_RX_IN_BH` path); `bes2600_sdio.c:1267` (`bes_sdio` workqueue, `max_active=2`).
|
||||
|
||||
**Current shape:** Every SDIO interrupt fires `queue_work(sdio_wq, &rx_work)`. `sdio_rx_work` reads up to `BES_SDIO_RX_MULTIPLE_NUM=16` frames (`hwio.h:294`) into per-frame SKBs, enqueues each onto `sbus_priv.rx_queue` under `rx_queue_lock`, then returns. Meanwhile the BH kthread (one work item queued at boot in `bh.c:93`, running an infinite loop inside `bes2600_bh()`) calls `pipe_read()` → `spin_lock(rx_queue_lock)` → `skb_dequeue()` → `wsm_handle_rx()` → `ieee80211_rx_irqsafe()` one frame at a time. When `pipe_read()` returns NULL and pending TX exists, `bes2600_sdio_pipe_read()` at `bes2600_sdio.c:941` re-dispatches `rx_work` — so a sustained RX stream fires **one `queue_work` per BH wakeup, not per burst**. That explains why `bh_wakeup` events are only 267/sec while `workqueue_execute_start` is 5,643/sec: the SDIO layer is firing a new `rx_work` item for every frame the BH loop drains.
|
||||
|
||||
**Proposed shape:** Collapse `sdio_rx_work` and `pipe_read()` into the BH loop directly. The BH already runs in a dedicated `WQ_HIGHPRI | WQ_CPU_INTENSIVE` workqueue (`bh.c:66`) and (with `BES2600_RX_IN_BH` defined per `Makefile:159`) `bes2600_bh_rx_helper()` already dequeues from `rx_queue`. Merge `sdio_rx_work` into a function called synchronously from `bes2600_bh_rx_helper()` before the dequeue, guarded by a trylock so re-entry is safe. This eliminates O(N) `queue_work` calls per burst while keeping the BH as the single SDIO-access context.
|
||||
|
||||
**Predicted delta vs Phase 1 metric:** Eliminates ~5 of the ~9 redundant workqueue dispatches per frame. 2–4× throughput improvement and a proportional drop in `_raw_spin_unlock_irqrestore` CPU cost.
|
||||
|
||||
**Effort:** Medium. SDIO host-lock protocol (`sdio_claim_host`/`sdio_release_host`) is already managed inside `sdio_rx_work`; moving the body is mechanical but requires care around the `sdio_wq` `max_active=2` concurrency assumption.
|
||||
|
||||
**Risks:** `sdio_rx_work` runs with `sdio_claim_host` held for the entire burst. Inside the BH it serialises all SDIO access fine. Watch `bes2600_sdio.c:1889` — flushes `rx_work` during teardown; that path must remain.
|
||||
|
||||
---
|
||||
|
||||
## Item 2 — `ieee80211_rx_irqsafe` instead of `ieee80211_rx` (pre-NAPI cw1200 ancestor pattern)
|
||||
|
||||
**File:line:** `txrx.c:1947`, `txrx.c:1950`, `ap.c:99`, `sta.c:1487`, `wsm.c:2416`.
|
||||
|
||||
**Current shape:** Every RX frame is delivered via `ieee80211_rx_irqsafe()`. This function enqueues the SKB onto a per-cpu `tasklet_rx` list and schedules a software IRQ. Under sustained load: one softirq wakeup per frame — 611 softirq wakeups/sec on top of the workqueue overhead.
|
||||
|
||||
**Proposed shape:** Switch to `ieee80211_rx_ni()` (process context, which `wsm_handle_rx` is already in) or, better, batch-deliver frames using `ieee80211_rx_list()` (introduced in kernel 5.12, available in 6.19). Accumulate frames from a single `sdio_rx_work` burst into a `list_head`, then call `ieee80211_rx_list()` once per burst.
|
||||
|
||||
**mac80211 contract:** `ieee80211_rx_list()` is safe from process context with the same `ieee80211_rx_status` rules as `ieee80211_rx_ni()`. Per `include/net/mac80211.h` — kerneldoc states it takes the RX path atomically only when called from softirq context; from process context it uses the same path as `ieee80211_rx_ni()`.
|
||||
|
||||
**Predicted delta:** Reduces per-frame softirq overhead. Hard to isolate independently of item 1, but combined the two deliver the < 10 % CPU-in-lock target.
|
||||
|
||||
**Effort:** Small (once item 1 is done — the batch list naturally exists at the burst boundary).
|
||||
|
||||
**Risks:** Must hold `rcu_read_lock()` at call site; `skb->cb` (`IEEE80211_SKB_RXCB`) must be filled before the call, as today. The `early_data` path at `txrx.c:1942` uses `skb_queue_tail` into a per-link queue before calling `ieee80211_rx_irqsafe` — that path must be excluded from batch collection.
|
||||
|
||||
---
|
||||
|
||||
## Item 3 — Per-frame `queue_work(sdio_wq, &tx_work)` in the TX send path
|
||||
|
||||
**File:line:** `bes2600_sdio.c:1236` (inside `bes2600_sdio_pipe_send()`).
|
||||
|
||||
**Current shape:** Every call to `bes2600_sdio_pipe_send()` appends one descriptor to `tx_bufferlist` and immediately calls `queue_work(sdio_wq, &tx_work)`. `sdio_tx_work` then drains the list with scatterlist batching (up to `BES_SDIO_TX_MULTIPLE_NUM=16` frames per SDIO transfer). At low rates the workqueue's pending-but-not-started dedup means only one dispatch fires; at high TX rates — especially after `atomic_add(1, &hw_priv->bh_tx)` in `bh.c` reschedules TX — successive `pipe_send` calls each hit `queue_work` before the previous fires, multiplying dispatches.
|
||||
|
||||
**Proposed shape:** Stage all frames into `tx_bufferlist` in the BH TX loop, then flush `sdio_tx_work` synchronously (call the work function body directly) before returning to the wait-event. The TX mirror of item 1.
|
||||
|
||||
**Predicted delta:** Removes redundant TX-side `queue_work` calls. Lower priority than RX side given current TX rate (13 `wsm_cmd_send`/sec is host→chip control plane; data-plane TX is also limited by firmware buffer count `numInpChBufs`), but it does remove one source of the 5,643/sec workqueue count.
|
||||
|
||||
**Effort:** Small.
|
||||
|
||||
**Risks:** `sdio_tx_work` calls `sdio_claim_host`/`sdio_release_host` internally. Running directly from BH context requires confirming no deadlock with the SDIO bus claim that `sdio_rx_work` (now merged per item 1) holds. The TX flush must happen after the RX burst, matching the existing BH loop structure (`rx:` → `tx:` ordering in `bh.c:1444`).
|
||||
|
||||
---
|
||||
|
||||
## Item 4 — `ba_lock` per-frame acquisition in the RX path
|
||||
|
||||
**File:line:** `txrx.c:998–1005` (`bes2600_rx_h_ba_stat()`); `txrx.c:1159–1164` and `txrx.c:1682–1698` (`tsm_lock`, `CONFIG_BES2600_TESTMODE`-gated).
|
||||
|
||||
**Current shape:** `bes2600_rx_cb()` calls `bes2600_rx_h_ba_stat()` for every non-multicast data frame. That function acquires `ba_lock` under BH (`spin_lock_bh`) to increment `ba_acc_rx` and `ba_cnt_rx`, then sets a timer. At 611 frames/sec that's 611 lock acquisitions/sec on `ba_lock` alone.
|
||||
|
||||
**Proposed shape:** Replace per-frame `ba_lock` with a per-cpu counter (or `atomic64_t`) for `ba_acc_rx` and `ba_cnt_rx`. The timer arm (`mod_timer`) is the actual reason for the lock — a `READ_ONCE`/`cmpxchg` on a flag to detect first-frame-in-interval is sufficient.
|
||||
|
||||
**Predicted delta:** Removes 611 lock acquisitions/sec from the RX hot path. Not the dominant cost but the next bottleneck after items 1–2 land.
|
||||
|
||||
**Effort:** Small.
|
||||
|
||||
**Risks:** `ba_lock` also serialises TX-side block-ack accounting (`txrx.c:1632`). The per-cpu approach requires a fold step in the timer callback — cheap.
|
||||
|
||||
---
|
||||
|
||||
## Item 5 — Skip `ps_state_lock` acquisitions when PSM is known-disabled
|
||||
|
||||
**File:line:** `bes2600.h:320` (decl); `txrx.c:1340–1365` (`vif_lock`); `txrx.c:1415–1426` (`ps_state_lock`); `txrx.c:1942–1948` (RX-side `ps_state_lock`).
|
||||
|
||||
**Current shape:** `ps_state_lock` is taken on every TX frame if powersave is active. Per memory `reference_bes2600_firmware_no_psm.md`, **PSM is non-functional on this firmware** — c7 already self-detects this and latches `pm_unsupported = true`. The `ps_state_lock` guards in the RX callback and TX path are therefore taking dead overhead.
|
||||
|
||||
**Proposed shape:** Add a `READ_ONCE()` check on `powersave_enabled` before taking `ps_state_lock`; if false, skip the lock and the PSM state update entirely. Since c7's `pm_unsupported` latches, this is safe.
|
||||
|
||||
**Predicted delta:** Small absolute gain at current TX rate, but prevents fast path from regressing as throughput improves.
|
||||
|
||||
**Effort:** Small.
|
||||
|
||||
**Risks:** `powersave_enabled` is written from process context (`bh.c:403`). `READ_ONCE` without lock is safe — at worst one spurious PSM notification, not a state corruption.
|
||||
|
||||
---
|
||||
|
||||
## Item 6 — Firmware block-read size cap (`EFFECTIVE_BUF_SIZE = 8190 bytes`)
|
||||
|
||||
**File:line:** `bh.c:33–36` (defines); `bes2600_sdio.c:721–783` (`bes2600_sdio_extract_packets()`); `hwio.h:294` (`BES_SDIO_RX_MULTIPLE_NUM=16`).
|
||||
|
||||
**Current shape:** `BES_SDIO_RX_MULTIPLE_NUM=16` and `BES_SDIO_OPTIMIZED_LEN` both defined (`Makefile:90,92`). The RX burst reads `PACKET_TOTAL_LEN(ctrl_reg)` bytes in a single CMD53; each sub-packet bounded by `EFFECTIVE_BUF_SIZE = (0x1000-4)*2 - 2 = 8190` bytes. At 611 frames/sec ÷ 267 BH wakeups/sec ≈ **2.3 frames per wakeup** — well under the 16-frame limit. **Not the bottleneck today.**
|
||||
|
||||
**Proposed shape:** No change needed now. Re-evaluate after items 1–3 land if throughput rises past ~3 MB/s. Verify ctrl_reg `PACKET_TOTAL_LEN` field values during high load — requires firmware-trace observation we don't currently have.
|
||||
|
||||
**Effort:** N/A.
|
||||
|
||||
**Risks:** Increasing beyond 16 requires a larger DMA allocation (currently `1632 × 16 = 26 KB`). Cortex-M4F firmware side is opaque.
|
||||
|
||||
---
|
||||
|
||||
## Item 7 — Duplicate workqueues (`hw_priv->workqueue` vs `hw_priv->bh_workqueue` vs `sbus_priv->sdio_wq`)
|
||||
|
||||
**File:line:** `bes2600.h:323` (`workqueue`); `bes2600.h:385` (`bh_workqueue`); `bes2600_sdio.c:63` (`sdio_wq`). `txrx.c` has 10 `queue_work(hw_priv->workqueue, ...)` calls for control-plane work.
|
||||
|
||||
**Current shape:** Three distinct workqueues. The 5,643 `workqueue_execute_start`/sec are dominated by `sdio_wq` items, not `workqueue`. `workqueue` items are control-plane events at rates well below the data-plane.
|
||||
|
||||
**Proposed shape:** After item 1 (merging `sdio_rx_work` into BH), `sdio_wq` only carries `tx_work`. After item 3 (synchronous TX flush from BH), `sdio_wq` is idle during normal data-plane and could be replaced with `system_highpri_wq`.
|
||||
|
||||
**Effort:** Small (follow-on to items 1 and 3).
|
||||
|
||||
**Risks:** None if items 1 and 3 land first.
|
||||
|
||||
---
|
||||
|
||||
## Item 8 — `BH_RX_CONT_LIMIT=3` cap on RX burst per BH wakeup
|
||||
|
||||
**File:line:** `bh.c:1380–1405` (timeout detection); `bh.c:1330` (`BH_RX_CONT_LIMIT=3`); `bh.c:1331` (`BH_TX_CONT_LIMIT=20`).
|
||||
|
||||
**Current shape:** BH loop limits RX burst to 3 consecutive iterations before breaking back to wait-event. At 611 frames/sec ÷ 267 wakeups/sec ≈ 2.3 frames per wakeup → not the bottleneck today. **After items 1–3 land**, per-burst frame rate will rise and `BH_RX_CONT_LIMIT=3` becomes the ceiling.
|
||||
|
||||
**Proposed shape:** Raise to 16 (matching `BES_SDIO_RX_MULTIPLE_NUM`) after items 1–3 are deployed and re-measured.
|
||||
|
||||
**Effort:** Trivial (constant change), but must wait for Phase 7 measurements post 1–3.
|
||||
|
||||
**Risks:** Too high a limit under firmware anomaly (corrupted ctrl_reg) can spin BH long enough to miss beacon ACK deadline. Bound to `BES_SDIO_RX_MULTIPLE_NUM` as safe ceiling.
|
||||
|
||||
---
|
||||
|
||||
## Ranking summary
|
||||
|
||||
| Rank | Item | Predicted gain | Effort |
|
||||
|------|------|----------------|--------|
|
||||
| 1 | Collapse `sdio_rx_work` relay into BH loop | ~5x workqueue dispatch reduction | Medium |
|
||||
| 2 | Batch deliver via `ieee80211_rx_list()` | Removes per-frame softirq | Small |
|
||||
| 3 | Synchronous TX flush from BH | Removes TX-side dispatch noise | Small |
|
||||
| 4 | Replace `ba_lock` per-frame with atomic/per-cpu | Removes 611 lock/sec from RX hot path | Small |
|
||||
| 5 | Skip `ps_state_lock` when PSM-known-disabled | Removes dead overhead | Small |
|
||||
| 6 | Raise `BH_RX_CONT_LIMIT` after 1–3 land | Unlocks residual throughput | Trivial |
|
||||
| 7 | Consolidate workqueues post-items 1&3 | Cleanup | Small |
|
||||
| 8 | Firmware block-read size | Not bottleneck at current rates | N/A |
|
||||
|
||||
**Items 1 + 2 together are the structural answer to the measurement**: ~9 workqueue events per delivered frame collapse to ~1, and the per-frame softirq cost disappears. Items 3–5 clean up the next layer. The beacon-loss cascade at 9 minutes is almost certainly starvation of the BH wait-event under the per-frame workqueue storm — item 1 removes the mechanism that makes the cascade possible.
|
||||
|
||||
---
|
||||
|
||||
## Next campaign step
|
||||
|
||||
A Phase 4 plan locking item 1 (and possibly item 2) follows in a separate PR. The remaining items go on the campaign backlog as follow-on patches once the Phase 7 verification of item-1-or-1-plus-2 confirms the predicted delta.
|
||||
@@ -0,0 +1,108 @@
|
||||
# Bug #5 RX-degradation campaign — Phase 0
|
||||
|
||||
**Date:** 2026-05-07
|
||||
**Module under test:** v3 + F (`bes2600.ko` srcversion `371C6606B73AF19299228CA`)
|
||||
**Hardware:** ohm (PineTab2, RK3566 + BES2600 SDIO), wired enu1 fallback path live.
|
||||
|
||||
---
|
||||
|
||||
## Research question (locked)
|
||||
|
||||
> **Why does the bes2600 RX path collapse from ~2 MB/s sustained @ fresh-chip uptime to ~180 B/s @ ~28-min uptime, with periodic `wsm_generic_confirm failed for request 0x0007` + `ieee80211 phy0: [SCAN] Scan failed (-22)` every 300 s in the intervening window?**
|
||||
|
||||
Reproduces on Patch B, Patch F, and Patch C v3 alike — independent of the relay/race issues v3 addressed. Side-effect that was masked by the throughput floor while v2's race was the dominant variable.
|
||||
|
||||
## Predecessor data (reference, not anchor)
|
||||
|
||||
| source | observation |
|
||||
|---|---|
|
||||
| Patch C v3 N=3 (uptime 200/391/582 s) | mean 2.352 MB/s @ 4 MB/s sender |
|
||||
| v3 single rep at uptime ~28 min (rep 2 of 2026-05-07 22:23) | 180 KB / 5 min = 600 B/s, sender saw "Connection reset by peer" |
|
||||
| v3 single rep at uptime ~47 min (N=3 first attempt 22:42) | 55 KB / 5 min = 180 B/s, sender timed out (exit 124) |
|
||||
| dmesg pattern observed at 47-min uptime | scan failures every 301-302 s starting at uptime 778 s (~13 min) |
|
||||
|
||||
The shape: **fresh chip → linear data flow at ~2 MB/s sustained → sometime around 13 min uptime, NetworkManager-triggered scans start failing → sometime around 28 min uptime, data throughput collapses to <1 KB/s while link still shows associated.**
|
||||
|
||||
Predecessor data is reference. Phase 0 will re-anchor at N=1 long-trace + 5 in-window stress probes; if the pattern doesn't reproduce, that's the campaign result.
|
||||
|
||||
## Mechanism candidates (Phase 4 will discriminate)
|
||||
|
||||
1. **Firmware-side resource exhaustion.** Per-scan or per-WSM-event accumulation in chip-side state. Scan-failed -22 (EINVAL) suggests firmware refusing the request — possibly out of scan handles, scan-buffer slots, or some other limit.
|
||||
2. **NetworkManager scan-fail recovery loop.** Each failed scan triggers NM retry. If retry overhead dominates the bh thread, data path starves. Verifiable by suppressing NM scans.
|
||||
3. **AP-side rate limiting.** Newton (AVM) AP could be applying QoS / fairness / probation after sustained 4 MB/s burst. Verifiable by Fritz!Box log access (Markus has it) or by switching to a different AP.
|
||||
4. **PSM state machine deadlock.** c7's `pm_unsupported` self-detect was supposed to handle this, but the latch state could become stale if a real PM_IND arrives mid-operation. Verifiable by `chip_pm_state` debugfs read at degradation onset.
|
||||
5. **SDIO bus clock degradation / mmc retune.** SDIO retune with `retune_protected` flag interacts with bes2600's data path. Verifiable by ftrace `mmc/mmc_request_*` event correlation with throughput drop.
|
||||
6. **Power-management busy-event accumulation.** `bes2600_pwr_set_busy_event` counters might leak — busy events not cleared lock the chip awake (no PSM) but also exhaust event capacity. Verifiable by `bes2600_pwr_busy_event_record` dump.
|
||||
|
||||
## Phase 0 measurement protocol (rig armed 2026-05-07 23:18:58 CEST, T0=1778188738)
|
||||
|
||||
Capturing for 35 minutes from fresh boot. All capture lives in `/root/bes2600-samples/run-20260507-bug5-degradation-rig/` on ohm.
|
||||
|
||||
### Always-on streams
|
||||
|
||||
| stream | tool | output |
|
||||
|---|---|---|
|
||||
| ftrace events | per-event `enable=1` | `trace.log` (via `trace_pipe`) |
|
||||
| cfg80211 events | `iw event -t -f` | `iw-event.log` |
|
||||
| kernel printks | `dmesg -wT` | `dmesg.log` |
|
||||
| netdev counters | per-30s shell loop | `snap.log` |
|
||||
|
||||
### ftrace event set
|
||||
|
||||
- `workqueue/workqueue_execute_start` — work dispatches
|
||||
- `workqueue/workqueue_queue_work` — work submissions
|
||||
- `mac80211/api_beacon_loss` — driver beacon-loss events
|
||||
- `mac80211/api_connection_loss` — driver-side conn-loss
|
||||
- `mac80211/api_disconnect` — driver-side disconnect
|
||||
- `mac80211/drv_hw_scan` — mac80211 → driver scan dispatch
|
||||
- `mac80211/drv_set_key` — key state changes
|
||||
- `cfg80211/rdev_assoc` — assoc requests
|
||||
- `cfg80211/rdev_deauth` — deauth requests
|
||||
- `cfg80211/rdev_disassoc` — disassoc requests
|
||||
- `cfg80211/cfg80211_assoc_comeback` — AP-side assoc-busy throttling
|
||||
- `cfg80211/cfg80211_send_auth_timeout` — auth timeouts
|
||||
- `cfg80211/cfg80211_scan_done` — scan completions
|
||||
- `power/suspend_resume` — PM transitions
|
||||
- `mmc/mmc_request_start` / `mmc_request_done` — bus-level transactions
|
||||
|
||||
### Scheduled stress probes
|
||||
|
||||
Sender on boltzmann (`/tmp/bug5-probe-loop.sh`) fires `pv -L 4m | nc ohm 12345` for 30 s at T+5/10/15/20/25 min. Each probe brackets uptime, RX-bytes pre, RX-bytes post, elapsed. Throughput-vs-uptime curve falls out of the snap.log + probe boundaries.
|
||||
|
||||
Probe markers logged via `logger -t bes2600-bug5 PROBE_N_START/END` so they appear in dmesg.log timeline.
|
||||
|
||||
## Anti-theatre receipts (must tick before claiming Phase 0 done)
|
||||
|
||||
- [ ] In-session baseline: long-capture across degradation window, N=1 for now; re-run if anomalous
|
||||
- [ ] ftrace events actually firing (verify by tail of trace.log mid-capture)
|
||||
- [ ] dmesg captures the scan-failure pattern timestamp (expected ~uptime 778 s)
|
||||
- [ ] Probes actually transferred data at fresh chip (T+5 should be > 1 MB/s)
|
||||
- [ ] At least one probe in-window after scan-failure onset (expected: T+15 or T+20)
|
||||
- [ ] Snap.log shows monotonic counter behaviour (no rx_bytes going backwards)
|
||||
|
||||
## Phase 1 hypothesis (provisional, refine after Phase 3 data)
|
||||
|
||||
Metric candidate: **probe throughput as function of uptime, with state-transition markers (first `wsm_generic_confirm 0x0007 failed`, first `[SCAN] Scan failed (-22)`, first NetworkManager-deauth-and-reassociate)**.
|
||||
|
||||
Discriminator question: does throughput collapse abruptly at the first scan failure, or gradually over a window? Abrupt = single-event causation; gradual = accumulator.
|
||||
|
||||
## Phase 4 candidates (post-Phase-3)
|
||||
|
||||
Depending on which mechanism (1-6) Phase 3 surfaces:
|
||||
- (1) firmware resource exhaustion: report to upstream; possibly disable NetworkManager scans pending firmware fix.
|
||||
- (2) NM scan-fail loop: configure `wpa_supplicant` to skip scans; or add scan-failure handling in driver to dampen retry cascade.
|
||||
- (3) AP-side: switch APs for testing; report to AVM if reproducible.
|
||||
- (4) PSM deadlock: extend c7 latch with timeout-or-progress recovery.
|
||||
- (5) SDIO retune: ftrace correlation guides the lock-ordering fix.
|
||||
- (6) PWR busy-event leak: audit set/clear pairs; add a warning-when-stale.
|
||||
|
||||
## Out-of-scope
|
||||
|
||||
- Patch C v3 closure (PR #5 merged, Phase 7 done).
|
||||
- Patch C2 (`ieee80211_rx_list` batch) — gated on Task #19 kerneldoc.
|
||||
- Patch D / E independent.
|
||||
- Reproduction at higher rates (8 MB/s ramp) — defer to Phase 4 once mechanism identified.
|
||||
|
||||
---
|
||||
|
||||
*Phase 0 plan written 2026-05-07 23:21 CEST by Claude (noether), at the close of Patch C v3 Phase 7. Rig armed; long capture in flight; probes scheduled at T+5/10/15/20/25 min. Post-capture analysis will populate Phase 3 results before Phase 4 plan branches off.*
|
||||
@@ -82,6 +82,91 @@ without board power-cycle").
|
||||
**Status**: task c3 (indirectly, via bes_chardev removal which currently
|
||||
gates the signal/nosignal mode switch path).
|
||||
|
||||
## Architect review — now BUG-#5-blocking (was backlog)
|
||||
|
||||
The Phase 0 perf trace for Bug #5 first exposed a "when in doubt, add a
|
||||
lock" pattern (~20 % CPU in `_raw_spin_unlock_irqrestore`). The
|
||||
follow-up ftrace measurement (2026-05-07 17:00) refined the root cause
|
||||
to an architectural problem: **the bes2600 driver dispatches every
|
||||
SDIO transaction through the kernel workqueue**. Numbers from a 3-min
|
||||
4 MB/s ohm capture (post-reboot, srcversion `1B3B3ED0`):
|
||||
|
||||
```
|
||||
wsm_cmd_send: 13/sec (host-to-chip command rate, surprisingly low)
|
||||
bes2600_rx_cb: 611/sec
|
||||
bes2600_bh_wakeup: 267/sec
|
||||
lock contention_begin: 50/sec
|
||||
workqueue_execute_start: 5,643/sec ← DOMINATES; matches the mmc
|
||||
transaction rate from earlier perf
|
||||
```
|
||||
|
||||
5.6 k workqueue dispatches per second is the throughput floor — not a
|
||||
specific lock, not WSM-command rate, not decrypt-state. A surgical fix
|
||||
to any single function won't move the floor; the architecture needs
|
||||
to be restructured to amortise SDIO transactions across fewer work-
|
||||
items (or move SDIO RX out of the workqueue entirely).
|
||||
|
||||
This is where the **Claude Sonnet architect review** belongs: a
|
||||
top-to-bottom assessment of `~/src/besser/bes2600-dkms-mobian/bes2600/`
|
||||
focused on:
|
||||
|
||||
- the workqueue dispatch shape (most actionable)
|
||||
- needless lock proliferation (the original signal)
|
||||
- BH / RX scheduling boundaries
|
||||
- error-handling coverage and dead-code from the cw1200 ancestor
|
||||
- API contract violations relative to mainline mac80211
|
||||
|
||||
Output: ranked list of restructuring targets, with predicted-delta
|
||||
estimates against the Phase 1 metric (≥ 2 MB/s sustained @ 4 MB/s cap,
|
||||
< 10 % CPU in lock-cycling, no link cascade in 30 min).
|
||||
|
||||
**Status**: now blocking on Bug #5 (was independent track). Surgical
|
||||
patches B5-1, B5-2, B5-3 from the original Phase 4 candidate list are
|
||||
all DEFERRED until the architect review's restructuring map is in.
|
||||
|
||||
## Bug #5 — RX path degrades under attempted-throughput pressure
|
||||
|
||||
**Suspect file**: bes2600 RX path (`txrx.c bes2600_rx_cb`, `bh.c bes2600_bh_work`,
|
||||
SDIO RX scheduling) — pinpoint pending.
|
||||
|
||||
**Symptom (observed 2026-05-07 13:43, srcversion `1B3B3ED0` = c-stack +
|
||||
Patch A + Patch B, ohm @ -57 dBm 2.4GHz ch11 5b:32, idle save for the
|
||||
netcat load):**
|
||||
|
||||
```
|
||||
sender cap 1 MB/s → ohm receives 1015 KB/s, signal -57 dBm, RX MCS 4
|
||||
sender cap 4 MB/s → ohm receives 563 KB/s, signal -67 dBm, RX MCS 3
|
||||
(Send-Q on boltzmann backed up to 1.16 MB)
|
||||
```
|
||||
|
||||
Pushing the sender-side cap from 1 MB/s to 4 MB/s **decreased** observed
|
||||
throughput at the receiver and degraded the link metrics. Signal dropped
|
||||
~10 dB and the chip downshifted MCS, suggesting the chip can't sustain
|
||||
the higher RX rate even with the link physically capable of more (link
|
||||
bitrate 65 Mb/s = ~8 MB/s theoretical).
|
||||
|
||||
**Hypothesis (Markus, 2026-05-07): driver/firmware locks itself to death
|
||||
under busy reads** — possibly a busy-wait loop or lock contention on the
|
||||
RX SDIO path that prevents draining at line rate. Plausible reason it
|
||||
didn't surface for the c-stack tasks: those operated at typical
|
||||
browse-rate traffic, well below the saturation threshold this bug needs
|
||||
to fire.
|
||||
|
||||
**May explain**: original Phase-0 observation that **YouTube DASH chunks
|
||||
drop ~10 frames per chunk fetch** on hardware-decoder playback. A chunk
|
||||
fetch is a brief burst at near-link-rate; if the driver throttles itself
|
||||
down during high-RX, the player buffer underruns for the duration of
|
||||
the fetch.
|
||||
|
||||
**How to drill (when prioritized)**:
|
||||
- Capture trace_pipe with `mmc:*` and `sdio*` events enabled during a
|
||||
controlled rate-ramp (e.g., pv -L 500K, 1M, 2M, 4M each for 60 s).
|
||||
- Watch `/proc/sys/kernel/sched_*` and the `bes2600_bh_work` kworker for
|
||||
CPU saturation.
|
||||
- `perf top -p $(pgrep -f bes_sdio)` during 4 MB/s load.
|
||||
|
||||
**Status**: backlog. No patch yet.
|
||||
|
||||
## Bug #4 — scan_complete_cb constant loop
|
||||
|
||||
**File**: `scan.c:883-909` — `bes2600_scan_complete_cb()`.
|
||||
|
||||
@@ -0,0 +1,190 @@
|
||||
# BES2600 WiFi structural analysis and code critique
|
||||
|
||||
**Author:** Claude (noether) — second-opinion as Opus 4.7 against Sonnet 4.6's review of 2026-05-07
|
||||
**Scope:** the WiFi half of the BES2600 driver as it lives in `bes2600-dkms-mobian/bes2600/` on top of the `cleanups` branch (srcversion `1B3B3ED0…`, c-stack + Patch A + Patch B deployed).
|
||||
**Reading frame:** Bug #5 prompted Sonnet's review; this writeup is independent — same source tree, different model, different priors. Where I concur I tighten; where I disagree I say so.
|
||||
|
||||
---
|
||||
|
||||
## 1. Top-line
|
||||
|
||||
The BES2600 WiFi driver is **not a BES2600 driver**. It is a CW12xx driver wearing a BES2600 nameplate. That sentence is not rhetoric — it is the design fact that explains every other smell I will list below.
|
||||
|
||||
- 30+ live references to `CW12XX_MAX_VIFS` across 9 files.
|
||||
- `cw12xx_hwpriv_to_vifpriv()` / `cw12xx_get_vif_from_ieee80211()` are the active vif accessors.
|
||||
- `is_hardware_cw1250(hw_priv) || is_hardware_cw1260(hw_priv)` is a runtime branch in `ap.c:1892` — the chip is BES2600, neither check ever matches, the branch is dead on this hardware but compiled in.
|
||||
- `CW1200_MAX_SW_RETRY_CNT` gates the active retry-decision logic in `bh.c:1269` (inside `KEY_FRAME_SW_RETRY`).
|
||||
- The header opens with "Based on the mac80211 Prism54 code, which is Copyright (c) 2006, Michael Wu" → **prism54 → islsm → ST-E CW1200 → CW1260 → CW12xx → BES2600**: at least five generations of vendor-SDK descent, with each generation preserving its predecessor as #if-0 blocks rather than removing it.
|
||||
|
||||
This is the Phase 6 "transcription trap" from `CLAUDE.md`, frozen into the codebase: every generation copied behaviour rather than re-derive against the API contract. The result is a driver that *works*, but whose structural choices are decisions made for a 2010 ST-Ericsson chip, not a 2022 Bestechnic one.
|
||||
|
||||
The downstream consequence — and the thing that actually pinches us in Bug #5 — is that the **hot path was designed for cw1200's IRQ-driven SPI bus, not for SDIO with multi-block coalescing**. Items 1 + 2 of Sonnet's review are the right surgical fix. The deep fix is bigger than the budget of any one campaign.
|
||||
|
||||
## 2. Concurrence with Sonnet — refined
|
||||
|
||||
### 2.1 RX relay (Sonnet item 1) — concur, refine
|
||||
|
||||
The flow on this build (`-DBES2600_RX_IN_BH` in Makefile, so this is the *real* path):
|
||||
|
||||
```
|
||||
SDIO IRQ
|
||||
→ bes2600_gpio_irq_handler (bes2600_sdio.c:413)
|
||||
→ queue_work(self->sdio_wq, &self->rx_work) (bes2600_sdio.c:416)
|
||||
→ sdio_rx_work runs (bes2600_sdio.c:829)
|
||||
→ bes2600_sdio_lock + memcpy_fromio
|
||||
→ bes2600_sdio_extract_packets (skb_queue_tail to self->rx_queue)
|
||||
→ self->irq_handler(self->irq_priv) (function call, not workqueue)
|
||||
→ atomic_add_return(1, &hw_priv->bh_rx) (bh.c:130)
|
||||
→ wake_up(&hw_priv->bh_wq)
|
||||
→ bh_work (already running, never re-queued):
|
||||
wait_event_interruptible_timeout returns
|
||||
→ bes2600_bh_rx_helper (bh.c:961)
|
||||
→ priv->sbus_ops->pipe_read (skb_dequeue from self->rx_queue)
|
||||
→ wsm_handle_rx (wsm.c)
|
||||
→ bes2600_rx_cb (txrx.c:1642)
|
||||
→ ieee80211_rx_irqsafe(skb) (txrx.c:1947 / 1950)
|
||||
```
|
||||
|
||||
**Where I refine Sonnet:** the "9 workqueue events per delivered RX frame" claim doesn't survive source reading. Per IRQ *batch* there is **one** workqueue dispatch (sdio_wq.rx_work). `bh_work` is registered once, runs as a long-lived work item using `wait_event_interruptible_timeout` to sleep — the wake-up path is a wait-queue, not a workqueue dispatch. `ieee80211_rx_irqsafe` schedules a mac80211 tasklet, not a workqueue. The 5,643 `workqueue_execute_start/sec` ftrace count from Bug #5 is **system-wide**, not bes2600-only — it should not be quoted as "per frame" without per-pid filtering.
|
||||
|
||||
**What is real:** the indirection adds two synchronization points per frame (`skb_queue_tail` + `skb_dequeue`, each `&rx_queue->lock`) plus a cross-CPU wake-up plus a tasklet schedule. That's enough to dominate at 4 MB/s. The collapse is justified — just not by the 9× number.
|
||||
|
||||
### 2.2 ieee80211_rx_irqsafe from process context (Sonnet item 2) — concur, gated on contract verification
|
||||
|
||||
Confirmed: `ieee80211_rx_irqsafe` is the right primitive only when called from hard-IRQ context — it defers to a tasklet. From process context (which is where `bh_work` and `sdio_rx_work` both live), it adds a tasklet hop for nothing.
|
||||
|
||||
`ieee80211_rx_list(hw, sta, &skbs)` is the correct call shape if, and only if, two contract claims hold:
|
||||
1. callable from process context with `local_bh_disable()` wrap (or callable bare),
|
||||
2. SKB list invariants don't impose NAPI-poll semantics we can't honour.
|
||||
|
||||
Sonnet asserted both; I have **not** verified them against `include/net/mac80211.h` kerneldoc on a 6.19-class kernel. **Task #19 blocks Patch C on that verification.** Until it lands, treat the API claim as unconfirmed — this is exactly the Phase 6 contract-citation rule, and skipping it would be the same trap the older driver fell into.
|
||||
|
||||
### 2.3 ba_lock per-frame (Sonnet item 4) — concur
|
||||
|
||||
`txrx.c:998-1005` (TX path) and `txrx.c:1632-1640` (RX path): `spin_lock_bh(&hw_priv->ba_lock)` to bump 4 ints (`ba_acc`, `ba_cnt`, `ba_acc_rx`, `ba_cnt_rx`) and conditionally `mod_timer(&hw_priv->ba_timer, …)`. The TODO comment in `bes2600.h:359-365` literally says *"TODO: Same as above"* on every field — the original author flagged it as deferred work, then shipped.
|
||||
|
||||
Replace with `atomic_t` for the four counters and `cmpxchg`-guarded `mod_timer` for the arm-once invariant. Patch D.
|
||||
|
||||
### 2.4 ps_state_lock when pm_unsupported (Sonnet item 5) — concur
|
||||
|
||||
`txrx.c:1942-1948`: per-RX-frame `spin_lock_bh(&priv->ps_state_lock)` on the early-data path, protecting a check on `entry->status == BES2600_LINK_SOFT`. The lock exists to coordinate with the AP-side power-save state machine.
|
||||
|
||||
c7's contribution (`pm_unsupported = true`) means we already know this firmware doesn't honour PSM; the LINK_SOFT branch is an AP-mode soft-link state that won't transition under us when PSM is dead. Gate the lock acquisition on `!hw_priv->pm_unsupported`. Patch E.
|
||||
|
||||
(This patch is *narrower* than Sonnet framed it: it only applies when `pm_unsupported` latches on, which is at boot for our firmware. Production reality on this hardware = always; but the patch must remain conditional in case a future firmware fixes PSM and c7 self-clears the flag.)
|
||||
|
||||
## 3. Push-back against Sonnet
|
||||
|
||||
### 3.1 "BES_SDIO_OPTIMIZED_LEN config flag"
|
||||
|
||||
Not a runtime/Kconfig knob on this build. `Makefile:18` hard-codes `ccflags-y += -DBES_SDIO_OPTIMIZED_LEN`. Whether to keep it is a separate question, but Sonnet's recommendation should not have framed it as toggleable.
|
||||
|
||||
### 3.2 "Multiple workqueues are unconditionally bad"
|
||||
|
||||
There are three driver-side workqueues:
|
||||
|
||||
| name | purpose | dispatch shape |
|
||||
|---|---|---|
|
||||
| `bh_workqueue` | hosts the single long-running `bh_work` | one-shot at register, wait-queue driven thereafter |
|
||||
| `sdio_wq` | sdio_rx_work + sdio_tx_work + sdio_scan_work | per-IRQ-batch dispatch |
|
||||
| `hw_priv->workqueue` | scan, AP, PM, multicast-start, link-id, set-tim, … | per-event dispatch (~20 producers) |
|
||||
|
||||
**`bh_workqueue` is fine** — it runs a single work item forever, which is just a kthread-shaped-as-workqueue. The cost is one alloc_workqueue at register and zero ongoing dispatch overhead. Don't kill it.
|
||||
|
||||
**`sdio_wq` is the actual surgical target** — collapsing item 1 means subsuming `sdio_rx_work` into the bh-loop, after which `sdio_wq` only hosts tx_work and scan_work and could be merged with `hw_priv->workqueue` for cleanup. But that merge is cosmetic; do it later or never.
|
||||
|
||||
**`hw_priv->workqueue` shouldn't be touched.** It hosts ~20 unrelated producers; merging it into sdio_wq is the wrong direction (priority inversion risk under coex pressure).
|
||||
|
||||
### 3.3 "BH_RX_CONT_LIMIT=3 is the bottleneck"
|
||||
|
||||
Half-true. The limit caps the burst-RX pass to 3 frames before yielding to TX work. Raising it past 3 only helps if RX has steady backlog, which under our 4 MB/s ramp it does. But there's also `BH_TX_CONT_LIMIT=20` paired with it — TX gets 20-frame bursts, RX gets 3. The asymmetry is from a previous campaign that found TX-starvation, and **flipping it without re-running that campaign is a regression risk**. Treat the constant as a phase-7-knob, not a one-liner.
|
||||
|
||||
## 4. New findings Sonnet did not surface
|
||||
|
||||
### 4.1 `bh.c` carries ~700 lines of `#if 0` dead code
|
||||
|
||||
`bh.c:196-877` is the cw1200 ancestor `bes2600_bh()` preserved verbatim alongside the active impl at `bh.c:1332+`. Same function name, same `goto rx:` / `goto tx:` labels, same loop variables. The fossil block contains a typo (`if ((i = (CW12XX_MAX_VIFS - 1)) || !priv)` at lines 438 and 562 — single `=` is assignment-not-compare; live code at `ap.c:696` uses `==` correctly) which would be a real bug if compiled. **It is not compiled** — `#if 0` saves us — but this is the maintenance hazard you discover *first* when reading the file in a hurry.
|
||||
|
||||
Action: kill the `#if 0` block. Standalone hygiene patch, not on the Bug-#5 critical path.
|
||||
|
||||
### 4.2 Allwinner-specific code in the SDIO bus path
|
||||
|
||||
`bes2600_sdio.c:475` calls `sw_mci_check_r1_ready(self->func->card->host, 1000)` from inside the IRQ-setup error path. This is the Allwinner mmc driver's R1-ready helper — not portable to RK3566's `dw_mmc-rockchip` host driver.
|
||||
|
||||
The call is reachable only on `set_func` cleanup (a comparatively rare error path), but it is a build-time portability hazard. Most likely a stub macro on non-Allwinner builds; verify on ohm or wrap behind `#ifdef CONFIG_MMC_SUNXI`.
|
||||
|
||||
### 4.3 `asm volatile ("nop")` placeholder in the live BH loop
|
||||
|
||||
`bh.c:1518` is where IRQ re-enable used to be (`__bes2600_irq_enable(1)` is commented out two lines above). The author left a literal nop instruction "asm volatile" instead of removing the dead block. Either re-enable IRQs (if the code was deleted prematurely) or remove the nop (if IRQs are intentionally always-on). This is non-cosmetic — it indicates an unresolved IRQ-handling decision.
|
||||
|
||||
### 4.4 `BUG_ON` in the steady-state hot path
|
||||
|
||||
`bh.c:1488`: `BUG_ON(hw_priv->hw_bufs_used > hw_priv->wsm_caps.numInpChBufs)` runs *every* BH iteration. Tripping it locks up the kernel during normal operation — by definition the wrong response to a bookkeeping bug. Should be `WARN_ON_ONCE` + bail-out. (Same critique applies to several other `BUG_ON`s in `bh.c` — search the active `#else` block.)
|
||||
|
||||
### 4.5 Build-system is a vendor SDK, not a kernel-style driver
|
||||
|
||||
`Makefile:1-50` defaults: `CONFIG_BES2600_TESTMODE ?= y`, `WIFI_BT_COEXIST_EPTA_ENABLE ?= y`, `BES2600_INTEGRATED_MODULE_V1/V2/V3` for *xiaomi R329 wifi module*, *sicun QM215 wifi module*, *bes evb*. 86 `#ifdef CONFIG_BES2600_TESTMODE` sites — testmode is essentially compiled-in dead code in non-test builds.
|
||||
|
||||
The driver was built by Bestechnic to ship per-customer board variants from one source tree. Upstreaming will require ripping that whole apparatus out, replacing with `Kconfig` toggles and platform-data lookups. This is **not** a Bug-#5 dependency, but it is a debt that pollutes every other patch — diff hunks land in `#ifdef`-walled territory and conflict on rebases for unrelated reasons.
|
||||
|
||||
### 4.6 8 `EXPORT_SYMBOL` declarations from a single-binary module
|
||||
|
||||
The driver exports `bes2600_irq_handler`, `bes2600_bh_wakeup`, `bes2600_bh_suspend`, `bes2600_bh_resume`, etc. — for whom? The only known consumer is `bes2600_btuart`, the BT sibling module. Either the BT module needs a coherent shared-driver API surface (refactor target), or these exports should become `static`. Random sibling-module coupling via global symbols is a known kernel anti-pattern.
|
||||
|
||||
### 4.7 No `__must_check` on functions that obviously return errors
|
||||
|
||||
Almost every `bes2600_data_read` / `bes2600_data_write` / `bes2600_reg_read*` call site is wrapped in `WARN_ON()`. That's defensive but not enforced. A single missed return-check (compiler will not warn) is a silent SDIO-path bug. Annotation cost is one keyword per declaration; benefit is a class of bugs caught at compile time.
|
||||
|
||||
### 4.8 `rx_queue` is per-sbus_priv, not per-vif
|
||||
|
||||
Multi-vif RX serializes through one `skb_queue` on the sbus side (`bes2600_sdio.c:867` queues to `self->rx_queue`, only dequeued by the single bh thread). For STA-only operation this doesn't matter; for STA+AP concurrent or P2P-multivif it's a structural ceiling on aggregate RX throughput. Out of scope for Bug #5 but worth recording — Markus's "P2P_MULTIVIF=y" Makefile default makes this potentially observable.
|
||||
|
||||
## 5. Ordering recommendation for the cleanup roadmap
|
||||
|
||||
Given (a) the current Bug-#5 budget, (b) Phase-7 stress-ramp cost per patch, (c) the constraint that the cleanups branch must rebase cleanly on Mobian's `mobian` for re-MR:
|
||||
|
||||
| order | patch | scope | phase-7 cost | risk |
|
||||
|---|---|---|---|---|
|
||||
| 1 | **Patch C (items 1+2 wrapped)** | hot path: collapse sdio_rx_work into bh, batch deliver via ieee80211_rx_list | full ramp 1→4→8 MB/s | high — touches RX hot path |
|
||||
| 2 | **Patch D (item 4)** | ba_lock → atomics + cmpxchg-guarded mod_timer | minimal — lock-stat delta + 5min @ 4MB/s smoke | low |
|
||||
| 3 | **Patch E (item 5)** | ps_state_lock skip when pm_unsupported | minimal — same as D | low (gated on c7's existing latch) |
|
||||
| ∞ | bh.c #if 0 graveyard removal | pure delete | none — recompile + smoke | zero |
|
||||
| ∞ | CW12XX → BES2600 rename | mass rename | none — but every open patch conflicts | high churn cost, zero behaviour change |
|
||||
| **NOT** | Allwinner abstraction layer | wrap sw_mci_check_r1_ready | n/a | scope-creep; do only if RK3566 fails on it |
|
||||
| **NOT** | Vendor-SDK Makefile rewrite | Kconfigify | n/a | upstream-prep work, not Bug-#5 |
|
||||
| **NOT** | bh_workqueue / sdio_wq merge | structural | n/a | speculation, no measured win |
|
||||
|
||||
Patch C is high-risk; merging items 1 and 2 into one patch is the user's call (made: "wrap them together") but should **be reviewed Phase-5 before Phase-6 implementation lands** — exactly the receipts-checklist that this CLAUDE.md exists to enforce. Splitting Patch C into 1-then-2 is *also* defensible; if Phase 7 finds item 1 regressed something, item 2 in isolation is harder to bisect.
|
||||
|
||||
## 6. Things I would explicitly NOT do
|
||||
|
||||
- **Don't paint the bikeshed on naming.** CW12XX → BES2600 rename is a 30+ file mass-substitute that conflict-spams every open topic branch. It is the right fix *for upstreaming*, not for the cleanups branch.
|
||||
- **Don't refactor the workqueue topology.** Three workqueues is fine. Two workqueues for cosmetic reasons risks priority inversion under coex pressure.
|
||||
- **Don't replace the BH thread architecture.** It works, the wait-queue model is well-suited to the IRQ → drain pattern, and replacing it with NAPI or threaded-IRQ would re-do six years of debugging in a single patch.
|
||||
- **Don't strip the `#ifdef CONFIG_BES2600_TESTMODE` blocks** until upstream-prep. They are vendor-SDK debt but harmless dead code.
|
||||
- **Don't wrap the Allwinner helper** unless RK3566 actually trips it. The path is rare-error.
|
||||
|
||||
## 7. What I would tell a fresh reviewer in one paragraph
|
||||
|
||||
> *This driver is genealogically a CW1200 driver (ST-Ericsson, ~2010) with chip-name search-and-replace done halfway. The hot path was designed for SPI with one-frame-per-IRQ; SDIO multi-block coalescing was bolted on with a worker-queue handoff that adds two synchronization points per frame. Bug #5's RX-throughput regression at 4 MB/s is a direct consequence: at low rate the handoff overhead is invisible; at high rate it dominates. Three small patches (Patches C, D, E) reclaim most of the floor without touching the genealogy. The genealogy itself is technical debt for upstreaming, not a Bug-#5 dependency. Don't conflate the two.*
|
||||
|
||||
---
|
||||
|
||||
## 8. Disagreements summary
|
||||
|
||||
| Sonnet claim | My finding |
|
||||
|---|---|
|
||||
| "9 workqueue events per delivered RX frame" | overstated; per IRQ batch is 1 workqueue dispatch on this build. The 5,643/sec ftrace count is system-wide, needs per-pid filtering before claiming as bes2600 dispatch rate. |
|
||||
| "BES_SDIO_OPTIMIZED_LEN config flag" | hard-baked in Makefile as `-D…` ccflags, not toggleable |
|
||||
| Item 4 / Item 5 sized as one patch each | concur — separate small patches as Markus directed |
|
||||
| Item 1 + 2 mergeable | concur — directionally; predicated on `ieee80211_rx_list()` contract (Task #19) |
|
||||
|
||||
## 9. Open questions for Markus
|
||||
|
||||
1. **Patch C split-or-merge:** user directive is "wrap together". I'd note that a Phase-7 regression in the merged patch is harder to bisect than two sequential Phase-7 runs. Keeping the directive but recording the bisect-cost as known.
|
||||
2. **`__bes2600_irq_enable(1)` commented out:** is IRQ re-enable intentionally always-on now, or is the `nop` a deletion-in-progress bug? Reading the c-stack history doesn't tell me. Worth a "what was this for" pass before any RX-architecture patch lands.
|
||||
3. **`sw_mci_check_r1_ready` on RK3566:** should we test or just trust the path is rare-error? My read is: trust + `WARN_ON` if it's ever called, then react.
|
||||
|
||||
---
|
||||
|
||||
*Written 2026-05-07. Reviewing as Opus 4.7 against Sonnet 4.6's review of the same source tree. Independent reads of: bh.c, bes2600_sdio.c (sdio_rx_work + pipe_read + IRQ handler), txrx.c (RX delivery sites + ba_lock + ps_state_lock sites), bes2600.h (struct lock topology), Makefile (build-system shape). No simulator runs; this is a static-analysis writeup, the dynamic verification of any claim above belongs in Phase 7 of the corresponding patch.*
|
||||
@@ -0,0 +1,184 @@
|
||||
# Patch C — Phase 4 Plan: collapse sdio_rx_work into BH
|
||||
|
||||
**Author:** Claude (noether)
|
||||
**Status:** Phase 4 — pending Phase 5 second-model review before any Phase 6 code.
|
||||
**Scope:** **item 1 only** (per merged PR #8 inline review: "do it sequentially; we're not on the clock").
|
||||
**Item 2** (batch deliver via `ieee80211_rx_list`) splits to **Patch C2**, gated on Task #19 kerneldoc verification.
|
||||
|
||||
---
|
||||
|
||||
## §0 Substrate — anchored
|
||||
|
||||
Bug #5 anchor (recorded 2026-05-07, see `notes/phase1-bug5-2026-05-07.md`):
|
||||
|
||||
- Sender: netcat-over-WiFi, 4 MB/s cap, 2.4 GHz, AVM AP, single-STA
|
||||
- Receiver: ohm (PineTab2, RK3566 + BES2600WM-SDIO)
|
||||
- N=3 baseline reps: 725 / 663 / 75 KB/s (rep 3 saw link-death at ~9 min)
|
||||
- `perf record -g` during 4MB/s window: `_raw_spin_unlock_irqrestore` ≈ 20% CPU
|
||||
- ftrace lock-instrumentation, system-wide: `workqueue_execute_start` ≈ 5,643/sec
|
||||
- Driver-side count: `wsm_cmd_send` 13/sec — wsm command path is *not* the dispatch source; the contributor is the per-SDIO-transaction relay.
|
||||
|
||||
Root cause traced in PR #7 (Sonnet review) and concurred in PR #8 (Opus review): RX path adds two synchronization points per frame and one wait-queue wake-up per IRQ batch via `sdio_rx_work` → `rx_queue` → `bh_work` indirection.
|
||||
|
||||
## §1 Goal (locked)
|
||||
|
||||
Reduce per-RX-frame overhead enough that observed receive ≥ 1.0 MB/s sustained @ 4 MB/s sender, with `_raw_spin_unlock_irqrestore` < 15 % CPU during the 4 MB/s window. No 30-min cascade to link-death.
|
||||
|
||||
(This is a partial step toward Phase 1's full target of ≥ 2 MB/s sustained @ 4 MB/s with < 10 % lock CPU. The full target is jointly addressed by Patch C + Patch C2; Patch C alone should *cross half the gap*.)
|
||||
|
||||
## §2 Situation
|
||||
|
||||
- `bes2600.ko` srcversion `1B3B3ED0…` deployed on ohm (c-stack + Patch A + Patch B).
|
||||
- `cleanups` branch on `marfrit/bes2600-dkms` is the current source-of-truth.
|
||||
- Build sandbox `/var/tmp/c6-sandbox/` on ohm, native `make -j4`.
|
||||
- `BES2600_RX_IN_BH` is **defined** in Makefile — `bes2600_bh_rx_helper` is the active RX consumer.
|
||||
- ohm reachable. Markus pushes the reboot button; never me.
|
||||
- Test rig under `/root/bes2600-samples/` — `rep-trace.sh` per-rep capture script.
|
||||
|
||||
## §3 Baseline measurements
|
||||
|
||||
Reused from Bug #5 Phase 0 (above). No re-anchor needed for Patch C — same regime.
|
||||
|
||||
**Specific Phase-3-units that this plan's predictions reference:**
|
||||
|
||||
| metric | tool | current value (4MB/s window) |
|
||||
|---|---|---|
|
||||
| observed receive throughput | netcat receiver byte-count | 75–725 KB/s, rep-variance high |
|
||||
| `_raw_spin_unlock_irqrestore` CPU% | perf record / report | ~20% |
|
||||
| `workqueue_execute_start`/sec | ftrace `workqueue:workqueue_execute_start` | ~5,643/sec system-wide |
|
||||
| `bes_sdio` workqueue dispatches | `cat /sys/kernel/tracing/events/workqueue/.../filter` filtered by `bes_sdio` | not measured pre-patch — **TODO before Phase 6** |
|
||||
| RX SKB rate at mac80211 boundary | trace `mac80211:drv_rx_irqsafe` count | not measured pre-patch — **TODO before Phase 6** |
|
||||
|
||||
Phase 6 must not start until the two TODOs above are filled in — otherwise Phase 7 has no reference point for the predicted-delta comparison.
|
||||
|
||||
## §4 Plan
|
||||
|
||||
### §4.1 What will be touched
|
||||
|
||||
- `bes2600_sdio.c::sdio_rx_work` — the relay loop. After this patch, it still drains the SDIO bus into SKBs but **delivers SKBs directly into `wsm_handle_rx`** instead of `skb_queue_tail`-ing them onto `self->rx_queue`.
|
||||
- `bes2600_sdio.c::bes2600_sdio_extract_packets` — the inner per-SKB extractor. Changes the in-loop action from `skb_queue_tail(&self->rx_queue, skb)` to a direct call (or callback) into the wsm dispatcher.
|
||||
- `bes2600_sdio.c::bes2600_sdio_pipe_read` — becomes unused, removed.
|
||||
- `bh.c::bes2600_bh_rx_helper` — its `BES_SDIO_RX_MULTIPLE_ENABLE` branch is no longer reachable for RX (RX path no longer feeds bh). Either gate the helper, or remove the helper outright if `bh_rx` atomic is no longer raised on RX.
|
||||
|
||||
### §4.2 What will NOT be touched
|
||||
|
||||
- `ieee80211_rx_irqsafe()` call sites — that's Patch C2 (item 2).
|
||||
- TX path — `sdio_tx_work`, `bes2600_bh_tx_helper`, etc. Untouched.
|
||||
- `sdio_wq` workqueue alloc — stays. After patch it hosts only `tx_work` + `scan_work` + (briefly during patch) `rx_work`. Renaming is cosmetic and out of scope.
|
||||
- The bh thread itself — still runs, still handles TX, still watches the timeouts.
|
||||
- `bh.c` `#if 0` graveyard — separate hygiene patch, not bundled.
|
||||
- `__bes2600_irq_enable(1)` commented-out / `asm volatile("nop")` placeholder — **deferred** per `feedback_dont_patch_downstream_artifacts`. These are symptom-shaped; Patch C may dissolve them. Re-evaluate at Task #24 (post-Patch-E observation).
|
||||
- `bh_rx` / `bh_tx` atomic split — out of scope.
|
||||
|
||||
### §4.3 Approach choice — Option A (sdio_rx_work direct delivery)
|
||||
|
||||
Two structural options surveyed in PR #8 §2.1; recap:
|
||||
|
||||
| | Option A: direct delivery from sdio_rx_work | Option B: subsume sdio_rx_work into bh thread |
|
||||
|---|---|---|
|
||||
| diff size | small | medium |
|
||||
| eliminates `rx_queue->lock` × 2 per frame | yes | yes |
|
||||
| eliminates `sdio_wq.rx_work` workqueue dispatch per IRQ | no | yes |
|
||||
| changes who calls `wsm_handle_rx` | sdio_wq context (already process context) | bh thread |
|
||||
| TX/RX SDIO bus contention | unchanged (sdio_rx_work and sdio_tx_work already share `bes2600_sdio_lock`) | adds bh ↔ sdio_tx_work contention on the SDIO mutex |
|
||||
| bisection isolation | clean: only the rx_queue handoff is removed | mixes "remove handoff" with "subsume thread" |
|
||||
|
||||
**Choosing Option A.** Reasons:
|
||||
1. Smaller diff = clearer Phase-7 attribution. If RX KB/s rises, we know it was the rx_queue handoff, not the workqueue topology.
|
||||
2. Per Markus's PR #8 review: split was for bisection clarity. Option A is narrower than Option B.
|
||||
3. The remaining cost (per-IRQ `sdio_wq.rx_work` dispatch) is ≤ 1 dispatch per IRQ batch; multi-RX coalescing means several frames per dispatch. If Phase 7 of Patch C shows that dispatch IS the residual cost, that becomes a concrete data point and motivates a *measured* Option-B follow-up, not a speculative one.
|
||||
|
||||
### §4.4 Implementation sketch (preview — actual code in Phase 6)
|
||||
|
||||
**Today** (`bes2600_sdio.c:783–831`):
|
||||
```c
|
||||
static int bes2600_sdio_extract_packets(...) {
|
||||
for each packet:
|
||||
skb = dev_alloc_skb(...);
|
||||
memcpy(skb->data, &data[pos], packet_len);
|
||||
spin_lock(&self->rx_queue_lock);
|
||||
skb_queue_tail(&self->rx_queue, skb); // ← handoff
|
||||
spin_unlock(&self->rx_queue_lock);
|
||||
}
|
||||
static void sdio_rx_work(...) {
|
||||
bes2600_sdio_extract_packets(...);
|
||||
self->irq_handler(self->irq_priv); // ← wakes bh_wq
|
||||
}
|
||||
// bh thread later: pipe_read = skb_dequeue(rx_queue) → wsm_handle_rx(skb)
|
||||
```
|
||||
|
||||
**After patch** (sketch):
|
||||
```c
|
||||
static int bes2600_sdio_extract_packets(struct sbus_priv *self, u32 ctrl_reg, u8 *data) {
|
||||
for each packet:
|
||||
skb = dev_alloc_skb(...);
|
||||
memcpy(skb->data, &data[pos], packet_len);
|
||||
ret = wsm_handle_rx(self->core, wsm_id_from(skb), wsm_hdr_of(skb), &skb);
|
||||
if (skb) dev_kfree_skb(skb);
|
||||
// no rx_queue, no spinlock, no wake-up
|
||||
}
|
||||
static void sdio_rx_work(...) {
|
||||
bes2600_sdio_extract_packets(...);
|
||||
// self->irq_handler(...) is no longer called for RX-only wakes
|
||||
// (it remains called for TX-confirm-completion paths, if any)
|
||||
}
|
||||
```
|
||||
|
||||
Caveats discovered during sketch:
|
||||
- `wsm_handle_rx`'s signature wants `(hw_priv, id, wsm_hdr*, **skb)`. `extract_packets` doesn't currently parse the wsm header — we either parse it inline (cheap; the cost is one `__le16_to_cpu`) or defer parsing into a new `bes2600_sdio_deliver_rx(skb)` helper that wraps it.
|
||||
- `hw_priv` is reachable as `self->core`.
|
||||
- Need to verify `wsm_handle_rx` is callable from sdio_wq context. **Hypothesis:** yes, because today's bh thread is also process-context-via-workqueue and that's where wsm_handle_rx already runs. Phase 6 contract-cite from `wsm.h` / call-graph confirms.
|
||||
- The `irq_handler(self->irq_priv)` wakeup at sdio_rx_work:902 — keep it, but confirm whether bh actually has remaining work after RX is gone. Possibilities: TX-confirm completions (`wsm_release_tx_buffer`) still need a bh wake. Verify in Phase 6.
|
||||
|
||||
### §4.5 Predicted delta (Phase 3 units)
|
||||
|
||||
Conservative because Patch C is item 1 only, not items 1+2.
|
||||
|
||||
| metric | predicted change | confidence |
|
||||
|---|---|---|
|
||||
| `rx_queue->lock` acquire/release rate | → 0 (lock is removed entirely; struct field deleted) | high |
|
||||
| RX-path wait-queue wakes (`bh_wq` from sdio_rx_work for RX) | → 0 (TX-confirm wakes remain) | high |
|
||||
| `_raw_spin_unlock_irqrestore` CPU% | 20 % → 12–15 % | **medium** — the rx_queue lock is one of several contributors; I don't have per-lock breakdown pre-patch |
|
||||
| `workqueue_execute_start`/sec | marginal change (≤ 5 %) | high — sdio_wq dispatch still happens per IRQ |
|
||||
| observed receive @ 4 MB/s | floor lifts from 75 KB/s → ≥ 1.0 MB/s; rep-variance shrinks | **medium** — rep 3's link death has multiple causes (decrypt-storm path is Patch A's territory; AP-side `aid 30` rejection is also possible) |
|
||||
| Phase 7 N=3 outcome | all reps ≥ 1 MB/s sustained for 30 min @ 4 MB/s | **medium** |
|
||||
|
||||
**Honest acknowledgement:** the medium-confidence predictions are the ones where Phase 7 either confirms the model or surfaces a new bug. If `_raw_spin_unlock_irqrestore` only drops to 18 %, the next-largest contributor was something else — `pool->lock` (workqueue infrastructure) or `ba_lock` — and Patch D/E/C2 become the answer.
|
||||
|
||||
### §4.6 Risks
|
||||
|
||||
1. **`wsm_handle_rx` not callable from sdio_wq**: low probability (process context, same shape as today's bh), but a cite-failure here means revert to Option B. **Phase 6 must produce a `wsm.h` contract citation** before code lands.
|
||||
2. **TX-confirm wake-ups stop firing**: if `wsm_handle_rx` was the only thing that ultimately bumped `bh_tx`, removing it from bh's input causes TX-confirm starvation. Mitigation: keep `irq_handler(irq_priv)` call in sdio_rx_work for now; let the bh's wait_event re-evaluate `bh_tx` on every wake. **Verify in Phase 6 that `wsm_release_tx_buffer` still wakes bh.**
|
||||
3. **SKB allocation under memory pressure**: `dev_alloc_skb` in extract_packets currently `msleep(100)` retries up to 10×. Calling `wsm_handle_rx` directly from extract_packets keeps us in sdio_wq context during sleep; that's the same as today, so no new risk.
|
||||
4. **rcu / locking invariants in `wsm_handle_rx`**: it traverses `priv->vif_list`, may grab `priv->vif_lock`. Currently called from bh thread. After patch: called from sdio_wq context. Both are process context, both can sleep. No new risk *unless* there's a held lock at sdio_wq level that wsm_handle_rx tries to re-acquire. **Phase 6 lock-graph audit required.**
|
||||
5. **`bes2600_chrdev_is_bus_error()` early-return**: currently checked in `pipe_read`. After patch, must move into `extract_packets` or `sdio_rx_work` so RX during a bus-error window still gets dropped, not passed to mac80211.
|
||||
6. **Multi-vif RX serialization**: the `rx_queue` is per-sbus_priv, not per-vif. After patch, multi-vif demux happens inside `wsm_handle_rx` (same as today). No new risk; same ceiling.
|
||||
|
||||
### §4.7 Phase 5 review handover
|
||||
|
||||
Goal/Situation/Measurements/Plan paste verbatim into DokuWiki when Markus initiates handover. **Do not curate** the plan for the reviewer — including the "medium-confidence" predictions and the §4.6 risk list verbatim. Reviewer should see the same uncertainty I have.
|
||||
|
||||
### §4.8 Phase 7 protocol (after Phase 6 lands)
|
||||
|
||||
Per `feedback_phase7_stress_ramp.md` — **stress ramp, not steady cap**:
|
||||
|
||||
1. Pre-patch baseline (re-anchor): 5 min @ 1 MB/s, 10 min @ 2 MB/s, 30 min @ 4 MB/s. Capture ftrace `workqueue/`, `lock/`, `mac80211/`, `mmc/`. perf record during the 4 MB/s window.
|
||||
2. Apply Patch C, install, reboot (Markus pushes).
|
||||
3. Post-patch: identical ramp, identical instrumentation.
|
||||
4. Compute deltas in **the same units** as §3 baseline. Compare to §4.5 predictions. Any unexplained delta is a finding, not a footnote — log it and loop back to Phase 4 if the model is wrong.
|
||||
5. **N=3 reps** post-patch. The user's stress-ramp memory and the receipts checklist both require this.
|
||||
6. Capture `sdio_work_debug` output and `dmesg` if any storm fires (Patch A's counter should hold steady).
|
||||
7. If Phase 7 numbers match prediction → Phase 8 memory update + proceed to Patch C2.
|
||||
8. If they don't match → loop back to Phase 4. Don't paper-fix.
|
||||
|
||||
## §5 Out-of-scope items recorded for follow-on patches
|
||||
|
||||
- **Patch C2**: items 2 — `ieee80211_rx_list` batch delivery. Gated on Task #19 kerneldoc verification.
|
||||
- **Patch D**: ba_lock atomicization at `txrx.c:998-1005, 1632`. Independent.
|
||||
- **Patch E**: ps_state_lock skip when `pm_unsupported = true` at `txrx.c:1942-1948`. Independent, gated on c7 latch.
|
||||
- **Task #24**: post-Patch-E observation of bh.c `asm volatile("nop")`, commented-out `__bes2600_irq_enable(1)`, BUG_ON in steady-state hot path. Symptom-shaped; observe before patching.
|
||||
- **Task #25**: measure `sw_mci_check_r1_ready` on RK3566 during testing.
|
||||
|
||||
---
|
||||
|
||||
*Plan written 2026-05-07 by Claude (noether). Awaiting Phase 5 second-model review on DokuWiki, initiated by Markus.*
|
||||
@@ -0,0 +1,136 @@
|
||||
# Patch C v2 — Phase 4 Plan: atomic_t prep + direct-deliver
|
||||
|
||||
**Author:** Claude (noether)
|
||||
**Status:** Phase 4 v2 — Phase 7 of Patch C (notes/patch-c-phase4-plan-2026-05-07.md, PR #9 merged) failed with a thread-safety race; this is the redesign.
|
||||
**Decision:** Option B from PR #3 close-out comment — `atomic_t` prep refactor first, direct-deliver on top.
|
||||
|
||||
---
|
||||
|
||||
## §0 What just happened (Phase 7 of Patch C)
|
||||
|
||||
Reproduced verbatim from boot -1 of ohm 2026-05-07 20:18:10 CEST, ~13 s into a 4 MB/s nc stress:
|
||||
|
||||
```
|
||||
WARNING: at wsm_release_tx_buffer+0x84/0xa0 [bes2600], CPU#0: kworker/0:3H/3912
|
||||
Workqueue: bes_sdio sdio_rx_work [bes2600]
|
||||
pc : wsm_release_tx_buffer+0x84/0xa0 [bes2600]
|
||||
lr : bes2600_bh_handle_rx_skb+0x134/0x370 [bes2600]
|
||||
sdio_rx_work+0x2a8/0x540 [bes2600]
|
||||
bes2600_wlan: wsm_release_tx_buffer failed: -1
|
||||
```
|
||||
|
||||
Storm continued; chip wedged; ohm fell off the WiFi (wlan0). Patch C module preserved at `/var/tmp/bes2600.patchC-broken.ko` for forensics. Patch B rolled back, currently on disk on ohm. Lesson saved as `feedback_phase6_contract_threadsafety` memory.
|
||||
|
||||
## §1 Why it failed
|
||||
|
||||
`wsm_release_tx_buffer()` (bh.c:222–243) does **unlocked** read–modify–write on `hw_priv->hw_bufs_used`. Pre-Patch-C invariant was single-writer = BH thread; the lock that mattered was structural, not annotated. Patch C's direct-deliver moved one writer (RX-confirm decrement) into `sdio_rx_work` workqueue context. BH thread + sdio_rx_work race on the int counter; underflow below zero, WARN, return -1, bookkeeping corrupt, TX wedges.
|
||||
|
||||
Phase 6 contract block correctly cited `wsm_handle_rx`'s sleepability and held-lock invariants — but stopped at the called function's signature. It did not enumerate `hw_bufs_used` as shared state mutated by the callee. That's the gap.
|
||||
|
||||
## §2 Shared-state delta table (the thing missing from Patch C)
|
||||
|
||||
Every field that `bes2600_bh_handle_rx_skb` mutates either directly or transitively, with current protection and required action:
|
||||
|
||||
| field | declared at | written by (today) | written by (after Patch C v2) | current protection | action needed |
|
||||
|---|---|---|---|---|---|
|
||||
| `hw_priv->hw_bufs_used` | bes2600.h | `wsm_alloc_tx_buffer` (bh thread, TX submit), `wsm_release_tx_buffer` (bh thread, RX confirm), `main.c:543` (init) | + `wsm_release_tx_buffer` from sdio_rx_work | single-writer = BH thread (structural) | **convert to `atomic_t`** |
|
||||
| `hw_priv->hw_bufs_used_vif[i]` | bes2600.h | `wsm_release_vif_tx_buffer` (bh thread), `bh.c:1271` (vif TX submit), init | + `wsm_release_vif_tx_buffer` from sdio_rx_work | single-writer = BH thread | **convert to `atomic_t [N]`** |
|
||||
| `hw_priv->wsm_rx_seq[i]` | bes2600.h | bh thread RX | sdio_rx_work only | single-writer = BH/sdio_rx context (was BH, now is sdio_rx_work, but still **one writer**) | OK — single writer |
|
||||
| `hw_priv->wsm_tx_pending[i]` | bes2600.h | `bes2600_bh_inc_pending_count` (TX submit, BH thread), `bes2600_bh_dec_pending_count` (RX confirm) | dec moves to sdio_rx_work; inc stays BH | single-writer = BH | **also needs `atomic_t`** |
|
||||
| `hw_priv->lmac_mon_timer` / `mcu_mon_timer` | bes2600.h | mod_timer / del_timer_sync from BH | ditto from sdio_rx_work | timer API is internally locked | OK — `mod_timer` is concurrency-safe |
|
||||
| `hw_priv->wsm_cmd.lock` (taken inside wsm_handle_rx) | wsm_buf | bh thread (today) | sdio_rx_work | spinlock | OK — already protected |
|
||||
| `hw_priv->vif_lock` (taken inside wsm_handle_rx for some paths) | per vif | bh thread today | sdio_rx_work | spinlock | OK |
|
||||
| `priv->bh_evt_wq` wake-up | bes2600.h | wsm_release_tx_buffer when count hits 0 | ditto from sdio_rx_work | wake_up is concurrency-safe | OK |
|
||||
| `bes2600_pwr_clear_busy_event` (called inside release) | bes_pwr | bh thread | sdio_rx_work | internal locking via `bes_power.lock` | OK |
|
||||
| `hw_priv->buf_released` | bes2600.h | only `wsm_release_buffer_to_fw` (MCAST_FWDING ifdef, AP-only) | unchanged — BH only | single-writer = BH | OK — not on Patch C v2 hot path |
|
||||
|
||||
**Three fields require atomic_t conversion:** `hw_bufs_used`, `hw_bufs_used_vif[]`, `wsm_tx_pending[]`. Everything else is already concurrency-safe or moves cleanly to single-writer-in-sdio_rx_work.
|
||||
|
||||
## §3 Read-site survey (the rest of the work — atomic_read swaps)
|
||||
|
||||
`grep -hE "hw_bufs_used\b|hw_bufs_used_vif\b" *.c *.h | wc -l` = **57 references** across the source tree:
|
||||
|
||||
- 5 writers (above)
|
||||
- 52 readers — converted mechanically to `atomic_read()`. Distribution:
|
||||
- `bh.c`: 22 read sites (most in the bh main loop, BUG_ON gates, idle / suspend predicates)
|
||||
- `sta.c`: 3 sites (PM idle check at sta.c:1231–1253)
|
||||
- `bes2600_sdio.c`: 1 site (PM idle check at line 958)
|
||||
- `main.c`: 2 sites (init zero, teardown wait)
|
||||
- `debug.c`: 1 site (debugfs stats)
|
||||
- `itp.c`: 1 site (test mode)
|
||||
|
||||
`wsm_tx_pending[i]` site count is smaller — ~6 references, all in bh.c and the timer monitors. Same mechanical conversion.
|
||||
|
||||
## §4 Plan v2 — two-step
|
||||
|
||||
**Patch C-prep** (NFC, lands first):
|
||||
|
||||
- Convert `hw_bufs_used` from `int` → `atomic_t`.
|
||||
- Convert `hw_bufs_used_vif[CW12XX_MAX_VIFS]` from `int[]` → `atomic_t[]`.
|
||||
- Convert `wsm_tx_pending[2]` from `int[]` → `atomic_t[]`.
|
||||
- Update writers:
|
||||
- `wsm_alloc_tx_buffer`: `atomic_inc(&hw_priv->hw_bufs_used)`.
|
||||
- `wsm_release_tx_buffer`: rewrite with `atomic_fetch_sub_release(count, &hw_priv->hw_bufs_used)` — returns prior value. Re-derive the "tx restart" predicate (`prior >= numInpChBufs - 1`) and the "wake bh_evt_wq + clear busy" predicate (`prior - count == 0`) from that. WARN if `prior - count < 0`.
|
||||
- `wsm_release_vif_tx_buffer`: same pattern on the array element.
|
||||
- `bes2600_bh_inc/dec_pending_count`: use `atomic_inc` and `atomic_dec_return` (need post-decrement value to decide whether to del_timer).
|
||||
- Update all 52+6 read sites: mechanical `atomic_read()` swap.
|
||||
- `main.c:543` init: `atomic_set(&hw_priv->hw_bufs_used_vif[i], 0)`.
|
||||
|
||||
**Patch C-prep does NOT change behaviour.** Same atomic ordering (`_release` / `_acquire` chosen to match the implicit memory ordering the BH-only path had). Phase 7 of C-prep alone should show **identical** numbers to pre-patch baseline (`run-20260507-patchC-preflight`): 1.36 MB/s, 86.4 sdio_rx_work/sec, 90.3 dispatches per 1000 RX pkts, 0 bh_work redispatches. If Phase 7 of C-prep shows a delta, the atomic ordering is wrong and we loop back here, not to C v2.
|
||||
|
||||
**Patch C v2** (the actual structural change, lands on top of C-prep):
|
||||
|
||||
- Identical to Patch C as merged in PR #3 (since closed): direct-deliver from `bes2600_sdio_extract_packets` into `bes2600_bh_handle_rx_skb`, no `rx_queue` indirection, no bh wake-up for RX.
|
||||
- The contract block in `bh.c::bes2600_bh_handle_rx_skb` is **expanded** to include the shared-state delta table from §2 of this plan, with explicit citations.
|
||||
- Same minimum-diff scope as Patch C: keep `rx_queue`, `pipe_read`, `bh_rx_helper` for clean bisection; remove in a follow-up hygiene patch.
|
||||
|
||||
## §5 What will NOT be touched (deferred or out of scope)
|
||||
|
||||
- mac80211-side `ieee80211_rx_irqsafe` → `ieee80211_rx_list` migration: that's Patch C2, gated on Task #19 kerneldoc verification.
|
||||
- The `#if 0` graveyard in bh.c, the `asm volatile("nop")` placeholder, the BUG_ON in steady-state hot path: still symptom-shaped per `feedback_dont_patch_downstream_artifacts`. Re-evaluate at Task #24 after C v2 / D / E land.
|
||||
- `ba_lock` (Patch D) and `ps_state_lock` (Patch E): independent.
|
||||
|
||||
## §6 Risk list (per Phase 6 contract-thread-safety memory)
|
||||
|
||||
1. **C-prep memory ordering**: I've chosen `atomic_fetch_sub_release` for `wsm_release_tx_buffer` to mirror the implicit BH-thread ordering (release before subsequent atomic ops on `bh_evt_wq` / `bes_power`). If the BH thread or other readers expect `_acquire` semantics on the value, we get reordering bugs that are hard to reproduce. **Mitigation:** pair with `_acquire` reads where the read-then-decision pattern is critical (e.g., the bh main loop's `if (!hw_priv->hw_bufs_used)` idle predicate). Cite the kerneldoc reference for `atomic_fetch_sub_release` in the commit message.
|
||||
|
||||
2. **`wsm_tx_pending[]` decrement-side timer interaction**: `bes2600_bh_dec_pending_count` does `if (--hw_priv->wsm_tx_pending[idx] == 0) del_timer_sync(timer); else mod_timer(timer, ...)`. After atomic_t conversion: `if (atomic_dec_return(&hw_priv->wsm_tx_pending[idx]) == 0) ...`. But *another* thread could `atomic_inc` between our dec and the timer call, racing the del_timer. `del_timer_sync` is internally safe (it can be called concurrently with `mod_timer`), but the **decision** "whether to delete vs mod" is racy. **Mitigation:** even after atomic conversion, this function still needs to be called from a single context. Verify `inc/dec_pending_count` callers — if both sides only fire from BH and sdio_rx_work and never overlap on the same idx, we're fine; if not, this needs a lock.
|
||||
|
||||
3. **`hw_bufs_used_vif[]` array vs `wsm_alloc_tx_buffer`**: vif counter increment lives at bh.c:1271, called from bh thread TX-submit path. Decrement (`wsm_release_vif_tx_buffer`) called from RX-confirm. After Patch C v2 the decrement is in sdio_rx_work — same race shape as the global counter. Already covered by the atomic_t array conversion.
|
||||
|
||||
4. **PM idle predicate at sta.c:1239**: reads `hw_priv->hw_bufs_used_vif[priv->if_id]` to decide can-sleep. Currently racy (was already reading BH-mutated state from a non-BH PM context). Atomic conversion makes the read coherent. PM context's read-then-decide is still fundamentally a snapshot — no change in semantics, just no torn-read.
|
||||
|
||||
5. **Reboot / module-unload teardown** (`main.c:840`): `wait_event_timeout(... !hw_priv->hw_bufs_used ...)`. Becomes `... !atomic_read(...)`. No semantic change — the wait_event macro re-evaluates the predicate on each wake.
|
||||
|
||||
6. **Phase 7 rig: Patch C v2 still wedges chip if I missed anything**: now mitigated by ohm's new wired interface (enu1, 192.168.88.80) — survives bes2600 wedges, lets us collect dmesg / ftrace / journalctl from a wedged ohm without reboot. See `reference_ohm_wired_iface` memory.
|
||||
|
||||
## §7 Phase 5 review handover
|
||||
|
||||
PR on git.reauktion.de/marfrit/besser, this file as the artifact (per `feedback_phase5_surface_is_pr`). Specifically request reviewer focus on §2 shared-state delta table — that's the part that should have caught Patch C's bug. Don't curate.
|
||||
|
||||
## §8 Phase 6 implementation order
|
||||
|
||||
1. Branch off `cleanups` on bes2600-dkms-mobian: `bes2600/atomic-tx-buf-counters` (= Patch C-prep).
|
||||
2. Mechanical refactor: `int hw_bufs_used` → `atomic_t hw_bufs_used`, all reads → `atomic_read`, all writes → atomic ops. Same for vif array and tx_pending array. No other changes.
|
||||
3. Build, install, smoke-test. Phase 7 of C-prep. Should be a no-op delta.
|
||||
4. PR + Phase 5 review + merge.
|
||||
5. Branch off C-prep: `bes2600/sdio-rx-direct-deliver-v2` (= Patch C v2).
|
||||
6. Re-apply the Patch C delta (3 files: bh.h, bh.c, bes2600_sdio.c — same edits as PR #3).
|
||||
7. Build, install, Phase 7 N=3 stress ramp.
|
||||
8. PR + Phase 5 review + merge.
|
||||
|
||||
## §9 Phase 7 v2 protocol (per `feedback_phase7_stress_ramp` + wired-rig)
|
||||
|
||||
1. Pre-C-prep baseline rep N=3 (re-anchor, since current N=1 baseline is from `run-20260507-patchC-preflight`).
|
||||
2. Apply C-prep, N=3. Compare to pre. Expect: zero meaningful delta. If non-zero → memory-ordering bug, loop back to §4 atomic-ordering choice.
|
||||
3. Apply C v2, N=3. Compare to C-prep baseline. Expect: §4.5 of original Patch C plan's predicted delta (rx_queue lock acquires → 0, observed RX KB/s lifts toward ≥1 MB/s sustained @ 4MB/s).
|
||||
4. **All Phase 7 stress runs use the wired path (`ssh mfritsche@192.168.88.80`) for telemetry collection.** When the chip wedges (it shouldn't this time, but planning for it), wlan0 stops responding but enu1 stays alive. Collect dmesg / ftrace / journalctl over enu1 BEFORE rebooting. This is the data we lost in Patch C boot -1 because wlan0 was the only path.
|
||||
5. N=3 reps per phase per `feedback_phase7_stress_ramp`. Don't accept N=1 as verification.
|
||||
|
||||
## §10 Closeout
|
||||
|
||||
If C-prep + C v2 both pass Phase 7: proceed to D (ba_lock atomicization), E (ps_state_lock skip). Markus's "we're not on the clock" applies — sequencing per bisection clarity, not delivery deadline.
|
||||
|
||||
---
|
||||
|
||||
*Plan written 2026-05-07 by Claude (noether), in response to Patch C Phase 7 failure. Phase 5 review = PR comments on this artifact at git.reauktion.de/marfrit/besser. Don't curate the shared-state delta table for the reviewer — that's the part the previous round's reviewer should have caught me on.*
|
||||
@@ -0,0 +1,127 @@
|
||||
# Patch C v3 — Phase 4 Plan: drop sdio_rx_work, match cw1200 architecture
|
||||
|
||||
**Author:** Claude (noether)
|
||||
**Status:** Phase 4 v3 — supersedes v2 (PR #10) after cw1200 mainline survey showed the race-free path is structural, not lock-based.
|
||||
**Decision:** drop the `sdio_rx_work` workqueue entirely; SDIO IRQ wakes `bh_wq`; bh thread does the SDIO read inline. Restores single-writer-from-bh invariant on `hw_bufs_used` *by construction*. No `atomic_t` prep needed.
|
||||
|
||||
---
|
||||
|
||||
## §0 Why v3 supersedes v2
|
||||
|
||||
PR #10's plan was: convert `hw_bufs_used` etc. to `atomic_t` (prep), then direct-deliver from `sdio_rx_work` (structural). That was a workaround for the race that *only existed because of the relay*.
|
||||
|
||||
The cw1200 mining (`~/src/linux-rockchip`, 228 cw1200 commits) showed the upstream answer: there is no relay. cw1200's IRQ handler bumps `bh_rx` and wakes the bh thread; the bh thread does the SDIO read itself inside `cw1200_bh_rx_helper` (`drivers/net/wireless/st/cw1200/bh.c:233`). Single thread = single writer for `hw_bufs_used` = no race. Same `int hw_bufs_used` as bes2600, never atomic_t'd in 16 years upstream because it never needed to be.
|
||||
|
||||
Patch C v3 brings bes2600 into that shape. The structural simplification is bigger than v2's diff but lands the right architecture in one move.
|
||||
|
||||
## §1 Goal
|
||||
|
||||
Same as Patch C v2 §1: ≥ 1 MB/s sustained receive @ 4 MB/s sender, < 15 % `_raw_spin_unlock_irqrestore` CPU%, no 30-min cascade to link-death. Stretch toward Phase 1's full 2 MB/s once Patch C2 (rx_list batch) lands separately.
|
||||
|
||||
## §2 Situation
|
||||
|
||||
- Cleanups branch is at Patch F merged (commit `b717251`). All Phase 5 reviews of the F series merged via PR #4.
|
||||
- ohm rebooted with F module live (srcversion `A9438692D6A8698F92AEEA1`) — F is the new baseline for Patch C v3 Phase 7 comparison.
|
||||
- Wired path `enu1` at `192.168.88.80` survives bes2600 wedges; lmcp `ohm` still goes through wlan0. Phase 7 telemetry collection over enu1.
|
||||
- Reboot-permission override active (ohm dev-allocated; I can `sudo reboot` directly — `feedback_user_pushes_reboot_button` override clause).
|
||||
|
||||
## §3 Baseline measurements
|
||||
|
||||
Carry forward from `run-20260507-patchC-preflight/baseline.tsv` (N=1, F-less Patch B module):
|
||||
|
||||
| metric | value |
|
||||
|---|---|
|
||||
| observed receive @ 4 MB/s | 1.362 MB/s |
|
||||
| sdio_rx_work dispatches | 86.4/s = 90.3 per 1000 RX packets |
|
||||
| sdio_tx_work dispatches | 276.1/s |
|
||||
| bes2600_bh_work redispatches | 0 (single long-lived) |
|
||||
|
||||
**Phase 6 prereq:** capture an N=3 baseline ON THE F MODULE before Patch C v3 code lands. Same instrumentation, same stress ramp. This is the post-F / pre-v3 reference. Without it, Phase 7's delta is C+F vs B+nothing — confounded.
|
||||
|
||||
## §4 Plan v3
|
||||
|
||||
### §4.1 What gets eliminated
|
||||
|
||||
- **`sdio_rx_work` (bes2600_sdio.c:829)** — function deleted. No longer queued, no longer runs.
|
||||
- **`self->rx_work` work_struct** — field deleted from `struct sbus_priv`. `INIT_WORK` removed.
|
||||
- **`self->rx_queue` + `self->rx_queue_lock`** — fields deleted. `skb_queue_head_init` removed. No SKB ever queued there.
|
||||
- **`bes2600_sdio_pipe_read`** — function deleted. No callers after this patch.
|
||||
- **`sbus_ops->pipe_read`** — sbus op slot deleted (or kept and stubbed; tx_loop.c also implements it for the test-loop bus, has to stay if test-loop is preserved).
|
||||
- **`queue_work(self->sdio_wq, &self->rx_work)`** at the 3 call sites in `bes2600_sdio.c` (lines 416, 941, 1199) — removed.
|
||||
|
||||
### §4.2 What gets added
|
||||
|
||||
- **A new `bes2600_bh_handle_rx_skb()`** in bh.c (same shape as Patch C added, same contract block; no longer needs to also wake the bh thread because we ARE the bh thread).
|
||||
- **A new helper `bes2600_sdio_read_rx_batch()`** in bes2600_sdio.c, exported, that does what `sdio_rx_work` used to do MINUS the queuing: lock → read ctrl_reg → memcpy_fromio → packets_check → for-each-frame extract+deliver. Called from bh.
|
||||
|
||||
### §4.3 What gets rewired
|
||||
|
||||
- **`bes2600_gpio_irq_handler`** in bes2600_sdio.c:413 (the GPIO-IRQ path used when CONFIG_BES2600_USE_GPIO_IRQ is set): drop `queue_work(self->sdio_wq, &self->rx_work)`; instead call `self->irq_handler(self->irq_priv)` directly (which is `bes2600_irq_handler` in bh.c, bumps `bh_rx` + wakes `bh_wq`). Matches cw1200_sdio_irq_handler shape.
|
||||
- **`bes2600_bh_rx_helper`** (bh.c:961, BES_SDIO_RX_MULTIPLE_ENABLE branch): instead of `pipe_read`-ing one SKB from the (now-gone) rx_queue, call the new `bes2600_sdio_read_rx_batch()` which does the SDIO read AND delivers each frame inline via `bes2600_bh_handle_rx_skb()`. Returns count delivered, or negative on error.
|
||||
- **`bes2600_bh()` outer loop**: after a successful rx_batch read, the helper signals whether to continue draining (more frames pending) — same shape as today's `BH_RX_CONT_LIMIT=3` outer loop.
|
||||
- **`bes2600_gpio_wakeup_mcu(SDIO_RX)`** + **`bes2600_gpio_allow_mcu_sleep(SDIO_RX)`** brackets: currently called inside sdio_rx_work. Move into bh thread around the `bes2600_sdio_read_rx_batch()` call. Same wake-flag bracketing, just from a different thread.
|
||||
- **`sdio_wq` workqueue**: keeps `tx_work` and (briefly) `scan_work`. Renamed or kept — cosmetic. Don't touch in this patch.
|
||||
|
||||
### §4.4 What stays untouched
|
||||
|
||||
- TX path (`sdio_tx_work`, `bes2600_bh_tx_helper`, `wsm_alloc_tx_buffer`). Independent.
|
||||
- WSM protocol layer (`wsm.c`, `wsm_handle_rx`). Same callees, just from bh thread now.
|
||||
- mac80211 RX delivery (`ieee80211_rx_irqsafe`). That's Patch C2.
|
||||
- `BES2600_RX_IN_BH` ifdef gate. Stays defined; the gated branch is now the only RX path.
|
||||
- Symptom-shaped artifacts (asm nop, BUG_ON in hot path) — still deferred, see task #24 post-cleanup.
|
||||
|
||||
## §5 Shared-state delta table (the v2 lesson, applied)
|
||||
|
||||
Every field `bes2600_bh_handle_rx_skb` mutates directly or transitively, with the v3 protection:
|
||||
|
||||
| field | written by (today) | written by (after v3) | concurrency | required action |
|
||||
|---|---|---|---|---|
|
||||
| `hw_priv->hw_bufs_used` | bh thread (TX submit + RX confirm), main.c init | **bh thread only** (RX moves into bh) | single-writer | none — `int` is fine, race-free by construction |
|
||||
| `hw_priv->hw_bufs_used_vif[i]` | bh thread (TX vif submit + RX vif confirm), main.c init | **bh thread only** | single-writer | none |
|
||||
| `hw_priv->wsm_rx_seq[i]` | sdio_rx_work today | bh thread | single-writer | none — moves cleanly between contexts |
|
||||
| `hw_priv->wsm_tx_pending[i]` | bh thread (inc on TX submit), bh+sdio_rx_work (dec on RX confirm) | **bh thread only** | single-writer | none |
|
||||
| `hw_priv->lmac_mon_timer` / `mcu_mon_timer` | mod_timer / del_timer_sync from bh + sdio_rx_work | bh thread only | timer API safe anyway | none |
|
||||
| `hw_priv->wsm_cmd.lock` | spinlock taken inside wsm_handle_rx | same | already protected | none |
|
||||
| `priv->bh_evt_wq` wake-up | wsm_release_tx_buffer when count→0 | same | wake_up is concurrency-safe | none |
|
||||
| `bes_pwr.lock` (inside bes2600_pwr_clear_busy_event) | bh thread (today) | bh thread | already protected | none |
|
||||
| `self->rx_data_cnt` etc. (sbus_priv stats) | sdio_rx_work | bh thread | single-writer | none |
|
||||
|
||||
**Zero fields require new locking.** The architectural pivot eliminates the race v2's atomic_t was working around.
|
||||
|
||||
## §6 Risks
|
||||
|
||||
1. **bh thread now holds the SDIO bus mutex during read** (currently held by sdio_rx_work). TX work in the same bh thread is unaffected (sdio_tx_work runs on a separate workqueue and shares the same mutex anyway). The sdio_lock contention pattern doesn't change.
|
||||
2. **Loss of "parallelism" between sdio_rx_work and bh TX**: sdio_rx_work and bh thread *appeared* to run in parallel today, but both serialize through `bes2600_sdio_lock(self)` for the actual bus operations. The parallelism was illusory. Net throughput should not regress.
|
||||
3. **bh thread CPU-busy-time per RX batch increases**: inline SDIO read is the same cost, just charged to bh instead of sdio_wq's worker. Mitigation: the per-IRQ workqueue dispatch cost (~86/s) is what we trade for it. Net: -86 dispatches/s, +0 µs per frame.
|
||||
4. **Multi-RX coalescing (BES_SDIO_RX_MULTIPLE_NUM=16)** stays. bes2600_sdio_extract_packets parses the multi-frame buffer same as before, just inline now. No functional change to chip-side behaviour.
|
||||
5. **GPIO wake-flag bracketing**: `bes2600_gpio_wakeup_mcu(SDIO_RX)` and `bes2600_gpio_allow_mcu_sleep(SDIO_RX)` currently bracket sdio_rx_work. Move them to bracket the new bh-side read. If the wake-flag accounting is sub-system-scoped (it is — flag bits per subsystem), this is a clean move.
|
||||
6. **IRQ re-enable in bh thread**: cw1200's bh re-enables IRQ via `__cw1200_irq_enable(priv, 1)` after each round. bes2600 has the analogous `__bes2600_irq_enable(0/1)` (commented out as the `asm volatile("nop")` symptom in `bh.c:1518-1520`). This patch does NOT re-engage the commented-out re-enable — that's still task #24's call. But if the IRQ stays disabled across rounds, we'd never receive the next IRQ. **Investigate before Phase 6 lands**: where does IRQ re-enable happen in the current bes2600 hot path? The sdio_func IRQ may be auto-managed by sdio core differently. Block Phase 6 on this audit.
|
||||
7. **Phase 7 wedge resilience**: if v3 has a different bug shape than v2's race (which it shouldn't, since the race is gone by construction), the wired path lets us collect telemetry from a wedged ohm.
|
||||
|
||||
## §7 Phase 5 / 6 / 7
|
||||
|
||||
- **Phase 5**: PR on `git.reauktion.de/marfrit/besser` with this artifact. Specifically request reviewer focus on §6 risk #6 (IRQ re-enable mechanism).
|
||||
- **Phase 6**: branch off cleanups (post-F): `bes2600/sdio-rx-no-relay`. Implement the file changes per §4. Build, install, smoke-test.
|
||||
- **Phase 7**:
|
||||
- First: N=3 stress-ramp **on F module** (post-F pre-v3 baseline). 10 min @ 1, 30 min @ 2, 30 min @ 4 MB/s. Use wired path for telemetry.
|
||||
- Then: install v3 module, identical N=3 ramp. Compare deltas.
|
||||
- Predicted: sdio_rx_work dispatch rate → 0/s (was 86/s). observed receive lifts toward ≥ 1.0 MB/s sustained. `_raw_spin_unlock_irqrestore` drops by the rx_queue lock contribution (was 1914/s acquires).
|
||||
|
||||
## §8 What gets dropped from v2 plan
|
||||
|
||||
- atomic_t prep refactor (`hw_bufs_used` → `atomic_t`): not needed. Single-writer invariant preserved structurally. Still a defensible standalone hardening patch *if mainlining bes2600 ever requires defense-in-depth*, but not on the Bug-#5 critical path.
|
||||
- `wsm_tx_pending[]` decrement-decision race (v2 risk #2): also moots. Both sides single-thread under v3.
|
||||
- v2 Phase 7's "C-prep should show zero delta" gate: replaced by "v3 should match cw1200's structural shape" gate.
|
||||
|
||||
## §9 Open question for reviewer
|
||||
|
||||
The big one is §6 risk #6 — IRQ re-enable. cw1200 explicitly does `__cw1200_irq_enable(priv, 1)` from bh after each round; bes2600 has the call **commented out** with an `asm volatile("nop")` placeholder. Either:
|
||||
|
||||
(a) bes2600's SDIO IRQ is level-triggered + auto-acked by SDIO core, so re-enable isn't needed (that would explain the nop).
|
||||
(b) The current code happens to work because sdio_rx_work is queued by the IRQ regardless of whether IRQ is "enabled" by the driver-side flag. After v3 we have to manually re-enable like cw1200 does.
|
||||
|
||||
Need to confirm (a) vs (b) before Phase 6 lands. Plan to grep for `__bes2600_irq_enable` callsites and trace back to whether it's load-bearing.
|
||||
|
||||
---
|
||||
|
||||
*Plan written 2026-05-07 by Claude (noether), after Patch F merged and Patch C v2 (PR #10) was superseded by the cw1200 architectural mining finding. Phase 5 review on PR. Don't curate.*
|
||||
@@ -0,0 +1,171 @@
|
||||
# Patch C2 — Phase 4 Plan: migrate ieee80211_rx_irqsafe → ieee80211_rx_list
|
||||
|
||||
**Author:** Claude (noether)
|
||||
**Status:** Phase 4 — pending Phase 5 PR review before any Phase 6 code.
|
||||
**Predecessor:** Patch C v3 (PR #5 merged, +73% throughput, no-relay architecture); Patch D + E + F + G also landed. Cleanups branch tip = 42fd0ce.
|
||||
**Task #19 contract**: `ieee80211_rx_list` callable from process context, **requires `local_bh_disable()` + `rcu_read_lock()` wrap**, **cannot mix with `ieee80211_rx_irqsafe()` for the same hardware** → all 6 sites convert in one shot.
|
||||
|
||||
---
|
||||
|
||||
## §0 Substrate
|
||||
|
||||
After Patch C v3:
|
||||
- bh thread is the sole RX-delivery context (no relay, no sdio_rx_work)
|
||||
- Per-frame work runs in process context (sleepable)
|
||||
- Single-writer-from-bh invariant covers `hw_bufs_used` and friends
|
||||
|
||||
`ieee80211_rx_irqsafe` is currently called from process context. Per kerneldoc (`include/net/mac80211.h:5399-5411`):
|
||||
|
||||
> **Like ieee80211_rx() but can be called in IRQ context** (internally defers to a tasklet.)
|
||||
|
||||
The tasklet hop is the cost we pay today for delivering each RX frame from process context. `ieee80211_rx_list` is the process-context replacement.
|
||||
|
||||
## §1 Goal
|
||||
|
||||
Per-frame: skip the tasklet hop. Batch: process multiple SKBs from one SDIO read inside a single `local_bh_disable()`/`rcu_read_lock()` window.
|
||||
|
||||
Phase 1 metric: **RX throughput @ 4 MB/s sender**, with v3 N=3 baseline = 2.352 MB/s. Hypothesis: small to moderate uplift (<10%) from removing the tasklet deferral. Larger improvement would be surprising — if observed, that's a finding to investigate.
|
||||
|
||||
## §2 Situation
|
||||
|
||||
- 6 call sites in bes2600 currently use `ieee80211_rx_irqsafe`:
|
||||
- `ap.c:96` (AP-mode link-id RX queue drain)
|
||||
- `sta.c:1487` (link-id rx_queue drain in ?)
|
||||
- `txrx.c:1960` (early-data + pm_unsupported branch — Patch E added)
|
||||
- `txrx.c:1967` (early-data + LINK_SOFT-not-set branch)
|
||||
- `txrx.c:1971` (normal RX path)
|
||||
- `wsm.c:2415` (beacon SKB delivery from `bes2600_beacon_handler`?)
|
||||
- All 6 must convert together (kerneldoc: cannot mix per hardware)
|
||||
- bh thread is single-writer post-v3 → `_rx_list`'s "calls must be synchronized" satisfied trivially
|
||||
- bh thread is process context → `_rx_list` callable
|
||||
|
||||
## §3 Baseline (carry forward)
|
||||
|
||||
From `notes/phase7-v3-2026-05-07.md` (v3 N=3 ramp, Phase 7 closed):
|
||||
|
||||
| metric | v3 fresh-chip N=3 |
|
||||
|---|---|
|
||||
| RX throughput @ 4 MB/s | mean 2.352 MB/s, min 2.102, max 2.590 |
|
||||
| sdio_rx_work dispatches | 0/s |
|
||||
| bh_work redispatches | 0 |
|
||||
|
||||
Phase 7 of C2 will compare against this baseline.
|
||||
|
||||
## §4 Plan
|
||||
|
||||
### §4.1 Conversion shape
|
||||
|
||||
Per call site:
|
||||
```c
|
||||
ieee80211_rx_irqsafe(priv->hw, skb);
|
||||
```
|
||||
becomes:
|
||||
```c
|
||||
ieee80211_rx_list(priv->hw, NULL, skb, &priv->rx_list);
|
||||
```
|
||||
|
||||
Where `priv->rx_list` is a `struct list_head` initialized once.
|
||||
|
||||
**Wrap requirement:** `local_bh_disable()` + `rcu_read_lock()` must be held across the call. Per the kerneldoc, that's also needed for batch correctness.
|
||||
|
||||
### §4.2 Wrap placement (the design decision)
|
||||
|
||||
**Option A — per-call wrap.** Wrap each individual `ieee80211_rx_list()` call. Simple but loses the batch benefit (each call's wrap+unwrap costs as much as the avoided tasklet defer).
|
||||
|
||||
**Option B — per-batch wrap.** Wrap the OUTER frame-iteration loop (e.g., the `for` in `bes2600_sdio_extract_packets`). All 16 SKBs from one SDIO read get delivered inside one wrap. This is the upstream-idiomatic pattern (mt76, iwl_pcie do this).
|
||||
|
||||
Choosing **Option B**. Concrete shape:
|
||||
|
||||
- `bes2600_sdio_read_rx_batch` (the per-SDIO-batch entry point added in Patch C v3) wraps the read+extract+deliver phase:
|
||||
```c
|
||||
rcu_read_lock();
|
||||
local_bh_disable();
|
||||
// existing read + extract_packets that calls bh_handle_rx_skb per frame
|
||||
local_bh_enable();
|
||||
rcu_read_unlock();
|
||||
```
|
||||
- Inside `bes2600_bh_handle_rx_skb`, the single `ieee80211_rx_irqsafe` swap becomes `ieee80211_rx_list(priv->hw, NULL, skb, &priv->rx_list)`.
|
||||
- The OTHER 5 call sites (in `ap.c`, `sta.c`, `txrx.c`'s branches, `wsm.c`) need the same treatment, but they're called from the bh thread (post-v3) so they're already in the right context. Each gets its own narrow wrap (Option A applied selectively because those paths process one frame at a time, not a batch).
|
||||
|
||||
### §4.3 The `rx_list` field
|
||||
|
||||
Add `struct list_head rx_list` to either `struct bes2600_common` (driver-wide) or `struct bes2600_vif` (per-vif). Per-vif is cleaner because the existing `priv->hw` parameter implies vif scope.
|
||||
|
||||
`INIT_LIST_HEAD(&priv->rx_list)` at vif setup; no teardown needed (mac80211 owns the SKBs once handed off).
|
||||
|
||||
**Open question for reviewer:** does the `rx_list` need to be drained explicitly after the batch (e.g., via a `list_for_each_entry_safe` + `netif_receive_skb_list_internal`)? Looking at mainline mt76 / iwl_pcie usage will clarify. Phase 6 must answer this before code lands.
|
||||
|
||||
### §4.4 What will NOT be touched
|
||||
|
||||
- The 6 call sites change atomically (all-or-nothing per kerneldoc) — no per-site progressive migration
|
||||
- `wsm.c:2415` beacon path: same conversion shape, but beacon delivery is once-per-beacon-interval (not hot path); could stay `_irqsafe` if upstream allows mixing per-SKB-type. Re-read kerneldoc carefully — it says "per hardware", not per-call-site, so we can't keep _irqsafe even on the slow paths.
|
||||
- bh thread structure (Patch C v3 stands)
|
||||
- atomic_t counters from Patch D
|
||||
- `pm_unsupported` lock-skip from Patch E
|
||||
- mac80211 batch-delivery semantics (mainline owns this; we just call the API)
|
||||
|
||||
### §4.5 Predicted delta in Phase 3 units
|
||||
|
||||
| metric | predicted |
|
||||
|---|---|
|
||||
| `rx_irqsafe` tasklet schedule rate | → 0 (function no longer called) |
|
||||
| RX throughput @ 4 MB/s sustained | 2.352 → +5-15% (medium confidence) |
|
||||
| `_raw_spin_unlock_irqrestore` CPU% | small drop (no tasklet schedule lock contribution) |
|
||||
|
||||
**Honest acknowledgment:** I don't have data on how much the tasklet hop actually costs. The improvement might be smaller than predicted if tasklet defer was already cheap on this kernel. If <2%, Phase 7 says "marginal but no regression" and we ship anyway for upstream-cleanliness.
|
||||
|
||||
### §4.6 Risks
|
||||
|
||||
1. **`ieee80211_rx_list` semantics surprise.** mainline drivers I have access to (mt76, iwl_pcie) use this via NAPI infrastructure. bes2600 doesn't have NAPI; we're doing process-context-direct. The kerneldoc says callable that way but we should verify a few mainline drivers actually do it. **Phase 6 contract-cite from at least one upstream caller** before code lands.
|
||||
|
||||
2. **`rx_list` lifetime in cross-batch / cross-vif scenarios.** Multiple vifs (P2P_MULTIVIF=y in Makefile) might race on the same hw's `rx_list`. The kerneldoc says "for a single hardware" — the list is per-call destination, which means each call appends to its argument list. Per-vif `rx_list` per-call is the natural shape. No per-hw aggregator needed.
|
||||
|
||||
3. **`local_bh_disable` cost in batch wrap.** Not free. If the batch is small (1-2 SKBs), the wrap might dominate. Estimated breakeven: 2-3 SKBs per wrap. Phase 7 should look at SKB-per-batch distribution to confirm.
|
||||
|
||||
4. **`rcu_read_lock` across SDIO read.** SDIO read can take multi-ms (multi-block transfers). RCU reader-cs across that is fine (no preemption blocked) but it's a longer reader-cs than typical. Verifiable but not a blocker — kerneldoc requires it.
|
||||
|
||||
5. **wsm.c:2415 (beacon) is a different SKB lifecycle** — `hw_priv->beacon` is owned by hw_priv, not allocated per-call. After `_rx_list` consumes it (by passing ownership to mac80211), `hw_priv->beacon` is dangling. **Phase 6 must verify the beacon path either reallocates after delivery or wasn't actually transferring ownership.** Risk #5 is the biggest open question.
|
||||
|
||||
### §4.7 Phase 5 review handover
|
||||
|
||||
PR on `git.reauktion.de/marfrit/besser` with this artifact. Specifically request reviewer focus on:
|
||||
- §4.2 wrap-placement choice (Option B vs A)
|
||||
- §4.3 rx_list scoping (per-vif)
|
||||
- §4.6 risks #1 (mainline-caller verification) and #5 (beacon path SKB ownership)
|
||||
|
||||
Don't curate.
|
||||
|
||||
### §4.8 Phase 6 implementation order
|
||||
|
||||
1. Branch off cleanups: `bes2600/rx-list-batch-delivery`
|
||||
2. Add `struct list_head rx_list` to `struct bes2600_vif`, `INIT_LIST_HEAD` in vif setup
|
||||
3. Convert all 6 call sites: `ieee80211_rx_irqsafe(...)` → `ieee80211_rx_list(...)`
|
||||
4. Wrap `bes2600_sdio_read_rx_batch` outer loop with `rcu_read_lock + local_bh_disable / local_bh_enable + rcu_read_unlock`
|
||||
5. For the non-bh-thread call sites (ap.c, sta.c, wsm.c beacon): per-call narrow wrap
|
||||
6. Verify beacon path in wsm.c:2415 (Risk #5)
|
||||
7. Build, install, smoke-test
|
||||
8. Phase 7 N=3 stress ramp — compare to v3 baseline
|
||||
|
||||
### §4.9 Phase 7 protocol (per `feedback_phase7_stress_ramp`)
|
||||
|
||||
- N=3 reps, 30s each at 4 MB/s, fresh-chip (uptime <15 min)
|
||||
- Use wired path (`ssh mfritsche@192.168.88.80`) for telemetry
|
||||
- Fresh nc listener per rep (per `feedback_rig_failure_is_finding`)
|
||||
- Compare: throughput delta + tasklet schedule rate (ftrace `irq:tasklet_*` events)
|
||||
- If predicted delta met → close C2 + memory entry
|
||||
- If NO delta → marginal patch but no regression; ship for upstream-cleanliness
|
||||
|
||||
## §5 Out of scope
|
||||
|
||||
- Patch D / E already shipped (PR #7, #8 merged)
|
||||
- Patch G already shipped (PR #6 merged)
|
||||
- bh.c `#if 0` graveyard removal (Task #24 hygiene)
|
||||
- Allwinner `sw_mci_check_r1_ready` (Task #25)
|
||||
|
||||
## §6 Summary
|
||||
|
||||
C2 is a 6-site mechanical migration with ONE design decision (per-batch wrap), TWO open questions for the reviewer (rx_list draining + beacon path SKB ownership), and SMALL expected throughput delta (<15%). Risk-low, upstream-prep-high. Worth shipping for the kernel.org submission story even if the throughput delta is marginal.
|
||||
|
||||
---
|
||||
|
||||
*Plan written 2026-05-08 by Claude (noether). Phase 5 review on PR. Phase 6 contingent on review passing.*
|
||||
@@ -0,0 +1,135 @@
|
||||
# Bug #5 campaign — Phase 1 metric + Phase 2 situation
|
||||
|
||||
Date assembled: 2026-05-07
|
||||
Module under test (baseline): bes2600.ko srcversion `1B3B3ED096AAD7217FEDE11`
|
||||
(cleanups + Patch A + Patch B)
|
||||
|
||||
Phase 0 anchor at N=3 reps (10 min each, 4 MB/s pv-cap on boltzmann → ohm
|
||||
2.4GHz 5b:32) reproduces the throughput regression and traces it to lock-
|
||||
cycling cost in the bes2600 BH path. See `notes/observed-bugs.md` Bug #5
|
||||
for the original report. This document locks Phase 1 and prepares Phase 4
|
||||
candidates.
|
||||
|
||||
---
|
||||
|
||||
## Phase 1 — measurable target (locked)
|
||||
|
||||
> *Reduce the bes2600 BH path's spin-unlock-irqrestore cost so that under sustained 4 MB/s sender pressure on a healthy 2.4GHz link (signal -55 to -65 dBm), ohm sustains ≥ 2 MB/s observed RX throughput (vs 663–725 KB/s baseline at N=3) AND the link survives ≥ 30 min continuous load without cascading into beacon-loss disconnect (vs rep3's failure at ~9 min).*
|
||||
|
||||
Three measurable outcomes, single sentence:
|
||||
|
||||
- **(a) Throughput floor**: ≥ 2 MB/s sustained RX at ohm
|
||||
- **(b) Lock-cycle ceiling**: % CPU in `_raw_spin_unlock_irqrestore` from `bes2600_bh.isra.0` callstack drops to < 10 % (currently ~10 % rep1, ~16 % rep3)
|
||||
- **(c) Cascade prevention**: no link death under continuous 30 min @ 4 MB/s
|
||||
|
||||
---
|
||||
|
||||
## Phase 0 anchor — receipts
|
||||
|
||||
### Reproduction protocol (same units as Phase 7 will use)
|
||||
|
||||
1. boltzmann: `pv -L 4M -q < /dev/zero | nc ohm.fritz.box 12345`
|
||||
2. ohm: `sudo $RUN/rep-trace.sh 600` (10 min capture window)
|
||||
3. Rep dirs: `bug5/rep-<ts>/{mmc.log, perf.data, rx_bytes.tsv, start.txt, end.txt}`
|
||||
4. N=3 reps with 60 s cooldowns
|
||||
|
||||
### Observed (2026-05-07 15:36–16:08)
|
||||
|
||||
| rep | duration | avg KB/s | near-zero ticks | end state |
|
||||
|---|---|---|---|---|
|
||||
| 1 | 600 s | 725 | 1/119 | associated, MCS 6 |
|
||||
| 2 | 600 s | 663 | 5/119 | associated, MCS 4 |
|
||||
| 3 | 600 s | 75 | 53/119 | **passive (link died at sample ~82, ~9 min in)** |
|
||||
|
||||
mmc transaction rate: rep1 = 5793/s sustained, rep3 = 6000/s for first ~10s then collapse to <100/s.
|
||||
|
||||
### Hot-symbol receipts (perf record on `bes_sdio | bes2600_bh` kworkers)
|
||||
|
||||
| symbol | rep 1 (healthy) | rep 3 (cascade) |
|
||||
|---|---|---|
|
||||
| `_raw_spin_unlock_irqrestore` (sum across kworker variants) | **~19 %** | **~21 %** |
|
||||
| `handle_softirqs` | 5.4 % | 4.3 % |
|
||||
| `__tasklet_schedule` | 2.4 % | 2.0 % |
|
||||
| `dw_mci_start_command` (SDIO host) | 1.5 % | < 0.6 % |
|
||||
| `bes2600_sw_retry_requeue` | 0.79 % | 0.70 % |
|
||||
|
||||
Top callchain leading to `_raw_spin_unlock_irqrestore`:
|
||||
|
||||
```
|
||||
process_one_work → worker_thread → wsm_configuration → wsm_cmd_send → bes2600_bh.isra.0
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Phase 2 — situation analysis
|
||||
|
||||
### Relevant source pins
|
||||
|
||||
- `bes2600/wsm.c:98` — `wsm_cmd_send()`, the function in the hot callstack. Body:
|
||||
- holds `&hw_priv->wsm_cmd.lock` (spin) only briefly to fill the cmd struct (lines ~145–152)
|
||||
- calls `bes2600_bh_wakeup()` then `wait_event_interruptible_timeout` for the response
|
||||
- outer lock: `down(&hw_priv->wsm_cmd_sema)` from callers (`wsm_cmd_lock()` at wsm.c:105)
|
||||
- `bes2600/bh.c:435,559,847` — `bes2600_bh_work()` takes `&hw_priv->vif_list_lock` 3× per pass through the dispatcher
|
||||
- `bes2600/bh.c:172,195,487,581,861,1361,1427` — multiple `wait_event*` calls; the BH thread cycles through wait/wake/dispatch
|
||||
|
||||
### What the lock-cycling cost is buying
|
||||
|
||||
Each WSM command from the host (mac80211, NM, kernel scan etc.) goes through the same path:
|
||||
1. caller acquires `wsm_cmd_sema` (outer)
|
||||
2. `wsm_cmd_send()` acquires `wsm_cmd.lock`, fills the struct, releases the lock
|
||||
3. `bes2600_bh_wakeup()` schedules the BH
|
||||
4. BH dispatches the command, takes `vif_list_lock` to walk vifs
|
||||
5. BH talks to chip via SDIO
|
||||
6. response arrives, BH wakes the waiter
|
||||
7. caller releases `wsm_cmd_sema`
|
||||
|
||||
Under sustained TCP RX, mac80211 issues lots of small WSM commands (TX-scheduling hints, rate updates, etc.) — every one cycles through this path. The spin-unlock-irqrestore cost is the floor on this cycle rate.
|
||||
|
||||
### What's been ruled out
|
||||
|
||||
- AP-side bug (AVM Fritz!Box, reliable per Markus's testimony — see `feedback_phase7_stress_ramp.md` reasoning in the campaign-so-far prior).
|
||||
- Patch A / Patch B (target different triggers; would not address lock cost).
|
||||
- Decrypt-failure storm (Patch A handles; this regression occurs in rep1 with zero decrypt-fails).
|
||||
- mac80211 scan-fail / scan-comeback (cosmetic; doesn't account for the throughput floor).
|
||||
|
||||
---
|
||||
|
||||
## Phase 4 — candidate plans (preliminary, not locked)
|
||||
|
||||
Three candidates surfaced from the perf data. Listed cheapest to most-invasive.
|
||||
|
||||
### B5-1 — Reduce `wsm_cmd_send` lock scope
|
||||
|
||||
The spin_lock around the cmd-struct fill (wsm.c:145–152) can probably be replaced with `WRITE_ONCE` of a single sentinel field, since the BH thread reads these fields cooperatively (BH only reads after `bh_wakeup` schedules it, and only writes back via the response path). Eliminates the per-command spin-lock cycle for the host side.
|
||||
|
||||
**Risk**: race with BH if the protocol isn't strictly happens-before. Need to read bh.c:486-500 (where BH reads wsm_cmd.ptr) carefully.
|
||||
|
||||
**Predicted delta**: small but measurable. Maybe 2–3 % CPU off the lock floor.
|
||||
|
||||
### B5-2 — Coalesce `vif_list_lock` in BH dispatcher
|
||||
|
||||
bh.c takes `vif_list_lock` 3× per dispatcher pass. If these 3 critical sections are within a single iteration, they should be merged into one acquire/release.
|
||||
|
||||
**Risk**: vif teardown might depend on releasing the lock between iterations to allow concurrent vif removal. Needs careful audit.
|
||||
|
||||
**Predicted delta**: significant under multi-vif (we're single-vif STA today, so probably small immediate gain).
|
||||
|
||||
### B5-3 — Move WSM send out of process context, use ringbuffer
|
||||
|
||||
Replace the wsm_cmd_sema + wsm_cmd struct mechanism with an SPSC ringbuffer between caller and BH. Caller writes to ring, no lock needed (one producer); BH reads from ring, no lock needed (one consumer).
|
||||
|
||||
**Risk**: significant rework. cw1200 ancestor doesn't have this; we'd be inventing it.
|
||||
|
||||
**Predicted delta**: large — could halve the lock cost. But cost-to-implement is also large.
|
||||
|
||||
---
|
||||
|
||||
## Open question for the reviewer
|
||||
|
||||
Which Phase 4 candidate to lock? My ranking by ROI:
|
||||
|
||||
1. B5-1 (smallest, fastest, cleanest source pin) — try first
|
||||
2. B5-2 (medium, conditional on multi-vif applicability)
|
||||
3. B5-3 (largest rework, biggest potential)
|
||||
|
||||
Or: instrument deeper before committing to a fix (e.g., add `tracepoints around wsm_cmd_send` enter/exit to measure lock holdtime distribution, not just CPU%).
|
||||
@@ -0,0 +1,104 @@
|
||||
# BES2600 WiFi-stability campaign — Phase 4 plan artifact
|
||||
|
||||
Date assembled: 2026-05-06
|
||||
Run dir: /root/bes2600-samples/run-20260506-0659-fresh/ on ohm
|
||||
Module under test: bes2600.ko srcversion 461AFB369355AE598D79BDF (c-final + c5.2.1)
|
||||
|
||||
This is the Phase 4 plan hand-off. Per project CLAUDE.md: paste verbatim, do not curate.
|
||||
|
||||
Drafted after the Phase 5 review of the artifact merged as PR #3 (notes/phase5-2026-05-06.md). Reviewer feedback and a new in-rig finding (Trigger A pinned to mac80211 `api_connection_loss`) are folded into the Phase 1 revision below.
|
||||
|
||||
---
|
||||
|
||||
## Phase 1 — revised metric (folds review feedback + Trigger-A discovery)
|
||||
|
||||
> Per hour of operation, count three event classes:
|
||||
>
|
||||
> (a) `WSM_STATUS_DECRYPTFAILURE` bursts (≥4 events / 60 s)
|
||||
> (b) mac80211 `api_connection_loss` events
|
||||
> (c) AP-side unprotected-deauth-reason-6 frames
|
||||
>
|
||||
> For each, also report the conditional probability that the event escalates to a recovery blackhole > 5 s.
|
||||
|
||||
Reviewer feedback applied:
|
||||
- "Also count AP-deauth" → class (c) added
|
||||
- "N=1 idle is fine" → no further idle reps required to lock
|
||||
- AP-side capture not needed → confirmed; no Fritz!Box logging required
|
||||
|
||||
New finding since the merged Phase 5 artifact (today, ftrace-instrumented suspend/resume reps):
|
||||
|
||||
- **Trigger B (decrypt storm path)**: `decrypt-fail × N → AP unprotected-deauth-6 → kernel local reason-2 deauth`. Receipts: 07:13 (77 events / 24 s), 11:03 (8 events / 9 s), 12:00–13:28 load run (12 bursts).
|
||||
- **Trigger A (beacon-loss path)**: `mac80211 api_connection_loss → kernel local reason-2 deauth`. Receipts: 17:23 and 18:03 today, both following ftrace `api_connection_loss`; yesterday 22:33 (presumed same path; not instrumented at the time).
|
||||
|
||||
---
|
||||
|
||||
## Phase 4 — Plan
|
||||
|
||||
### Patch A — Decrypt-storm fast-recover (Trigger B)
|
||||
|
||||
#### What will be touched
|
||||
|
||||
- `bes2600/txrx.c`, at the `bes_warn` for `WSM_STATUS_DECRYPTFAILURE / goto drop` site (currently line 1696).
|
||||
- A new sliding-window counter on `bes2600_common` (or equivalent struct) tracking decrypt-fail timestamps.
|
||||
- On threshold (proposed: ≥5 within 5 s), `schedule_work(&priv->reassoc_work)` that calls `ieee80211_connection_loss(vif)` so mac80211 enters its clean-reassoc path.
|
||||
- A small struct field for the counter, plus init in probe and reset on assoc.
|
||||
|
||||
#### What will NOT be touched
|
||||
|
||||
- `bes2600/bes2600_sdio.c` bus-level paths (no SDIO change).
|
||||
- Any of the c5.x or c7 stacks (PM, scan defer, LMAC monitor, multi-func reset).
|
||||
- Firmware. The fix is host-side recovery, not chip- or AP-side.
|
||||
- mac80211 / cfg80211 core. Only `ieee80211_connection_loss` is called; no kernel API addition.
|
||||
|
||||
#### Predicted delta on Phase 1 metric (same units as Phase 3 receipts)
|
||||
|
||||
- Decrypt-burst rate **(a)**: UNCHANGED. We don't address the root cause of why decrypts fail; we only catch the storm earlier.
|
||||
- AP-deauth-6 rate **(c)**: DECREASES toward zero, because we pre-empt the AP by initiating a clean reassoc before the AP fires its unprotected deauth. Predicted: c_after / c_before ≤ 0.2.
|
||||
- Conditional probability of >5 s blackhole given a burst: DECREASES from current 100 % (idle baseline, N=1) toward ≤ 10 %. Recovery time falls from 109 s (worst observed) to <5 s.
|
||||
|
||||
Dimensions match Phase 3's idle/load comparison table; numbers are predictions, to be verified in Phase 7.
|
||||
|
||||
#### API contracts to confirm before writing code
|
||||
|
||||
Per project CLAUDE.md "contract before code":
|
||||
|
||||
- `ieee80211_connection_loss(vif)` — semantics + caller-context constraints. Header: `linux/mac80211.h`. Must be called from process context (work item is fine), must NOT be called from interrupt or with rx-skb lock held.
|
||||
- `bes2600_vif` / `bes2600_common` struct fields available for the counter — counter must be safe to update from the `wsm_handle_rx` path.
|
||||
- cw1200 / cw1260 ancestor: any pre-existing storm-recovery logic? If yes, follow that pattern; if no, this is a clean addition.
|
||||
- Existing bes2600 work-item plumbing (e.g., `bes2600_chrdev_do_bus_reset` from c5.2) — same shape, same allocation rules.
|
||||
|
||||
These will be cited in the commit message body and in the patch header comment per Phase 6 rules.
|
||||
|
||||
#### Risk
|
||||
|
||||
- If `ieee80211_connection_loss` is called too aggressively, normal occasional decrypt fails (e.g., one-off MIC failures on bad RX) could trigger spurious reassocs. Threshold (5 in 5 s) is chosen to be stricter than the steady-state decrypt-fail rate observed (60+/h under load ≈ 1/min, never 5/5 s outside a true storm).
|
||||
- If the chip's RX path is the actual cause of the storm, the reassoc will hit the same chip-level issue. The patch may move the symptom from "stuck for 109 s" to "rapidly cycling reassocs". That itself would be visible in Phase 7 measurement.
|
||||
|
||||
---
|
||||
|
||||
### Patch B — Beacon-loss fast-recover (Trigger A) — PARKED
|
||||
|
||||
Not part of this Phase 4. Locked behind one more diagnostic rep:
|
||||
|
||||
- Add to the snap loop (rig is live): track wlan0 station-dump `beacon loss` counter at 10-second cadence (currently 60 s). Want to see the per-tick increase before `api_connection_loss` fires.
|
||||
- Goal: distinguish "chip silently drops beacons" from "real beacon loss in the air" before committing to a host-side patch.
|
||||
- Requires no new instrumentation install — just a snap-loop cadence change. Estimate two reps with this finer cadence will make the picture clean enough.
|
||||
- Once that data lands, Patch B becomes its own Phase 4 plan + Phase 5 review.
|
||||
|
||||
---
|
||||
|
||||
## Receipt checklist for Phase 4
|
||||
|
||||
- [x] What will and will not be touched: stated above
|
||||
- [x] Predicted delta in Phase 3 units: stated above (a/c rate predictions, conditional-probability prediction, recovery-time prediction)
|
||||
- [x] Out-of-scope items explicitly listed
|
||||
- [x] Risk items explicitly listed
|
||||
|
||||
---
|
||||
|
||||
## Asks of the reviewer
|
||||
|
||||
1. Is the threshold (≥5 decrypt-fails in 5 s) the right shape? Should it be more conservative (≥10 in 10 s)? More aggressive (≥3 in 3 s)? The 12 observed bursts ranged from 4 to 9 events per 60 s window (the Phase 1 looser definition). The patch threshold will fire on the same bursts under any of those choices; pick the one most defensible against false positives.
|
||||
2. Is `ieee80211_connection_loss(vif)` the right kernel API? Alternative: `cfg80211_disconnected` with a reason code. Which is cleaner per mac80211 contract for a host-driven preemptive reassoc?
|
||||
3. Should Patch A include a debugfs counter exposing how many storms it has caught, so Phase 7 verification has a host-side counter rather than relying on journal grep alone?
|
||||
4. Patch B parked correctly, or fold it into this same Phase 4?
|
||||
@@ -0,0 +1,153 @@
|
||||
# BES2600 WiFi-stability campaign — Phase 4 plan (Patch B / Trigger A)
|
||||
|
||||
Date assembled: 2026-05-07
|
||||
Run dir: /root/bes2600-samples/run-20260506-2113-patchA/ on ohm
|
||||
Module: bes2600.ko srcversion 21BD07B3782B144D478CE43 (c-stack + Patch A merged)
|
||||
|
||||
This is the Phase 4 plan for **Patch B (Trigger A: beacon-loss / mac80211 `api_connection_loss` chain)**, drafted after the Phase 7 verification of Patch A on 2026-05-07. Per project CLAUDE.md: paste verbatim, do not curate.
|
||||
|
||||
---
|
||||
|
||||
## What changed since the merged Patch-A plan (`notes/phase4-2026-05-06.md`)
|
||||
|
||||
Patch A is **landed (PR #1)** and **active on ohm** (srcversion `21BD07B3`). Phase 7 verification:
|
||||
|
||||
```
|
||||
duration: 10h30m sustained 1 MB/s load on 2.4GHz (5b:32)
|
||||
DecryptStormRecoveries: 0
|
||||
Decrypt-fails total: 183 (~1 every 3.5 min — never bursted ≥5/5s)
|
||||
api_connection_loss events: 9 ← Trigger A
|
||||
unprotected deauth (AP): 7 ← AP-deauth-6 cluster at 02:42:11
|
||||
mac80211 reason 4 deauth: yes ← inactivity (Trigger A flavor)
|
||||
mac80211 reason 2 deauth: no ← what Patch A handles
|
||||
```
|
||||
|
||||
**Patch A's predicted delta is unobserved** (no decrypt-storm fired during Phase 7). Patch A is dormant but caused no harm. This Phase 4 pivots to **Trigger A** — the dominant failure path in the overnight rep.
|
||||
|
||||
---
|
||||
|
||||
## Phase 1 — revised metric (Trigger-A scope)
|
||||
|
||||
> Per hour of operation: count `mac80211 api_connection_loss` events and the conditional probability that each escalates into a > 5 s reauth blackhole (assoc-comeback timeouts followed by AP unprotected-deauth-6 cluster).
|
||||
|
||||
Observed rate from the Phase 7 rep: 9 events over 10h30m = **0.86/h** under sustained load. Not all of them escalated to a visible blackhole — some apparently recovered cleanly. But the 02:42 cluster (1/9 = 11 % escalation rate in this rep) shows the catastrophic shape.
|
||||
|
||||
---
|
||||
|
||||
## Phase 0 / 3 receipt — the 02:42 chain (verbatim from iw-event)
|
||||
|
||||
```
|
||||
02:40:32 scan started, full-band
|
||||
02:40:34 scan aborted
|
||||
02:40:45 del station 5b:32
|
||||
02:40:45 kernel: deauth 8a:2e:77:1f:ec:05 → 5b:32 reason 4
|
||||
(Disassociated due to inactivity) ← TRIGGER A
|
||||
02:40:45 cfg80211: disconnected (local request) reason 4
|
||||
02:40:45 scan started → finished: 2462 2412, "newton"
|
||||
02:40:45 new station 61:b0
|
||||
02:40:45 AP→ohm: auth status 0: Successful
|
||||
02:40:45 AP→ohm: assoc comeback bssid 61:b0 timeout 1000 ← BSS load mgmt
|
||||
02:40:47 del station 61:b0
|
||||
02:40:47 assoc: timed out
|
||||
02:40:47 scan → new station cc:ce:1e:2b:74:17
|
||||
02:40:48 auth: timed out
|
||||
02:40:49 scan → new station 5b:32 (back to where we started)
|
||||
02:40:49 AP→ohm: auth status 0
|
||||
02:40:49 AP→ohm: assoc comeback bssid 5b:32 timeout 881
|
||||
02:40:51 assoc: timed out
|
||||
02:42:11 ── AP-deauth-6 cluster (×7 within 1 ms) from 61:b0 ──
|
||||
reason 6: Class 2 frame received from non-authenticated station
|
||||
02:42:11 reason 9 = STA_REQ_ASSOC_WITHOUT_AUTH
|
||||
```
|
||||
|
||||
**Net**: ~86 s in reauth-blackhole, recovery via cross-channel fallback. Same shape as Phase 3's 11:03 blackhole (~109 s), but trigger here is **inactivity-deauth → assoc-comeback rejection**, not decrypt-storm.
|
||||
|
||||
---
|
||||
|
||||
## Hypothesis on the mechanism
|
||||
|
||||
Three plausible chains for why post-`api_connection_loss` reauth blackholes:
|
||||
|
||||
1. **AP's assoc-comeback timer disrespected.** The AP says "wait 1000 TU before retrying", but mac80211 / wpa_supplicant retries fast. AP keeps deferring; eventually a stale frame triggers the AP's "Class 2 from unauth" reaction.
|
||||
|
||||
2. **Driver state stale across deauth.** After mac80211 fires `ieee80211_connection_loss`, the bes2600 driver's per-vif state (link_id, key state, queues) may not be fully scrubbed. Subsequent reassoc starts with mixed state; AP rejects.
|
||||
|
||||
3. **Chip-level RX wedge.** The chip's RX state machine got stuck during the inactivity period; reauth sends out frames, but RX of AP's responses is lossy. mac80211 perceives timeout when actually frames arrived but were dropped. Hard to verify without monitor mode (which the chip doesn't support concurrent with managed).
|
||||
|
||||
Each hypothesis suggests a different fix surface.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4 — Plan candidates
|
||||
|
||||
### Candidate B-1 — Chip-level reset on api_connection_loss flood
|
||||
|
||||
**What touches:**
|
||||
- New work-item on `bes2600_common`: `api_connection_loss_recover_work`.
|
||||
- mac80211 → driver `disconnect()` op → bump a sliding-window counter.
|
||||
- On threshold (e.g., 3 events within 60 s): schedule the work that calls `bes2600_chrdev_do_bus_reset()` (the existing c5.2 LMAC-wedge recovery path).
|
||||
- After bus reset, NM auto-reconnects from a fresh chip state.
|
||||
|
||||
**Why this candidate:** reuses the c5.2 infrastructure already deployed; small surface; if hypothesis 3 (chip wedge) is right, this fixes the root cause. If hypothesis 1 or 2 are right, this is overkill but harmless (a brief reset).
|
||||
|
||||
**API contracts to confirm:**
|
||||
- `bes2600_chrdev_do_bus_reset()` re-entrancy and worker-context safety
|
||||
- mac80211 ops or callbacks around `ieee80211_connection_loss`/`disconnect`
|
||||
- cw1200/cw1260 ancestor for any similar pattern
|
||||
|
||||
**Predicted delta (Phase 7 units):**
|
||||
- `api_connection_loss` rate: unchanged (we don't address the trigger)
|
||||
- conditional escalation to >5 s blackhole: target ≤ 30 % (need realistic)
|
||||
- worst-case recovery: 86 s → < 10 s
|
||||
|
||||
### Candidate B-2 — Respect assoc-comeback timer
|
||||
|
||||
**What touches:**
|
||||
- Possibly NOT in bes2600 — this looks like a mac80211 / wpa_supplicant concern.
|
||||
- If the driver does anything assoc-related itself, audit for racing the comeback timer.
|
||||
|
||||
**Status:** out of scope for a bes2600 patch unless the driver is observed sending frames during the comeback window.
|
||||
|
||||
### Candidate B-3 — Audit and scrub vif state on disconnect
|
||||
|
||||
**What touches:**
|
||||
- `bes2600_unjoin_work` — verify link_id, keys, queues all reset
|
||||
- Add explicit reset on `ieee80211_disconnect`/`disconnect` ops
|
||||
|
||||
**Status:** speculative without further instrumentation.
|
||||
|
||||
---
|
||||
|
||||
## Lock proposal
|
||||
|
||||
**Lock Candidate B-1 first.** It has:
|
||||
- the cleanest re-use (c5.2's bus_reset)
|
||||
- the smallest patch surface
|
||||
- a measurable predicted delta against Phase 7's `api_connection_loss` rate
|
||||
|
||||
If Phase 7-of-B-1 shows the rate unchanged but escalation still high → loop back to B-2/B-3 hypothesis space.
|
||||
|
||||
---
|
||||
|
||||
## What will NOT be touched
|
||||
|
||||
- mac80211 / cfg80211 core — host-side STA driver only
|
||||
- The c-stack patches (c5.x, c6.x, c7) — independent recovery paths
|
||||
- Patch A — stays in place, untouched
|
||||
- AP / firmware
|
||||
|
||||
---
|
||||
|
||||
## Receipt checklist for Phase 4
|
||||
|
||||
- [x] What will and will not be touched: stated above
|
||||
- [x] Predicted delta in Phase 3 units: stated for Candidate B-1
|
||||
- [x] Out-of-scope items explicitly listed
|
||||
- [x] Risk items: bus_reset has a known multi-function-SDIO consideration (handled by c5.2.1)
|
||||
|
||||
## Asks of the reviewer
|
||||
|
||||
1. Candidate B-1 (bus_reset on api_connection_loss flood) the right scope, or should we instrument deeper before committing?
|
||||
2. Threshold (3 events / 60 s): too aggressive (false-positive bus_resets on transient RF issues) or about right?
|
||||
3. Should bus_reset be conditional on ALSO seeing post-deauth assoc-comeback timeouts, to avoid resetting on benign connection_loss events?
|
||||
4. Hypothesis 1 (assoc-comeback disrespected) — is this a mac80211/wpa_supplicant bug rather than a bes2600 bug? If yes, we file it elsewhere.
|
||||
@@ -0,0 +1,96 @@
|
||||
# BES2600 WiFi-stability campaign — Phase 7 verdict (Patches A + B)
|
||||
|
||||
Date assembled: 2026-05-07
|
||||
Module under test: bes2600.ko srcversion `1B3B3ED096AAD7217FEDE11`
|
||||
(cleanups + Patch A + Patch B)
|
||||
Run dir: `/root/bes2600-samples/run-20260507-1248-patchB/` on ohm
|
||||
|
||||
Phase 7 verification window: 2026-05-07 12:48 → ~15:13 CEST (≈ 2 h 25 m)
|
||||
of which: ~50 min @ 1 MB/s pv-cap, ~1 h 30 m @ 4 MB/s pv-cap on 2.4 GHz
|
||||
newton (5b:32, signal -57 to -67 dBm).
|
||||
|
||||
---
|
||||
|
||||
## Result table (vs the Phase 4 predicted delta)
|
||||
|
||||
### Patch A — decrypt-storm fast-recover (Trigger B)
|
||||
|
||||
| metric | Phase 3 baseline | Phase 4 prediction | Phase 7-of-B observed |
|
||||
|---|---|---|---|
|
||||
| decrypt-burst rate | 8.18/h | unchanged | 2 bursts in ~22 min once 4MB/s pressure was on |
|
||||
| AP-deauth-6 rate following burst | 100 % escalation | ≤ 0.2 × baseline | **0/2 = 0 % escalation** |
|
||||
| recovery time given burst | up to 109 s | < 5 s | **~1 s** (×2) |
|
||||
|
||||
**Verdict: predicted delta CONFIRMED at N=2.** CLAUDE.md ideal is N=3; we're directionally locked at 2 reproductions, both behaving as predicted (threshold trip → `[bes2600] decrypt-storm fast-recover: forcing reassoc` log line → mac80211 disassoc → userspace reauth in ≈1 s).
|
||||
|
||||
#### Receipts (verbatim)
|
||||
|
||||
```
|
||||
13:47:56 bes2600_wlan: [bes2600] decrypt-storm fast-recover: forcing reassoc
|
||||
13:47:57 wlan0: associated to cc:ce:1e:2b:74:17 (cross-BSSID, 1 s)
|
||||
13:49:26 bes2600_wlan: [bes2600] decrypt-storm fast-recover: forcing reassoc
|
||||
13:49:27 wlan0: associated to c0:25:06:e6:5b:32 (back home, 1 s)
|
||||
```
|
||||
|
||||
`DecryptStormRecoveries: 2` exposed via debugfs at `/sys/kernel/debug/ieee80211/phy0/bes2600/vif_0/status`.
|
||||
|
||||
### Patch B — connection-loss-storm bus_reset (Trigger A)
|
||||
|
||||
| metric | Phase 7-of-A observed | Phase 4 prediction | Phase 7-of-B observed |
|
||||
|---|---|---|---|
|
||||
| api_connection_loss rate | 0.86/h | unchanged | 2 events in ~2 h (≈ 1/h) |
|
||||
| ConnectionLossStormRecoveries | n/a | trips on 3-in-60s bursts | **0** |
|
||||
| Threshold trip events | n/a | (when burst occurs) | **0** (events spaced 91 s apart) |
|
||||
|
||||
**Verdict: installed but UNTRIGGERED.** The 3-in-60s threshold was never reached (max-cluster observed: 2-in-91s). No false positives, no spurious bus_resets. Predicted delta unobserved — same shape as Patch A's first Phase 7 run.
|
||||
|
||||
The threshold may be too conservative for typical event rates (we'd need a true api_connection_loss flood to trip it). Tuning is a future Phase-1 question if more reproductions accumulate.
|
||||
|
||||
### Trigger C — AP unprotected-deauth-6 cluster without preceding storm
|
||||
|
||||
```
|
||||
12:53:10.475 → 12:53:11.756 AP fires 17 unprotected-deauth-6 from 5b:32 over 1.3 s
|
||||
(2 mgmt-TX no-ack from our chip in the middle)
|
||||
12:53:12.309 kernel: deauthenticating ... reason 2 = PREV_AUTH_NOT_VALID
|
||||
12:53:14–15 reauth via 61:b0 → 5b:32, recovery in ~4 s
|
||||
```
|
||||
|
||||
Neither Patch A (zero decrypt-fails preceded) nor Patch B (zero api_connection_loss) fired. Background: AVM Fritz!Boxes (newton) are reliable; the AP correctly classified ohm's frames as Class 2 from non-auth, meaning **bes2600 sent something the AP couldn't authenticate**. New backlog entry: `notes/observed-bugs.md` Bug #5 (RX path under throughput pressure) is the leading hypothesis surface.
|
||||
|
||||
Recovery was fast (4 s) so this isn't a P0 — but a Patch C investigation is warranted when prioritized.
|
||||
|
||||
---
|
||||
|
||||
## Bug #5 — RX path degradation under attempted-throughput pressure (NEW)
|
||||
|
||||
```
|
||||
sender 1 MB/s → ohm receives 1015 KB/s, -57 dBm, RX MCS 4
|
||||
sender 4 MB/s → ohm receives 563 KB/s, -67 dBm, RX MCS 3
|
||||
```
|
||||
|
||||
Higher attempted-throughput on the sender side → LOWER observed throughput at ohm. Signal degraded ~10 dB, MCS dropped a notch. Link-physical max is ~8 MB/s; we're getting ~7 % of that under load.
|
||||
|
||||
**Hypothesis (Markus): driver/firmware locks itself to death under busy reads.** Plausibly the same root-cause as the Phase 0 YouTube DASH chunk-fetch drops (~10 frames per chunk fetch on hardware-decoder playback). Documented as Bug #5 in `notes/observed-bugs.md`.
|
||||
|
||||
---
|
||||
|
||||
## Lessons captured for memory (Phase 8 anchor)
|
||||
|
||||
1. **Stress-rate matters for verification.** Patch A's predicted delta only became observable when the netcat cap went 1 → 4 MB/s. The previous Phase 7 (10h30m @ 1 MB/s) saw zero decrypt-storms. Future Phase 7 protocols should plan a stress ramp from steady to near-saturation, not just the steady setting.
|
||||
2. **"Untriggered, no harm" is a valid Phase 7 verdict** for installed patches. Patch B fits this exactly. The patch is ready; the trigger pattern just doesn't fire often enough in this RF / load regime to verify the recovery delta. Don't let unobserved verifications block the loop.
|
||||
3. **Build infrastructure on `cleanups` not `mobian`.** The Phase 6 attempt to base Patch B on mobian forced a refactor mid-flight; the c-stack lives on cleanups, and re-using c5.2's `bes2600_chrdev_do_bus_reset` requires that. The cleanups branch is the campaign's working trunk.
|
||||
4. **AP-side bug is unlikely on AVM hardware.** AVM Fritz!Boxes don't fire spurious deauth-6 storms. When ohm sees AP-deauth-6 unprovoked, the suspect chain is bes2600 sending something the AP can't authenticate. The bias toward "bes2600 is the broken thing" is empirically validated.
|
||||
5. **AP-deauth-6 can fire without our local triggers.** Trigger C is a real failure mode neither Patch A nor B addresses. Adding a Phase-1-style metric for "AP-deauth-6 rate without preceding decrypt-storm or api_connection_loss" would surface Trigger C cleanly.
|
||||
6. **`pv -L` cap interacts with TCP retransmit recovery.** When the link can't sustain the cap, TCP backs off and pv blocks. Observed throughput is then a **floor on chip RX capacity at that signal level**, not the sender's intent. Useful for chip-load-characterization, but the cap should be set based on observed pull-rate, not on the link's nominal MCS rate.
|
||||
|
||||
---
|
||||
|
||||
## Loop status
|
||||
|
||||
- Phase 7: closed.
|
||||
- Patch A: confirmed (N=2). Stays in.
|
||||
- Patch B: installed, dormant in this regime, no harm. Stays in.
|
||||
- Bug #5: backlog, no patch yet. Documented.
|
||||
- Trigger C: backlog candidate, no patch yet. Documented.
|
||||
|
||||
Next campaign cycle would be re-anchoring Phase 0 around Bug #5 or Trigger C.
|
||||
@@ -0,0 +1,63 @@
|
||||
# Patch C2 Phase 7 — N=3 ramp results
|
||||
|
||||
**Date:** 2026-05-08
|
||||
**Module:** `bes2600.ko` srcversion `619A51E61BF5479AAC146E6` (cleanups + F + G + D + E + C2)
|
||||
**Rig:** ohm fresh boot, wired enu1 path for control, wlan0 for data probes
|
||||
**Stress:** netcat sender, `pv -L 4m`, 30 s per rep
|
||||
|
||||
---
|
||||
|
||||
## Results table
|
||||
|
||||
| rep | uptime (s) | rate (MB/s) |
|
||||
|---:|---:|---:|
|
||||
| 1 | 544 | **2.289** |
|
||||
| 2 | 716 | **2.165** |
|
||||
| 3 | 750 | **2.376** |
|
||||
|
||||
**N=3:** mean 2.277, median 2.289, min 2.165, max 2.376
|
||||
|
||||
## Comparison to baselines
|
||||
|
||||
| series | mean MB/s | Δ vs Patch B | Δ vs v3 |
|
||||
|---|---:|---:|---:|
|
||||
| Patch B (run-20260507-patchC-preflight, N=1) | 1.362 | — | -42% |
|
||||
| Patch C v3 N=3 (run-20260507-N3v3-rep*) | 2.352 | +73% | — |
|
||||
| Patch C v3 + F + G + D + E + C2 N=3 (this rep set) | 2.277 | +67% | -3% |
|
||||
|
||||
Δ vs v3 is **within rep variance** (v3 N=3 had min 2.102, max 2.590 → spread ±20%; this set's spread is similar). Statistically indistinguishable.
|
||||
|
||||
## Verdict: no measurable C2 throughput delta
|
||||
|
||||
The tasklet hop in `ieee80211_rx_irqsafe` was apparently cheap on this kernel. Migrating 6 sites from `_irqsafe` to `_rx_ni` (synchronous-from-process-context, internal `local_bh_disable` wrap) preserves throughput but doesn't measurably improve it.
|
||||
|
||||
**This was a predicted outcome.** The C2 Phase 4 plan §4.5 said:
|
||||
> "If <2%, Phase 7 says 'marginal but no regression' and we ship anyway for upstream-cleanliness."
|
||||
|
||||
Observed: -3% (within noise) → falls into the "marginal but no regression" bucket. Ship for the kernel.org submission story (no `_irqsafe` from process context = upstream-idiomatic) even though performance is unchanged.
|
||||
|
||||
## Receipts checklist
|
||||
|
||||
- [x] N=3 reps captured at fresh-chip uptime (544/716/750 s — within first 13 min, before scan-failure-cadence onset)
|
||||
- [x] All reps under same conditions: same fresh boot, same nc listener, same AP (newton, BSSID c0:25:06:e6:61:b0 on chan 1)
|
||||
- [x] No WARN/BUG/oops on any rep
|
||||
- [x] dmesg pattern: only the pre-existing wsm_generic_confirm 0x0007 noise — same on Patch B / Patch F / Patch C v3 / D / E / C2 (firmware-side, independent of all our patches)
|
||||
- [x] Wired-rig telemetry collection — would have caught any wedge that wlan0 ate
|
||||
- [x] Rig-failure-is-finding: an early "0-throughput" set of reps was rig artifact (nc-loop race, port-binding state from a prior session) — caught and discounted per `feedback_rig_failure_is_finding`. The recovered N=3 reps used setsid-detached listener + post-reboot fresh state.
|
||||
|
||||
## Phase 8 lesson
|
||||
|
||||
**Drop-in replacements with the right kerneldoc reading still need Phase 7 measurement.** I expected +5-15% from removing the tasklet schedule. Got -3% (noise). The cost we were saving was already amortised by something else (NAPI infra? per-CPU softirq scheduling?). The kerneldoc-correctness story stands; the perf story does not.
|
||||
|
||||
**Memory entry:** the perf-vs-correctness distinction is worth keeping. `_irqsafe → _rx_ni` is a CORRECTNESS / API-cleanliness move, not a performance optimization. Don't oversell predicted deltas without baseline measurement.
|
||||
|
||||
## Out-of-scope follow-ups
|
||||
|
||||
- Patch C v3 architectural win is the durable +73%. C / D / E / C2 / F / G are smaller cleanups that don't compound visibly.
|
||||
- Bug #5 RX-degradation campaign already closed (hypothesis falsified).
|
||||
- Task #24 (post-cleanup observation of bh.c symptom-shaped artifacts): mostly answered.
|
||||
- Task #25 (Allwinner sw_mci_check_r1_ready measurement): can be done during any future stress run; not on critical path.
|
||||
|
||||
---
|
||||
|
||||
*Phase 7 captured 2026-05-08 by Claude (noether). Patch C2 closes the post-Bug-#5 cleanup track. Throughput ceiling on this hardware = ~2.4 MB/s sustained @ 4 MB/s sender, fresh chip; further improvement would need firmware-side fixes (the wsm_generic_confirm 0x0007 path), not driver-side.*
|
||||
@@ -0,0 +1,94 @@
|
||||
# Patch C v3 Phase 7 — N=3 verification results
|
||||
|
||||
**Date:** 2026-05-07
|
||||
**Module:** `bes2600.ko` srcversion `371C6606B73AF19299228CA` (cleanups+F+v3)
|
||||
**Rig:** ohm (PineTab2, RK3566 + BES2600 SDIO), wired enu1 path for telemetry
|
||||
**Stress:** netcat sender from boltzmann, `pv -L 4m` rate cap (4 MB/s), 3-min window per rep
|
||||
**Boot:** fresh — uptime 200 s / 391 s / 582 s at rep 1/2/3 starts (all within fresh-chip window before the ~13-min Bug #5 RX-degradation point)
|
||||
|
||||
---
|
||||
|
||||
## Results table
|
||||
|
||||
| rep | elapsed (s) | RX bytes | RX MB | MB/s | sdio_rx_work | sdio_tx_work | bes2600_bh_work redispatches |
|
||||
|---:|---:|---:|---:|---:|---:|---:|---:|
|
||||
| 1 | 180.72 | 447,758,333 | 427.0 | **2.363** | 0 | 368 | 0 |
|
||||
| 2 | 180.67 | 490,669,836 | 467.9 | **2.590** | 0 | 20 | 0 |
|
||||
| 3 | 180.69 | 398,224,992 | 379.8 | **2.102** | 0 | 39 | 0 |
|
||||
|
||||
**N=3 stats:** mean 2.352 MB/s · median 2.363 MB/s · min 2.102 MB/s · max 2.590 MB/s
|
||||
|
||||
## Comparison to baselines
|
||||
|
||||
### vs Patch B baseline (`run-20260507-patchC-preflight`, N=1, 5 min @ 4 MB/s, fresh chip)
|
||||
|
||||
| | Patch B | v3 mean | Δ |
|
||||
|---|---:|---:|---:|
|
||||
| throughput | 1.362 MB/s | 2.352 MB/s | **+73%** |
|
||||
|
||||
### vs original Bug #5 baseline (`run-20260506-0659-fresh`, N=3, decay over time)
|
||||
|
||||
Bug #5 anchor was 725 / 663 / **75** KB/s — rep 3 saw link-death at ~9 min.
|
||||
|
||||
| | Bug #5 floor (rep 3) | v3 floor (rep 3) | Δ |
|
||||
|---|---:|---:|---:|
|
||||
| throughput | 0.075 MB/s | 2.102 MB/s | **28× improvement** |
|
||||
|
||||
### vs Phase 4 v3 plan §4.5 predictions
|
||||
|
||||
| metric | predicted | observed | verdict |
|
||||
|---|---|---|---|
|
||||
| sdio_rx_work dispatch rate | → 0/s (high confidence) | 0/s all 3 reps | ✅ |
|
||||
| `bes2600_bh_work` redispatches | → 0 (high confidence) | 0 all 3 reps | ✅ |
|
||||
| observed RX @ 4 MB/s | floor lifts toward ≥ 1 MB/s sustained (medium) | 2.10 MB/s floor | ✅ exceeds prediction |
|
||||
| `_raw_spin_unlock_irqrestore` CPU% | 20% → 12-15% (medium) | not measured | deferred — perf-record run can confirm |
|
||||
|
||||
## Workqueue dispatch rate collapse
|
||||
|
||||
Patch B baseline (per `run-20260507-patchC-preflight`):
|
||||
- sdio_rx_work: 86.4/s
|
||||
- sdio_tx_work: 276.1/s
|
||||
- bes2600_bh_work redispatches: 0
|
||||
|
||||
v3 N=3 mean:
|
||||
- **sdio_rx_work: 0.0/s** (function deleted)
|
||||
- **sdio_tx_work: 0.8/s** (post-tx queue_work → self->irq_handler call; the chip-side TX driver no longer needs to wake a separate workqueue)
|
||||
- bes2600_bh_work redispatches: 0 (preserved invariant; bh thread still single long-lived work item)
|
||||
|
||||
The 99.7% reduction in `sdio_tx_work` dispatch rate is a side-effect of v3's IRQ→bh-direct rewiring: the post-TX `queue_work(self->sdio_wq, &self->rx_work)` call I replaced with `self->irq_handler()` was actually firing more often than I'd assumed (276/s on Patch B). Folding it into the bh wake-up cuts 275/s of workqueue dispatches that weren't doing anything useful.
|
||||
|
||||
## Risks observed
|
||||
|
||||
- **Bug #5 RX-degradation after ~13-min uptime is independent of v3.** Same scan-failure pattern observed (`wsm_generic_confirm failed for request 0x0007` + `[SCAN] Scan failed (-22)` every 300s) on v3 as on Patch B. v3 did NOT fix Bug #5; it fixed the v2-race that was ALSO present. RX-degradation is firmware-side, likely needs a separate campaign.
|
||||
- **N=3 reps were 3 minutes each instead of 5** to fit within the fresh-chip window. Direct comparison with Patch B's 5-min baseline is approximate; chip-side throughput in 3-min vs 5-min should be similar given the bug fires on uptime, not on transferred-bytes.
|
||||
- **No regression observed in 3×3 min = 9 min of stress.** The v2 race that wedged Patch C v1 within 13 s did NOT reproduce. v3's structural fix held.
|
||||
|
||||
## Phase 8 — lesson distilled
|
||||
|
||||
**The cw1200 mining was decisive.** Patch C v2 (atomic_t prep + direct-deliver on top of relay, PR #10 closed) would have worked correctly but kept the structural relay that was the source of the race. v3 removed the relay entirely — restoring single-writer-from-bh invariant by construction, no atomic_t needed, and delivering a 73% throughput improvement as side benefit.
|
||||
|
||||
Without the cw1200 history mine (`~/src/linux-rockchip`, 228 cw1200 commits over 16 years), v2's atomic_t prep would have shipped. The structural fix is upstream-grade because it matches the reference driver. v2's atomic_t wrapper would have been bes2600-specific bookkeeping with no upstream parallel — defensible as a fix, but worse to maintain.
|
||||
|
||||
**Memory entry:** *When you have an upstream-ancestral driver still in the kernel tree, mine its bug-fix history before patching the inherited fork. The architectural answer may already be there; you just have to look.*
|
||||
|
||||
## Receipts checklist (Phase 7 done)
|
||||
|
||||
- [x] N=3 reps captured at fresh-chip uptime (200/391/582 s)
|
||||
- [x] Same instrumentation pre/post (workqueue ftrace + rx_packets/rx_bytes counters)
|
||||
- [x] Predicted delta matched (sdio_rx_work → 0; bh redispatches → 0; throughput ≥ 1 MB/s sustained)
|
||||
- [x] No WARN/BUG/oops during stress on any rep
|
||||
- [x] Wired-rig telemetry collection (would have caught a wedge if v3 had one)
|
||||
- [x] Receiver `nc` listener restarted fresh per rep (avoiding rep-2-style TCP race)
|
||||
- [x] Stress-ramp memory honored: not steady-state low-rate; saw 4 MB/s saturate
|
||||
|
||||
## Out-of-scope follow-ups
|
||||
|
||||
- Patch C2 — `ieee80211_rx_list` batch delivery — gated on Task #19 kerneldoc verification.
|
||||
- Patch D — ba_lock atomicization — independent.
|
||||
- Patch E — ps_state_lock skip when pm_unsupported — independent.
|
||||
- Bug #5 RX-degradation after 13-min uptime — separate campaign, scan-failure pattern is the entry point.
|
||||
- Task #24 — observe whether `bh.c` `asm volatile("nop")` / commented-out `__bes2600_irq_enable(1)` / BUG_ON in hot path are still load-bearing post-v3. Already partially answered: `__bes2600_irq_enable` is a stub (PR #11 comment). The other artifacts can be re-read fresh.
|
||||
|
||||
---
|
||||
|
||||
*Phase 7 results captured 2026-05-07 by Claude (noether). v3 (PR #5) closes Patch C campaign with structural improvement + race fix + measurable throughput win.*
|
||||
Reference in New Issue
Block a user