On Wed, 2024-09-18 at 21:10 +0300, Kalle Valo wrote: > > There is now a new sparse warning, but to keep this long patch simple the > labels will be cleaned up in following patches: > > drivers/net/wireless/ath/ath12k/mac.c:6635:1: warning: statement expected after label I believe this is a compiler error on some compilers (in particular clang), so you probably need to combine patches a little bit. > +++ b/drivers/net/wireless/ath/ath12k/debugfs.c > @@ -15,14 +15,14 @@ static ssize_t ath12k_write_simulate_radar(struct file *file, > struct ath12k *ar = file->private_data; > int ret; > > - mutex_lock(&ar->conf_mutex); > + wiphy_lock(ath12k_ar_to_hw(ar)->wiphy); I don't think this is an issue here, but I'm not sure if you're aware that in general, locking the wiphy mutex in some debugfs file handlers can lead to deadlocks, specifically if those files are later _removed_ while holding the wiphy lock, as is e.g. the case for station, netdev and link debugfs files. For this, we have wiphy_locked_debugfs_read() and wiphy_locked_debugfs_write() [1]. [1] you are not required to understand how they are implemented ;-) > @@ -4310,7 +4301,7 @@ static void ath12k_sta_rc_update_wk(struct work_struct *wk) > > spin_unlock_bh(&ar->data_lock); > > - mutex_lock(&ar->conf_mutex); > + wiphy_lock(ath12k_ar_to_hw(ar)->wiphy); Baochen already pointed out that you seem to not remove this later in patch 4, but in this patch alone you also introduce a bug (that lockdep should point out to you), which is that you cancel_work_sync() this in ath12k_mac_op_sta_state(), which clearly holds the wiphy mutex already. This causes a deadlock. It's fine after patch 4: > /* cancel must be done outside the mutex to avoid deadlock */ > if ((old_state == IEEE80211_STA_NONE && > new_state == IEEE80211_STA_NOTEXIST)) > - cancel_work_sync(&arsta->update_wk); > + wiphy_work_cancel(hw->wiphy, &arsta->update_wk); since for wiphy work it's required to ca<ll it with the mutex held./ You really should remove that comment too though, and perhaps then the code can be simplified by moving this to the later code also handling removal (none->notexist transition). johannes
