marfrit/bes2600-dkms
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bes2600: Patch C2 — replace ieee80211_rx_irqsafe with ieee80211_rx_ni #9

Merged
marfrit merged 1 commits from bes2600/rx-list-batch-delivery into cleanups 2026-05-08 04:43:15 +00:00
Conversation 0 Commits 1 Files Changed 4
+18 -7
claude-noether commented 2026-05-08 04:41:59 +00:00
Collaborator
Copy Link
Copy Source

Per Phase 4 plan (besser PR #14 merged) + kerneldoc audit task #19. 6 call sites in bes2600 deferred per-RX-frame mac80211 dispatch via tasklet (ieee80211_rx_irqsafe); replace with synchronous-from-process-context API ieee80211_rx_ni which does its own local_bh_disable wrap.

Why _ni and not _list

The Phase 4 plan originally targeted ieee80211_rx_list for batch delivery. Mining mt76 mainline (the only driver upstream using _list) showed the canonical pattern requires threading a struct list_head through the per-frame call chain.

bes2600's WSM dispatcher (wsm_handle_rx → bes2600_rx_cb / wsm.c beacon path) sits between the bh thread's SDIO read and the mac80211 hand-off; threading a list_head through the dispatcher is a non-trivial refactor. ieee80211_rx_ni() is the simpler drop-in: no list management, still removes the tasklet hop. Per-call local_bh_disable cost is trivial vs the saved tasklet schedule. Future refactor can revisit _list if measurements warrant.

Sites converted (6 total)

site context extra work
ap.c:96 bes2600_sta_add link-id rx_queue drain splice-out-of-lock refactor — was inside spin_lock_bh(&ps_state_lock), would otherwise hold the lock across full mac80211 dispatch
sta.c:1487 deauth-frame inject in inactivity event direct conversion (not under lock)
txrx.c:1960 early-data + pm_unsupported branch (Patch E gated path) direct
txrx.c:1967 early-data + LINK_SOFT-not-set branch direct
txrx.c:1971 normal RX path in bes2600_rx_cb direct
wsm.c:2415 beacon delivery in scan-complete WSM handler beacon SKB ownership preserved by existing skb_copy → beacon_bkp pattern; no lifecycle change (Phase 4 plan §6 risk #5 cleared)

ap.c:96 splice refactor

The original code drained entry->rx_queue with _irqsafe calls inside a spin_lock_bh. _irqsafe deferred to a tasklet (no actual mac80211 work under the lock). After conversion to _ni, mac80211's full RX dispatch runs synchronously — would hold ps_state_lock across that. Refactor:

struct sk_buff_head local_drain;
__skb_queue_head_init(&local_drain);

spin_lock_bh(&priv->ps_state_lock);
... entry->status = BES2600_LINK_HARD; ...
skb_queue_splice_init(&entry->rx_queue, &local_drain);
spin_unlock_bh(&priv->ps_state_lock);
while ((skb = __skb_dequeue(&local_drain)))
    ieee80211_rx_ni(priv->hw, skb);

Mixing constraint

Kerneldoc (include/net/mac80211.h:5399-5430): ieee80211_rx_ni() cannot mix with ieee80211_rx_irqsafe() for a single hardware. All 6 sites convert atomically; no mixed state.

Diff

4 files (ap.c, sta.c, txrx.c, wsm.c), +18/-7 net.

Build verified

bes2600.ko builds clean on ohm sandbox: srcversion 619A51E61BF5479AAC146E6, 0 warnings, 0 errors.

Predicted Phase 7

+5-15% over post-D+E baseline (single-rep was 3.22 MB/s; v3 N=3 mean was 2.35 MB/s). Modest improvement from removing the tasklet schedule per RX frame. Smaller delta still a net win for upstream-cleanliness — kernel.org submission story benefits from not using _irqsafe from process context.

Test plan

  • Reload module on ohm (reboot per dev-allocation override)
  • Smoke-test association + scan + traffic
  • N=3 stress @ 4 MB/s; compare to D+E baseline
  • Fallback rollback: /var/tmp/bes2600.patchCv3.rollback.ko on ohm
Per Phase 4 plan (besser PR #14 merged) + kerneldoc audit task #19. 6 call sites in bes2600 deferred per-RX-frame mac80211 dispatch via tasklet (`ieee80211_rx_irqsafe`); replace with synchronous-from-process-context API `ieee80211_rx_ni` which does its own `local_bh_disable` wrap. ## Why `_ni` and not `_list` The Phase 4 plan originally targeted `ieee80211_rx_list` for batch delivery. Mining mt76 mainline (the only driver upstream using `_list`) showed the canonical pattern requires threading a `struct list_head` through the per-frame call chain. bes2600's WSM dispatcher (`wsm_handle_rx` → `bes2600_rx_cb` / `wsm.c` beacon path) sits between the bh thread's SDIO read and the mac80211 hand-off; threading a `list_head` through the dispatcher is a non-trivial refactor. `ieee80211_rx_ni()` is the simpler drop-in: no list management, still removes the tasklet hop. Per-call `local_bh_disable` cost is trivial vs the saved tasklet schedule. Future refactor can revisit `_list` if measurements warrant. ## Sites converted (6 total) | site | context | extra work | |---|---|---| | ap.c:96 | `bes2600_sta_add` link-id rx_queue drain | **splice-out-of-lock refactor** — was inside `spin_lock_bh(&ps_state_lock)`, would otherwise hold the lock across full mac80211 dispatch | | sta.c:1487 | deauth-frame inject in inactivity event | direct conversion (not under lock) | | txrx.c:1960 | early-data + pm_unsupported branch (Patch E gated path) | direct | | txrx.c:1967 | early-data + LINK_SOFT-not-set branch | direct | | txrx.c:1971 | normal RX path in `bes2600_rx_cb` | direct | | wsm.c:2415 | beacon delivery in scan-complete WSM handler | beacon SKB ownership preserved by existing `skb_copy → beacon_bkp` pattern; **no lifecycle change** (Phase 4 plan §6 risk #5 cleared) | ## ap.c:96 splice refactor The original code drained `entry->rx_queue` with `_irqsafe` calls inside a spin_lock_bh. `_irqsafe` deferred to a tasklet (no actual mac80211 work under the lock). After conversion to `_ni`, mac80211's full RX dispatch runs synchronously — would hold ps_state_lock across that. Refactor: ```c struct sk_buff_head local_drain; __skb_queue_head_init(&local_drain); spin_lock_bh(&priv->ps_state_lock); ... entry->status = BES2600_LINK_HARD; ... skb_queue_splice_init(&entry->rx_queue, &local_drain); spin_unlock_bh(&priv->ps_state_lock); while ((skb = __skb_dequeue(&local_drain))) ieee80211_rx_ni(priv->hw, skb); ``` ## Mixing constraint Kerneldoc (`include/net/mac80211.h:5399-5430`): `ieee80211_rx_ni()` cannot mix with `ieee80211_rx_irqsafe()` for a single hardware. All 6 sites convert atomically; no mixed state. ## Diff 4 files (ap.c, sta.c, txrx.c, wsm.c), +18/-7 net. ## Build verified `bes2600.ko` builds clean on ohm sandbox: srcversion **`619A51E61BF5479AAC146E6`**, 0 warnings, 0 errors. ## Predicted Phase 7 +5-15% over post-D+E baseline (single-rep was 3.22 MB/s; v3 N=3 mean was 2.35 MB/s). Modest improvement from removing the tasklet schedule per RX frame. Smaller delta still a net win for upstream-cleanliness — kernel.org submission story benefits from not using `_irqsafe` from process context. ## Test plan - [ ] Reload module on ohm (reboot per dev-allocation override) - [ ] Smoke-test association + scan + traffic - [ ] N=3 stress @ 4 MB/s; compare to D+E baseline - [ ] Fallback rollback: `/var/tmp/bes2600.patchCv3.rollback.ko` on ohm
claude-noether added 1 commit 2026-05-08 04:42:00 +00:00
bes2600: Patch C2 — replace ieee80211_rx_irqsafe with ieee80211_rx_ni 0ec58c0ad5 
Per Phase 4 plan PR #14 + kerneldoc audit (Task #19).  Six call sites
deferred per-RX-frame mac80211 dispatch via tasklet; replace with the
synchronous-from-process-context API ieee80211_rx_ni() which does its
own local_bh_disable wrap.

Why _ni and not _list:

  Phase 4 plan originally targeted ieee80211_rx_list for batch
  delivery.  Mining mt76 mainline (the only driver using _list)
  showed the canonical pattern requires threading a struct list_head
  through the per-frame call chain.  bes2600s WSM dispatcher
  (wsm_handle_rx -> bes2600_rx_cb / wsm.c beacon path) sits between
  the bh threads SDIO read and the mac80211 hand-off; threading a
  list_head through the dispatcher is a non-trivial refactor.
  ieee80211_rx_ni() is the simpler drop-in: no list management, still
  removes the tasklet hop.  Per-call local_bh_disable cost is trivial
  vs the saved tasklet schedule.  Future refactor can revisit _list
  if measurements warrant.

Sites converted:

  - ap.c:96       (bes2600_sta_add link-id rx_queue drain on AP-mode
                   STA add).  Was inside spin_lock_bh(&ps_state_lock);
                   refactored to splice the queue under the lock then
                   deliver after unlock — _ni runs the synchronous
                   mac80211 RX path inline, would otherwise hold the
                   lock across mac80211 dispatch.  splice via
                   skb_queue_splice_init into a local sk_buff_head.
  - sta.c:1487    (deauth-frame inject in inactivity-event handler).
                   Not under any lock; direct conversion.
  - txrx.c:1960   (early-data + pm_unsupported branch from Patch E).
  - txrx.c:1967   (early-data + LINK_SOFT-not-set branch).
  - txrx.c:1971   (normal RX path in bes2600_rx_cb).
  - wsm.c:2415    (beacon delivery in scan-complete WSM handler).
                   beacon SKB ownership is preserved by the existing
                   skb_copy(beacon, GFP_ATOMIC) -> beacon_bkp pattern;
                   no lifecycle change needed.

Mixing constraint (kerneldoc include/net/mac80211.h:5399-5430):
ieee80211_rx_ni() cannot mix with ieee80211_rx_irqsafe() for a
single hardware.  All 6 sites convert atomically; no mixed state.

Build verified clean on ohm sandbox: srcversion 619A51E61BF5479AAC146E6.

Predicted Phase 7 delta: +5-15% over v3+D+E baseline (2.35 MB/s mean
on v3 alone; D+E single-rep was 3.22 MB/s).  Modest improvement
expected from removing the tasklet schedule per RX frame.  Smaller
deltas would still be a net win for upstream-cleanliness — the
kernel.org submission story benefits from not using _irqsafe from
process context.
marfrit merged commit 0750df2611 into cleanups 2026-05-08 04:43:15 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claude-noether (Claude (noether)) marfrit (Markus Fritsche)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marfrit/bes2600-dkms#9
Delete Branch "bes2600/rx-list-batch-delivery"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?