On 1/24/2026 1:56 AM, Ziyi Guo wrote:
> ath10k_wmi_event_peer_sta_ps_state_chg() uses lockdep_assert_held() to
> assert that ar->data_lock should be held by the caller, but neither
> ath10k_wmi_10_2_op_rx() nor ath10k_wmi_10_4_op_rx() acquire this lock
> before calling this function.
>
> The field arsta->peer_ps_state is documented as protected by
> ar->data_lock in core.h, and other accessors (ath10k_peer_ps_state_disable,
> ath10k_dbg_sta_read_peer_ps_state) properly acquire this lock.
there is another instance in ath10k_sta_state() where the lock is not acquired,
but I
guess that is OK since during the NOTEXIST -> NONE transition no race is
expected.
>
> Add spin_lock_bh()/spin_unlock_bh() around the peer_ps_state update,
> and remove the lockdep_assert_held() to be aligned with new locking,
> following the pattern used by other WMI event handlers in the driver.
>
> Signed-off-by: Ziyi Guo <[email protected]>
> ---
> v2:
> - Remove lockdep_assert_held() as suggested, since
> we are now taking the lock internally.
>
> drivers/net/wireless/ath/ath10k/wmi.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
> b/drivers/net/wireless/ath/ath10k/wmi.c
> index b4aad6604d6d..061a2fa8f00f 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -5289,7 +5289,6 @@ ath10k_wmi_event_peer_sta_ps_state_chg(struct ath10k
> *ar, struct sk_buff *skb)
> struct ath10k_sta *arsta;
> u8 peer_addr[ETH_ALEN];
>
> - lockdep_assert_held(&ar->data_lock);
>
> ev = (struct wmi_peer_sta_ps_state_chg_event *)skb->data;
> ether_addr_copy(peer_addr, ev->peer_macaddr.addr);
> @@ -5305,7 +5304,9 @@ ath10k_wmi_event_peer_sta_ps_state_chg(struct ath10k
> *ar, struct sk_buff *skb)
> }
>
> arsta = (struct ath10k_sta *)sta->drv_priv;
> + spin_lock_bh(&ar->data_lock);
> arsta->peer_ps_state = __le32_to_cpu(ev->peer_ps_state);
> + spin_unlock_bh(&ar->data_lock);
>
> exit:
> rcu_read_unlock();
Reviewed-by: Baochen Qiang <[email protected]>
