Re: [RFC PATCH 00/12] wifi: AP side locking improvements

[Jeff Johnson <jeff.johnson@oss.qualcomm.com>]

On 10/4/2026 2:40 PM, Johannes Berg wrote:
> This obviously goes on top of my (fixed) RTNL redux series.
> 
> The idea here is that wiphy mutex can be held for quite a bit
> of time (e.g. waiting for firmware), but beacon updates (e.g.
> with the critical update design I had proposed a long while
> back) and other things should be fast.
> 
> So with this not all things require wiphy mutex. These are:
> 
>  - beacon and related template updates
>    (this one needs driver opt-in),
>  - mgmt frame TX,
>  - control port TX, and
>  - peer probe.
> 
> This does make the implementation slightly more complex, but
> the added complexity is almost entirely in mac80211 (and some
> in cfg80211), unless a driver wants to opt in to unlocked
> beacon/template updates, which it has to implement itself for
> obvious reasons.
> 
> I think we probably want this even without the critical update
> design we're still debating, but I thought I'd throw this out
> here because a lot of the discussion has been focused on the
> latencies throughout the stack, and I never thought they were
> fundamental. QED ;-)
> 
> johannes

With both the redux and this series applied, I prompted my review agent:
prior to the 2 patchsets (18 patches and 12 patches) ath12k relied upon the
wiphy mutex to serialize many of the mac80211 operations. are any ath12k
object references now subject to new race conditions with this entire set of
30 patches applied?

The reply:
<LLM>
New Race: ath12k_reg_notifier vs ath12k_mac_op_start/ath12k_mac_op_stop on
ah->state

Before the series: ath12k_reg_notifier was called from
notify_self_managed_wiphys(), which ran inside reg_todo() under RTNL.
ath12k_mac_op_start() and ath12k_mac_op_stop() both assert
lockdep_assert_wiphy() at entry — and mac80211 calls them holding the wiphy
lock. The wiphy lock was acquired under RTNL (rtnl_lock() → wiphy_lock()), so
RTNL serialized the notifier against start/stop.

After the series (patch 18, 7fccd0f981b5): reg_todo() now holds cfg80211_mutex
instead of RTNL. notify_self_managed_wiphys() calls ath12k_reg_notifier()
holding only cfg80211_mutex — no wiphy lock, no RTNL.
ath12k_mac_op_start()/ath12k_mac_op_stop() hold only the wiphy lock — no
cfg80211_mutex. These two locks are disjoint.

Inside ath12k_reg_notifier(), the first thing it does is read ah->state
without holding ah->hw_mutex:

if (ah->state != ATH12K_HW_STATE_ON)   /* plain read, no hw_mutex */
    return;

ah->state is written exclusively under ah->hw_mutex (in ath12k_mac_op_start(),
ath12k_mac_op_stop(), and ath12k_mac_op_reconfig_complete()), none of which
hold cfg80211_mutex. The reg notifier (on CPU A, holding cfg80211_mutex) and
mac_op_stop (on CPU B, holding wiphy lock + hw_mutex) can now execute
concurrently. The ah->state read in the notifier and the ah->state =
ATH12K_HW_STATE_OFF write in stop share no common lock. This is a new data
race.

The same applies to the ah->regd_updated = false write further down in the
notifier and the ah->regd_updated = true write in ath12k_regd_update() (called
from a workqueue with no lock), and the ah->regd_updated read in
ath12k_regd_update() — none of these are serialized.

Before the series, RTNL acting as a global serializer masked all of this.

Pre-existing but Newly Exposed: ath12k_regd_update vs ath12k_reg_notifier on
ah->regd_updated

ath12k_regd_update() is called from ath12k_regd_update_work (a workqueue) and
reads then writes ah->regd_updated with no lock at all. ath12k_reg_notifier()
writes ah->regd_updated = false also with no lock. These two were already
concurrent before the series — workqueue items are not serialized by RTNL — so
this is not a new problem introduced by the patches. However, RTNL previously
prevented the reg notifier from being called while a mac op was running,
which reduced the window. That window is now wider.
</LLM>

So we'll probably need to address those issue.

/jeff