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 979d5436ee
commit 44b296647b
5 changed files with 85 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bes2600/bes2600.h
+17 -9
View File
@@ -356,15 +356,23 @@ struct bes2600_common {
* Keeping in common structure for the time being. Will be moved to VIFF
* after the mechanism is clear */
u8 ba_tid_mask;
int ba_acc; /*TODO: Same as above */
int ba_cnt; /*TODO: Same as above */
int ba_cnt_rx; /*TODO: Same as above */
int ba_acc_rx; /*TODO: Same as above */
int ba_hist; /*TODO: Same as above */
struct timer_list ba_timer;/*TODO: Same as above */
spinlock_t ba_lock; /*TODO: Same as above */
bool ba_ena; /*TODO: Same as above */
struct work_struct ba_work; /*TODO: Same as above */
/*
* Patch D: ba_lock removed. Per-frame TX/RX hot-path bumped these
* counters under spin_lock_bh; the lock did not protect any
* compound invariant that atomic ops can't satisfy. Counters are
* now atomic_t; ba_armed gates the once-per-window mod_timer
* arm via cmpxchg so concurrent TX/RX at a fresh window each
* try to claim the arm and exactly one succeeds.
*/
atomic_t ba_acc;
atomic_t ba_cnt;
atomic_t ba_cnt_rx;
atomic_t ba_acc_rx;
atomic_t ba_armed;
int ba_hist;
struct timer_list ba_timer;
atomic_t ba_ena;
struct work_struct ba_work;
bool is_BT_Present;
bool is_go_thru_go_neg;
u8 conf_listen_interval;