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

bes2600: Patch D — atomicize ba_lock counters, drop the spinlock

Browse Source 
The block-ack policy uses 4 int counters (ba_acc, ba_cnt, ba_acc_rx,
ba_cnt_rx) bumped per data frame in the TX and RX hot paths under
spin_lock_bh(&hw_priv->ba_lock).  The lock was the heaviest per-frame
synchronization cost remaining after Patch C v3 (which fixed the
sdio_rx_work relay).  Per the Opus structural critique (PR #8), this
pattern matches mac80211 driver convention for per-frame statistics:
atomic_t suffices, no lock needed.

Field-by-field changes in struct bes2600_common:
  ba_acc, ba_cnt, ba_acc_rx, ba_cnt_rx: int -> atomic_t
  ba_armed:                              new atomic_t (timer-arm flag)
  ba_ena:                                bool -> atomic_t
  ba_lock:                               removed (spinlock_t deleted)
  ba_hist:                               int (single-writer = ba_timer)

Producer hot path (txrx.c TX submit + RX receive):
  - atomic_add for the byte accumulator
  - atomic_inc for the frame counter
  - atomic_cmpxchg(&ba_armed, 0, 1) to claim the once-per-window
    mod_timer arm — at most ONE producer succeeds; race-free
  - no spin_lock_bh

Consumer paths (sta.c bes2600_ba_timer, sta.c disconnect-reset, sta.c
bes2600_ba_work, debug.c debugfs reader):
  - atomic_read snapshots all 4 counters into locals; the threshold
    predicate (acc/cnt >= THLD) tolerates approximate snapshots — the
    timer fires periodically, a single misclassification just delays
    the policy update by one tick
  - atomic_set zeroes the counters at end of timer-callback window;
    racing producer increments after the snapshot are lost (acceptable
    for stats; same approximation the original lock allowed under
    contention)
  - atomic_set(&ba_armed, 0) re-enables the next window's arm

Followup-amenable simplification: ba_hist remains int because only
the single ba_timer callback writes it; multiple writers would need
to upgrade it too.

This patch follows the cw1200-mainline-idiom established by Patch C v3
(structural fix, not bandaid).  The cw1200 reference doesn't have a
similar lock to compare; bes2600 inherited this from a later
Bestechnic addition rather than the upstream tree.
This commit is contained in:
Markus Fritsche
2026-05-08 00:17:46 +02:00
parent a02f8b7629
commit 93f2aab656
5 changed files with 85 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bes2600/debug.c
+8 -5
View File
@@ -110,17 +110,20 @@ static int bes2600_status_show_common(struct seq_file *seq, void *v)
int ba_cnt, ba_acc, ba_cnt_rx, ba_acc_rx, ba_avg = 0, ba_avg_rx = 0;
bool ba_ena;
spin_lock_bh(&hw_priv->ba_lock);
ba_cnt = hw_priv->debug->ba_cnt;
ba_acc = hw_priv->debug->ba_acc;
/*
* Patch D: ba_lock removed. hw_priv->debug->ba_* are written only
* by the timer callback (single writer); reading without a lock is
* fine for stats. ba_ena is atomic_t.
*/
ba_cnt = hw_priv->debug->ba_cnt;
ba_acc = hw_priv->debug->ba_acc;
ba_cnt_rx = hw_priv->debug->ba_cnt_rx;
ba_acc_rx = hw_priv->debug->ba_acc_rx;
ba_ena = hw_priv->ba_ena;
ba_ena = !!atomic_read(&hw_priv->ba_ena);
if (ba_cnt)
ba_avg = ba_acc / ba_cnt;
if (ba_cnt_rx)
ba_avg_rx = ba_acc_rx / ba_cnt_rx;
spin_unlock_bh(&hw_priv->ba_lock);
seq_puts(seq, "BES2600 Wireless LAN driver status\n");
seq_printf(seq, "Hardware: %d.%d\n",