[PATCH v3 ath-current] wifi: ath12k: convert scan timeout to wiphy delayed work

[PATCH v3 ath-current] wifi: ath12k: convert scan timeout to wiphy delayed work

[Runyu Xiao]

ath12k_mac_op_stop() is called with the wiphy mutex held and calls
ath12k_mac_stop(), which cancels ar->scan.timeout. The timeout worker
also takes the wiphy mutex before aborting the scan, so synchronously
cancelling it from the stop path can deadlock if the worker has started
and is waiting for the same mutex.

Do not drop the wiphy mutex inside the mac80211 stop callback. Convert
ar->scan.timeout to wiphy_delayed_work instead, so the timeout callback
runs in wiphy work context with the wiphy mutex held. This matches the
locking model used by the scan vdev cleanup work and lets stop/cancel
paths use wiphy_delayed_work_cancel() while they already hold the wiphy
mutex.

The old scan-finish path could cancel the delayed work directly from WMI
event context. With wiphy_delayed_work that cancellation must happen from
wiphy context, so keep it in scan.vdev_clean_wk. Mark scans whose cleanup
work has been queued so a timeout work item that was already queued before
cleanup runs does not abort a scan that is already finishing.

Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Suggested-by: Johannes Berg <johannes@sipsolutions.net>
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
Changes in v3:
  - Convert scan.timeout to wiphy_delayed_work instead of dropping the
    mac80211-owned wiphy lock in ath12k_mac_op_stop().
  - Cancel the timeout from scan.vdev_clean_wk to preserve the
    scan-finish cancellation from wiphy context.
  - Add scan.finish_queued so an already queued timeout work item does not
    abort a scan that has already queued its cleanup work.

Changes in v2:
  - Rebase on ath.git ath-current.
  - Use ath-current subject tag as requested.
  - Move the synchronous scan timeout drain out of the locked stop path
    instead of adding it in the wrong start-path context.
  - Update Fixes to the ath12k commit that introduced the stop-time
    scan.timeout drain.

 drivers/net/wireless/ath/ath12k/core.c |  2 +-
 drivers/net/wireless/ath/ath12k/core.h |  3 +-
 drivers/net/wireless/ath/ath12k/mac.c  | 50 ++++++++++++++++++--------
 3 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 742d4fd1b598..95673d3be9da 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -1463,7 +1463,7 @@ void ath12k_core_halt(struct ath12k *ar)
 
 	ath12k_mac_scan_finish(ar);
 	ath12k_mac_peer_cleanup_all(ar);
-	cancel_delayed_work_sync(&ar->scan.timeout);
+	wiphy_delayed_work_cancel(ath12k_ar_to_hw(ar)->wiphy, &ar->scan.timeout);
 	cancel_work_sync(&ar->regd_update_work);
 	cancel_work_sync(&ar->regd_channel_update_work);
 	cancel_work_sync(&ab->rfkill_work);
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index fc5127b5c1a3..b6fd5e9dfdb7 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -635,11 +635,12 @@ struct ath12k {
 		struct completion started;
 		struct completion completed;
 		struct completion on_channel;
-		struct delayed_work timeout;
+		struct wiphy_delayed_work timeout;
 		enum ath12k_scan_state state;
 		bool is_roc;
 		int roc_freq;
 		bool roc_notify;
+		bool finish_queued;
 		struct wiphy_work vdev_clean_wk;
 		struct ath12k_link_vif *arvif;
 	} scan;
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 51c4df32e716..f53df8d4c808 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -5162,7 +5162,7 @@ void __ath12k_mac_scan_finish(struct ath12k *ar)
 			ieee80211_remain_on_channel_expired(hw);
 		fallthrough;
 	case ATH12K_SCAN_STARTING:
-		cancel_delayed_work(&ar->scan.timeout);
+		ar->scan.finish_queued = true;
 		complete_all(&ar->scan.completed);
 		wiphy_work_queue(ar->ah->hw->wiphy, &ar->scan.vdev_clean_wk);
 		break;
@@ -5254,14 +5254,23 @@ static void ath12k_scan_abort(struct ath12k *ar)
 	spin_unlock_bh(&ar->data_lock);
 }
 
-static void ath12k_scan_timeout_work(struct work_struct *work)
+static void ath12k_scan_timeout_work(struct wiphy *wiphy,
+				     struct wiphy_work *work)
 {
-	struct ath12k *ar = container_of(work, struct ath12k,
-					 scan.timeout.work);
+	struct wiphy_delayed_work *dwork;
+	struct ath12k *ar;
+
+	dwork = container_of(work, struct wiphy_delayed_work, work);
+	ar = container_of(dwork, struct ath12k, scan.timeout);
+
+	spin_lock_bh(&ar->data_lock);
+	if (ar->scan.finish_queued) {
+		spin_unlock_bh(&ar->data_lock);
+		return;
+	}
+	spin_unlock_bh(&ar->data_lock);
 
-	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
 	ath12k_scan_abort(ar);
-	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 }
 
 static void ath12k_mac_scan_send_complete(struct ath12k *ar,
@@ -5292,6 +5301,8 @@ static void ath12k_scan_vdev_clean_work(struct wiphy *wiphy, struct wiphy_work *
 
 	arvif = ar->scan.arvif;
 
+	wiphy_delayed_work_cancel(wiphy, &ar->scan.timeout);
+
 	/* The scan vdev has already been deleted. This can occur when a
 	 * new scan request is made on the same vif with a different
 	 * frequency, causing the scan arvif to move from one radio to
@@ -5323,6 +5334,7 @@ static void ath12k_scan_vdev_clean_work(struct wiphy *wiphy, struct wiphy_work *
 	}
 
 	ar->scan.state = ATH12K_SCAN_IDLE;
+	ar->scan.finish_queued = false;
 	ar->scan_channel = NULL;
 	ar->scan.roc_freq = 0;
 	spin_unlock_bh(&ar->data_lock);
@@ -5609,6 +5621,7 @@ static int ath12k_mac_initiate_hw_scan(struct ieee80211_hw *hw,
 		reinit_completion(&ar->scan.completed);
 		ar->scan.state = ATH12K_SCAN_STARTING;
 		ar->scan.is_roc = false;
+		ar->scan.finish_queued = false;
 		ar->scan.arvif = arvif;
 		ret = 0;
 		break;
@@ -5665,6 +5678,7 @@ static int ath12k_mac_initiate_hw_scan(struct ieee80211_hw *hw,
 
 		spin_lock_bh(&ar->data_lock);
 		ar->scan.state = ATH12K_SCAN_IDLE;
+		ar->scan.finish_queued = false;
 		spin_unlock_bh(&ar->data_lock);
 		goto exit;
 	}
@@ -5672,9 +5686,10 @@ static int ath12k_mac_initiate_hw_scan(struct ieee80211_hw *hw,
 	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac scan started");
 
 	/* Add a margin to account for event/command processing */
-	ieee80211_queue_delayed_work(ath12k_ar_to_hw(ar), &ar->scan.timeout,
-				     msecs_to_jiffies(arg->max_scan_time +
-						      ATH12K_MAC_SCAN_TIMEOUT_MSECS));
+	wiphy_delayed_work_queue(ath12k_ar_to_hw(ar)->wiphy,
+				 &ar->scan.timeout,
+				 msecs_to_jiffies(arg->max_scan_time +
+						  ATH12K_MAC_SCAN_TIMEOUT_MSECS));
 
 exit:
 	if (arg) {
@@ -5757,6 +5772,7 @@ int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
 				spin_lock_bh(&ar->data_lock);
 				ar->scan.arvif = NULL;
 				ar->scan.state = ATH12K_SCAN_IDLE;
+				ar->scan.finish_queued = false;
 				ar->scan_channel = NULL;
 				ar->scan.roc_freq = 0;
 				spin_unlock_bh(&ar->data_lock);
@@ -5792,7 +5808,7 @@ void ath12k_mac_op_cancel_hw_scan(struct ieee80211_hw *hw,
 
 		ath12k_scan_abort(ar);
 
-		cancel_delayed_work_sync(&ar->scan.timeout);
+		wiphy_delayed_work_cancel(hw->wiphy, &ar->scan.timeout);
 	}
 }
 EXPORT_SYMBOL(ath12k_mac_op_cancel_hw_scan);
@@ -9935,7 +9951,7 @@ static void ath12k_mac_stop(struct ath12k *ar)
 
 	clear_bit(ATH12K_FLAG_CAC_RUNNING, &ar->dev_flags);
 
-	cancel_delayed_work_sync(&ar->scan.timeout);
+	wiphy_delayed_work_cancel(ath12k_ar_to_hw(ar)->wiphy, &ar->scan.timeout);
 	wiphy_work_cancel(ath12k_ar_to_hw(ar)->wiphy, &ar->scan.vdev_clean_wk);
 	cancel_work_sync(&ar->regd_channel_update_work);
 	cancel_work_sync(&ar->regd_update_work);
@@ -10962,6 +10978,7 @@ void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
 			}
 
 			ar->scan.state = ATH12K_SCAN_IDLE;
+			ar->scan.finish_queued = false;
 			ar->scan_channel = NULL;
 			ar->scan.roc_freq = 0;
 			spin_unlock_bh(&ar->data_lock);
@@ -13876,7 +13893,7 @@ int ath12k_mac_op_cancel_remain_on_channel(struct ieee80211_hw *hw,
 
 	ath12k_scan_abort(ar);
 
-	cancel_delayed_work_sync(&ar->scan.timeout);
+	wiphy_delayed_work_cancel(hw->wiphy, &ar->scan.timeout);
 	wiphy_work_flush(hw->wiphy, &ar->scan.vdev_clean_wk);
 
 	return 0;
@@ -13956,6 +13973,7 @@ int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 		reinit_completion(&ar->scan.on_channel);
 		ar->scan.state = ATH12K_SCAN_STARTING;
 		ar->scan.is_roc = true;
+		ar->scan.finish_queued = false;
 		ar->scan.arvif = arvif;
 		ar->scan.roc_freq = chan->center_freq;
 		ar->scan.roc_notify = true;
@@ -13998,6 +14016,7 @@ int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 
 		spin_lock_bh(&ar->data_lock);
 		ar->scan.state = ATH12K_SCAN_IDLE;
+		ar->scan.finish_queued = false;
 		spin_unlock_bh(&ar->data_lock);
 		return ret;
 	}
@@ -14011,8 +14030,8 @@ int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 		return -ETIMEDOUT;
 	}
 
-	ieee80211_queue_delayed_work(hw, &ar->scan.timeout,
-				     msecs_to_jiffies(duration));
+	wiphy_delayed_work_queue(hw->wiphy, &ar->scan.timeout,
+				 msecs_to_jiffies(duration));
 
 	return 0;
 }
@@ -15052,6 +15071,7 @@ static void ath12k_mac_setup(struct ath12k *ar)
 	ar->num_tx_chains = hweight32(pdev->cap.tx_chain_mask);
 	ar->num_rx_chains = hweight32(pdev->cap.rx_chain_mask);
 	ar->scan.arvif = NULL;
+	ar->scan.finish_queued = false;
 	ar->vdev_id_11d_scan = ATH12K_11D_INVALID_VDEV_ID;
 
 	spin_lock_init(&ar->data_lock);
@@ -15077,7 +15097,7 @@ static void ath12k_mac_setup(struct ath12k *ar)
 	ar->thermal.temperature = 0;
 	ar->thermal.hwmon_dev = NULL;
 
-	INIT_DELAYED_WORK(&ar->scan.timeout, ath12k_scan_timeout_work);
+	wiphy_delayed_work_init(&ar->scan.timeout, ath12k_scan_timeout_work);
 	wiphy_work_init(&ar->scan.vdev_clean_wk, ath12k_scan_vdev_clean_work);
 	INIT_WORK(&ar->regd_channel_update_work, ath12k_regd_update_chan_list_work);
 	INIT_LIST_HEAD(&ar->regd_channel_update_queue);
-- 
2.34.1

Re: [PATCH v3 ath-current] wifi: ath12k: convert scan timeout to wiphy delayed work

[Jeff Johnson]

On 7/16/2026 7:56 PM, Runyu Xiao wrote:
> ath12k_mac_op_stop() is called with the wiphy mutex held and calls
> ath12k_mac_stop(), which cancels ar->scan.timeout. The timeout worker

you are missing a very important detail, namely it is a synchronous cancel

> also takes the wiphy mutex before aborting the scan, so synchronously
> cancelling it from the stop path can deadlock if the worker has started
> and is waiting for the same mutex.
> 
> Do not drop the wiphy mutex inside the mac80211 stop callback. Convert

What does this mean? Where are we currently dropping the wiphy mutex?

You should describe the patch in terms of the current code, not in terms of
any prior versions of your patch.

> ar->scan.timeout to wiphy_delayed_work instead, so the timeout callback
> runs in wiphy work context with the wiphy mutex held. This matches the
> locking model used by the scan vdev cleanup work and lets stop/cancel
> paths use wiphy_delayed_work_cancel() while they already hold the wiphy
> mutex.
> 
> The old scan-finish path could cancel the delayed work directly from WMI

what is "old"? There is the current code and the new proposed code.

> event context. With wiphy_delayed_work that cancellation must happen from
> wiphy context, so keep it in scan.vdev_clean_wk. Mark scans whose cleanup
> work has been queued so a timeout work item that was already queued before
> cleanup runs does not abort a scan that is already finishing.

My AI review agent thinks this logic is unnecessary, but I haven't looked at
this in detail.

"summary": "finish_queued flag is a redundant substitute for
wiphy_delayed_work_cancel: __ath12k_mac_scan_finish is always called under
wiphy lock, making direct cancellation safe",

"failure_scenario": "Every caller of __ath12k_mac_scan_finish holds the wiphy
lock (directly or via wiphy_work dispatch), so wiphy_delayed_work_cancel could
be called there directly — as the old cancel_delayed_work was. The new
finish_queued flag instead defers suppression to the timeout work itself.
Since ath12k_scan_abort already has an explicit ATH12K_SCAN_IDLE bail-out path
(line 5232) with a comment covering the timeout-vs-completion race, the
finish_queued guard in ath12k_scan_timeout_work provides no additional safety.
The cost is 8 reset sites (lines 5337, 5624, 5681, 5775, 10971, 13969, 14012,
15061) where a future error path that forgets the reset leaves
finish_queued=true, silently suppressing the next legitimate timeout abort and
leaving the driver wedged in SCAN_RUNNING with no recovery."
> 
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Suggested-by: Johannes Berg <johannes@sipsolutions.net>
> Cc: stable@vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> Changes in v3:
>   - Convert scan.timeout to wiphy_delayed_work instead of dropping the
>     mac80211-owned wiphy lock in ath12k_mac_op_stop().
>   - Cancel the timeout from scan.vdev_clean_wk to preserve the
>     scan-finish cancellation from wiphy context.
>   - Add scan.finish_queued so an already queued timeout work item does not
>     abort a scan that has already queued its cleanup work.
> 
> Changes in v2:
>   - Rebase on ath.git ath-current.
>   - Use ath-current subject tag as requested.
>   - Move the synchronous scan timeout drain out of the locked stop path
>     instead of adding it in the wrong start-path context.
>   - Update Fixes to the ath12k commit that introduced the stop-time
>     scan.timeout drain.

FWIW I'd suggest looking into using the 'b4' tool for future contributions.
It will help you with patch history, including providing links to the prior
versions

https://b4.docs.kernel.org/en/latest/#for-developers

> 
>  drivers/net/wireless/ath/ath12k/core.c |  2 +-
>  drivers/net/wireless/ath/ath12k/core.h |  3 +-
>  drivers/net/wireless/ath/ath12k/mac.c  | 50 ++++++++++++++++++--------
>  3 files changed, 38 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
> index 742d4fd1b598..95673d3be9da 100644
> --- a/drivers/net/wireless/ath/ath12k/core.c
> +++ b/drivers/net/wireless/ath/ath12k/core.c
> @@ -1463,7 +1463,7 @@ void ath12k_core_halt(struct ath12k *ar)
>  
>  	ath12k_mac_scan_finish(ar);
>  	ath12k_mac_peer_cleanup_all(ar);
> -	cancel_delayed_work_sync(&ar->scan.timeout);
> +	wiphy_delayed_work_cancel(ath12k_ar_to_hw(ar)->wiphy, &ar->scan.timeout);
>  	cancel_work_sync(&ar->regd_update_work);
>  	cancel_work_sync(&ar->regd_channel_update_work);
>  	cancel_work_sync(&ab->rfkill_work);
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index fc5127b5c1a3..b6fd5e9dfdb7 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -635,11 +635,12 @@ struct ath12k {
>  		struct completion started;
>  		struct completion completed;
>  		struct completion on_channel;
> -		struct delayed_work timeout;
> +		struct wiphy_delayed_work timeout;
>  		enum ath12k_scan_state state;
>  		bool is_roc;
>  		int roc_freq;
>  		bool roc_notify;
> +		bool finish_queued;
>  		struct wiphy_work vdev_clean_wk;
>  		struct ath12k_link_vif *arvif;
>  	} scan;
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 51c4df32e716..f53df8d4c808 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -5162,7 +5162,7 @@ void __ath12k_mac_scan_finish(struct ath12k *ar)
>  			ieee80211_remain_on_channel_expired(hw);
>  		fallthrough;
>  	case ATH12K_SCAN_STARTING:
> -		cancel_delayed_work(&ar->scan.timeout);
> +		ar->scan.finish_queued = true;
>  		complete_all(&ar->scan.completed);
>  		wiphy_work_queue(ar->ah->hw->wiphy, &ar->scan.vdev_clean_wk);
>  		break;
> @@ -5254,14 +5254,23 @@ static void ath12k_scan_abort(struct ath12k *ar)
>  	spin_unlock_bh(&ar->data_lock);
>  }
>  
> -static void ath12k_scan_timeout_work(struct work_struct *work)
> +static void ath12k_scan_timeout_work(struct wiphy *wiphy,
> +				     struct wiphy_work *work)
>  {
> -	struct ath12k *ar = container_of(work, struct ath12k,
> -					 scan.timeout.work);
> +	struct wiphy_delayed_work *dwork;
> +	struct ath12k *ar;
> +
> +	dwork = container_of(work, struct wiphy_delayed_work, work);
> +	ar = container_of(dwork, struct ath12k, scan.timeout);
> +
> +	spin_lock_bh(&ar->data_lock);
> +	if (ar->scan.finish_queued) {
> +		spin_unlock_bh(&ar->data_lock);
> +		return;
> +	}
> +	spin_unlock_bh(&ar->data_lock);

if the flag is necessary, all of that could become:
	scoped_guard(spinlock_bh, &ar->data_lock)
		if (ar->scan.finish_queued)
			return;

>  
> -	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
>  	ath12k_scan_abort(ar);
> -	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
>  }
>  
>  static void ath12k_mac_scan_send_complete(struct ath12k *ar,
> @@ -5292,6 +5301,8 @@ static void ath12k_scan_vdev_clean_work(struct wiphy *wiphy, struct wiphy_work *
>  
>  	arvif = ar->scan.arvif;
>  
> +	wiphy_delayed_work_cancel(wiphy, &ar->scan.timeout);
> +
>  	/* The scan vdev has already been deleted. This can occur when a
>  	 * new scan request is made on the same vif with a different
>  	 * frequency, causing the scan arvif to move from one radio to
> @@ -5323,6 +5334,7 @@ static void ath12k_scan_vdev_clean_work(struct wiphy *wiphy, struct wiphy_work *
>  	}
>  
>  	ar->scan.state = ATH12K_SCAN_IDLE;
> +	ar->scan.finish_queued = false;
>  	ar->scan_channel = NULL;
>  	ar->scan.roc_freq = 0;
>  	spin_unlock_bh(&ar->data_lock);
> @@ -5609,6 +5621,7 @@ static int ath12k_mac_initiate_hw_scan(struct ieee80211_hw *hw,
>  		reinit_completion(&ar->scan.completed);
>  		ar->scan.state = ATH12K_SCAN_STARTING;
>  		ar->scan.is_roc = false;
> +		ar->scan.finish_queued = false;
>  		ar->scan.arvif = arvif;
>  		ret = 0;
>  		break;
> @@ -5665,6 +5678,7 @@ static int ath12k_mac_initiate_hw_scan(struct ieee80211_hw *hw,
>  
>  		spin_lock_bh(&ar->data_lock);
>  		ar->scan.state = ATH12K_SCAN_IDLE;
> +		ar->scan.finish_queued = false;
>  		spin_unlock_bh(&ar->data_lock);
>  		goto exit;
>  	}
> @@ -5672,9 +5686,10 @@ static int ath12k_mac_initiate_hw_scan(struct ieee80211_hw *hw,
>  	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac scan started");
>  
>  	/* Add a margin to account for event/command processing */
> -	ieee80211_queue_delayed_work(ath12k_ar_to_hw(ar), &ar->scan.timeout,
> -				     msecs_to_jiffies(arg->max_scan_time +
> -						      ATH12K_MAC_SCAN_TIMEOUT_MSECS));
> +	wiphy_delayed_work_queue(ath12k_ar_to_hw(ar)->wiphy,
> +				 &ar->scan.timeout,
> +				 msecs_to_jiffies(arg->max_scan_time +
> +						  ATH12K_MAC_SCAN_TIMEOUT_MSECS));
>  
>  exit:
>  	if (arg) {
> @@ -5757,6 +5772,7 @@ int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
>  				spin_lock_bh(&ar->data_lock);
>  				ar->scan.arvif = NULL;
>  				ar->scan.state = ATH12K_SCAN_IDLE;
> +				ar->scan.finish_queued = false;
>  				ar->scan_channel = NULL;
>  				ar->scan.roc_freq = 0;
>  				spin_unlock_bh(&ar->data_lock);
> @@ -5792,7 +5808,7 @@ void ath12k_mac_op_cancel_hw_scan(struct ieee80211_hw *hw,
>  
>  		ath12k_scan_abort(ar);
>  
> -		cancel_delayed_work_sync(&ar->scan.timeout);
> +		wiphy_delayed_work_cancel(hw->wiphy, &ar->scan.timeout);
>  	}
>  }
>  EXPORT_SYMBOL(ath12k_mac_op_cancel_hw_scan);
> @@ -9935,7 +9951,7 @@ static void ath12k_mac_stop(struct ath12k *ar)
>  
>  	clear_bit(ATH12K_FLAG_CAC_RUNNING, &ar->dev_flags);
>  
> -	cancel_delayed_work_sync(&ar->scan.timeout);
> +	wiphy_delayed_work_cancel(ath12k_ar_to_hw(ar)->wiphy, &ar->scan.timeout);
>  	wiphy_work_cancel(ath12k_ar_to_hw(ar)->wiphy, &ar->scan.vdev_clean_wk);
>  	cancel_work_sync(&ar->regd_channel_update_work);
>  	cancel_work_sync(&ar->regd_update_work);
> @@ -10962,6 +10978,7 @@ void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
>  			}
>  
>  			ar->scan.state = ATH12K_SCAN_IDLE;
> +			ar->scan.finish_queued = false;
>  			ar->scan_channel = NULL;
>  			ar->scan.roc_freq = 0;
>  			spin_unlock_bh(&ar->data_lock);
> @@ -13876,7 +13893,7 @@ int ath12k_mac_op_cancel_remain_on_channel(struct ieee80211_hw *hw,
>  
>  	ath12k_scan_abort(ar);
>  
> -	cancel_delayed_work_sync(&ar->scan.timeout);
> +	wiphy_delayed_work_cancel(hw->wiphy, &ar->scan.timeout);
>  	wiphy_work_flush(hw->wiphy, &ar->scan.vdev_clean_wk);
>  
>  	return 0;
> @@ -13956,6 +13973,7 @@ int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
>  		reinit_completion(&ar->scan.on_channel);
>  		ar->scan.state = ATH12K_SCAN_STARTING;
>  		ar->scan.is_roc = true;
> +		ar->scan.finish_queued = false;
>  		ar->scan.arvif = arvif;
>  		ar->scan.roc_freq = chan->center_freq;
>  		ar->scan.roc_notify = true;
> @@ -13998,6 +14016,7 @@ int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
>  
>  		spin_lock_bh(&ar->data_lock);
>  		ar->scan.state = ATH12K_SCAN_IDLE;
> +		ar->scan.finish_queued = false;
>  		spin_unlock_bh(&ar->data_lock);
>  		return ret;
>  	}
> @@ -14011,8 +14030,8 @@ int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
>  		return -ETIMEDOUT;
>  	}
>  
> -	ieee80211_queue_delayed_work(hw, &ar->scan.timeout,
> -				     msecs_to_jiffies(duration));
> +	wiphy_delayed_work_queue(hw->wiphy, &ar->scan.timeout,
> +				 msecs_to_jiffies(duration));
>  
>  	return 0;
>  }
> @@ -15052,6 +15071,7 @@ static void ath12k_mac_setup(struct ath12k *ar)
>  	ar->num_tx_chains = hweight32(pdev->cap.tx_chain_mask);
>  	ar->num_rx_chains = hweight32(pdev->cap.rx_chain_mask);
>  	ar->scan.arvif = NULL;
> +	ar->scan.finish_queued = false;
>  	ar->vdev_id_11d_scan = ATH12K_11D_INVALID_VDEV_ID;
>  
>  	spin_lock_init(&ar->data_lock);
> @@ -15077,7 +15097,7 @@ static void ath12k_mac_setup(struct ath12k *ar)
>  	ar->thermal.temperature = 0;
>  	ar->thermal.hwmon_dev = NULL;
>  
> -	INIT_DELAYED_WORK(&ar->scan.timeout, ath12k_scan_timeout_work);
> +	wiphy_delayed_work_init(&ar->scan.timeout, ath12k_scan_timeout_work);
>  	wiphy_work_init(&ar->scan.vdev_clean_wk, ath12k_scan_vdev_clean_work);
>  	INIT_WORK(&ar->regd_channel_update_work, ath12k_regd_update_chan_list_work);
>  	INIT_LIST_HEAD(&ar->regd_channel_update_queue);

Re: [PATCH v3 ath-current] wifi: ath12k: convert scan timeout to wiphy delayed work

[Jeff Johnson]

On 7/17/2026 7:40 AM, Jeff Johnson wrote:
> On 7/16/2026 7:56 PM, Runyu Xiao wrote:
>> ath12k_mac_op_stop() is called with the wiphy mutex held and calls
>> ath12k_mac_stop(), which cancels ar->scan.timeout. The timeout worker
> 
> you are missing a very important detail, namely it is a synchronous cancel
> 
>> also takes the wiphy mutex before aborting the scan, so synchronously
>> cancelling it from the stop path can deadlock if the worker has started
>> and is waiting for the same mutex.
>>
>> Do not drop the wiphy mutex inside the mac80211 stop callback. Convert
> 
> What does this mean? Where are we currently dropping the wiphy mutex?
> 
> You should describe the patch in terms of the current code, not in terms of
> any prior versions of your patch.
> 
>> ar->scan.timeout to wiphy_delayed_work instead, so the timeout callback
>> runs in wiphy work context with the wiphy mutex held. This matches the
>> locking model used by the scan vdev cleanup work and lets stop/cancel
>> paths use wiphy_delayed_work_cancel() while they already hold the wiphy
>> mutex.
>>
>> The old scan-finish path could cancel the delayed work directly from WMI
> 
> what is "old"? There is the current code and the new proposed code.
> 
>> event context. With wiphy_delayed_work that cancellation must happen from
>> wiphy context, so keep it in scan.vdev_clean_wk. Mark scans whose cleanup
>> work has been queued so a timeout work item that was already queued before
>> cleanup runs does not abort a scan that is already finishing.
> 
> My AI review agent thinks this logic is unnecessary, but I haven't looked at
> this in detail.
> 
> "summary": "finish_queued flag is a redundant substitute for
> wiphy_delayed_work_cancel: __ath12k_mac_scan_finish is always called under
> wiphy lock, making direct cancellation safe",
> 
> "failure_scenario": "Every caller of __ath12k_mac_scan_finish holds the wiphy
> lock (directly or via wiphy_work dispatch), so wiphy_delayed_work_cancel could
> be called there directly — as the old cancel_delayed_work was. The new
> finish_queued flag instead defers suppression to the timeout work itself.
> Since ath12k_scan_abort already has an explicit ATH12K_SCAN_IDLE bail-out path
> (line 5232) with a comment covering the timeout-vs-completion race, the
> finish_queued guard in ath12k_scan_timeout_work provides no additional safety.
> The cost is 8 reset sites (lines 5337, 5624, 5681, 5775, 10971, 13969, 14012,
> 15061) where a future error path that forgets the reset leaves
> finish_queued=true, silently suppressing the next legitimate timeout abort and
> leaving the driver wedged in SCAN_RUNNING with no recovery."

You never responded to my response. However, since this seems to be a
legitimate issue I did a little more research and now see why the
finish_queued flag is needed. A better commit text should explain that since
the current commit text is confusing

I propose something like the following. Yes it is AI-assisted, but IMO it does
a good job of describing both the existing problem and the reason the new flag
is needed in the modified version.


ath12k_mac_op_stop() is called with the wiphy mutex held and calls
ath12k_mac_stop(), which synchronously cancels ar->scan.timeout via
cancel_delayed_work_sync(). The timeout worker takes the wiphy mutex
before aborting the scan, so if the worker has already started and is
waiting for that mutex, the stop path blocks indefinitely: stop holds
the wiphy mutex waiting for the worker to finish, and the worker waits
for the wiphy mutex to proceed.

In order to avoid this deadlock, convert ar->scan.timeout from a
plain delayed_work to a wiphy_delayed_work. The timeout callback then
runs in wiphy work context with the wiphy mutex already held,
matching the locking model used by ar->scan.vdev_clean_wk. Stop and
cancel paths can use wiphy_delayed_work_cancel() while holding the
mutex, which removes the _sync wait that caused the deadlock.

There is, however, one additional issue with the scan complete path.
wiphy_delayed_work_cancel() requires the wiphy mutex, but
__ath12k_mac_scan_finish() is called from WMI event handlers under
data_lock only, without the wiphy mutex. The existing code can call
cancel_delayed_work() there directly, but that is no longer valid when
using wiphy_delayed_work. Instead, set a finish_queued flag and queue
vdev_clean_wk, which runs under the wiphy mutex and performs the
cancel. Add a check in the timeout worker so that if it was already
queued before finish_queued was set, it exits without aborting a scan
that is already finishing.

/jeff

Re: [PATCH v3 ath-current] wifi: ath12k: convert scan timeout to wiphy delayed work

[Runyu Xiao]

On Fri, Sep 11, 2026 at 01:36:47PM -0700, Jeff Johnson wrote:
> You never responded to my response. However, since this seems to be a
> legitimate issue I did a little more research and now see why the
> finish_queued flag is needed. A better commit text should explain that since
> the current commit text is confusing

Sorry for the delay. I was unsure how this part should be resolved, so I
held off on replying.

> There is, however, one additional issue with the scan complete path.
> wiphy_delayed_work_cancel() requires the wiphy mutex, but
> __ath12k_mac_scan_finish() is called from WMI event handlers under
> data_lock only, without the wiphy mutex. The existing code can call
> cancel_delayed_work() there directly, but that is no longer valid when
> using wiphy_delayed_work. Instead, set a finish_queued flag and queue
> vdev_clean_wk, which runs under the wiphy mutex and performs the
> cancel. Add a check in the timeout worker so that if it was already
> queued before finish_queued was set, it exits without aborting a scan
> that is already finishing.

Agreed. Your explanation clarifies why the finish_queued flag is needed.

I will keep the finish_queued logic and update the commit message accordingly
in the next revision.

Thanks for taking another look.

[PATCH v4 ath-current] wifi: ath12k: convert scan timeout to wiphy delayed work

[Runyu Xiao]

ath12k_mac_op_stop() runs with the wiphy mutex held and calls
ath12k_mac_stop(), which synchronously cancels ar->scan.timeout.
The timeout worker takes the same mutex before aborting the scan. If
the worker has started and is waiting for the mutex, stop waits for the
worker while the worker waits for the mutex, resulting in a deadlock.

Convert ar->scan.timeout to a wiphy_delayed_work. Its callback then runs
as wiphy work with the mutex held, allowing stop and other cancellation
paths to use wiphy_delayed_work_cancel() without waiting for the work
callback. The same conversion is applied to ath12k_core_halt(), which
also runs with the wiphy mutex held.

WMI event handlers call __ath12k_mac_scan_finish() while holding only
data_lock, so they cannot cancel the wiphy delayed work directly. Set
finish_queued and rely on the existing vdev_clean_wk to cancel the
timeout from wiphy context. Check finish_queued in ath12k_scan_abort()
under data_lock before changing a running scan to aborting, so a timeout
racing with completion does not abort a scan whose completion has already
queued cleanup.

Fixes: b8c67509b91ec ("wifi: ath12k: switch to using wiphy_lock() and remove ar->conf_mutex")
Suggested-by: Johannes Berg <johannes@sipsolutions.net>
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
Changes in v4:
  - Rebase on Linux 7.3-rc2.
  - Rewrite the commit message and close the completion/timeout race by
    checking scan.finish_queued under data_lock.

 drivers/net/wireless/ath/ath12k/core.c |  2 +-
 drivers/net/wireless/ath/ath12k/core.h |  3 +-
 drivers/net/wireless/ath/ath12k/mac.c  | 46 ++++++++++++++++++++++----------
 3 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 262a2045309b1f1b85a9ad2fc72cbfcd18bc0586..7290616607795365963f6d4b538575abce918b37 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -1444,7 +1444,7 @@ void ath12k_core_halt(struct ath12k *ar)
 
 	ath12k_mac_scan_finish(ar);
 	ath12k_mac_peer_cleanup_all(ar);
-	cancel_delayed_work_sync(&ar->scan.timeout);
+	wiphy_delayed_work_cancel(ath12k_ar_to_hw(ar)->wiphy, &ar->scan.timeout);
 	cancel_work_sync(&ar->regd_update_work);
 	cancel_work_sync(&ar->regd_channel_update_work);
 	cancel_work_sync(&ab->rfkill_work);
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index a98fc6e0699d7678ddd86bc44471fc6ff46aa22c..1f6b5547f4d90b6d5299dc2c400c0cbd89a4608d 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -628,11 +628,12 @@ struct ath12k {
 		struct completion started;
 		struct completion completed;
 		struct completion on_channel;
-		struct delayed_work timeout;
+		struct wiphy_delayed_work timeout;
 		enum ath12k_scan_state state;
 		bool is_roc;
 		int roc_freq;
 		bool roc_notify;
+		bool finish_queued;
 		struct wiphy_work vdev_clean_wk;
 		struct ath12k_link_vif *arvif;
 	} scan;
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 99bf5cf79d10286e9959191536aecacbbf343287..0e1e9117b79fb4532ef2117715e21e1a14d3b887 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -5231,7 +5231,7 @@ void __ath12k_mac_scan_finish(struct ath12k *ar)
 			ieee80211_remain_on_channel_expired(hw);
 		fallthrough;
 	case ATH12K_SCAN_STARTING:
-		cancel_delayed_work(&ar->scan.timeout);
+		ar->scan.finish_queued = true;
 		complete_all(&ar->scan.completed);
 		wiphy_work_queue(ar->ah->hw->wiphy, &ar->scan.vdev_clean_wk);
 		break;
@@ -5311,4 +5311,6 @@ static void ath12k_scan_abort(struct ath12k *ar)
 	case ATH12K_SCAN_RUNNING:
+		if (ar->scan.finish_queued)
+			break;
 		ar->scan.state = ATH12K_SCAN_ABORTING;
 		spin_unlock_bh(&ar->data_lock);
 
@@ -5323,14 +5325,17 @@ static void ath12k_scan_abort(struct ath12k *ar)
 	spin_unlock_bh(&ar->data_lock);
 }
 
-static void ath12k_scan_timeout_work(struct work_struct *work)
+static void ath12k_scan_timeout_work(struct wiphy *wiphy,
+				     struct wiphy_work *work)
 {
-	struct ath12k *ar = container_of(work, struct ath12k,
-					 scan.timeout.work);
+	struct wiphy_delayed_work *dwork;
+	struct ath12k *ar;
+
+	dwork = container_of(work, struct wiphy_delayed_work, work);
+	ar = container_of(dwork, struct ath12k, scan.timeout);
+	lockdep_assert_wiphy(wiphy);
 
-	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
 	ath12k_scan_abort(ar);
-	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 }
 
 static void ath12k_mac_scan_send_complete(struct ath12k *ar,
@@ -5361,6 +5370,8 @@ static void ath12k_scan_vdev_clean_work(struct wiphy *wiphy, struct wiphy_work *
 
 	arvif = ar->scan.arvif;
 
+	wiphy_delayed_work_cancel(wiphy, &ar->scan.timeout);
+
 	/* The scan vdev has already been deleted. This can occur when a
 	 * new scan request is made on the same vif with a different
 	 * frequency, causing the scan arvif to move from one radio to
@@ -5392,6 +5403,7 @@ static void ath12k_scan_vdev_clean_work(struct wiphy *wiphy, struct wiphy_work *
 	}
 
 	ar->scan.state = ATH12K_SCAN_IDLE;
+	ar->scan.finish_queued = false;
 	ar->scan_channel = NULL;
 	ar->scan.roc_freq = 0;
 	spin_unlock_bh(&ar->data_lock);
@@ -5678,6 +5690,7 @@ static int ath12k_mac_initiate_hw_scan(struct ieee80211_hw *hw,
 		reinit_completion(&ar->scan.completed);
 		ar->scan.state = ATH12K_SCAN_STARTING;
 		ar->scan.is_roc = false;
+		ar->scan.finish_queued = false;
 		ar->scan.arvif = arvif;
 		ret = 0;
 		break;
@@ -5734,6 +5747,7 @@ static int ath12k_mac_initiate_hw_scan(struct ieee80211_hw *hw,
 
 		spin_lock_bh(&ar->data_lock);
 		ar->scan.state = ATH12K_SCAN_IDLE;
+		ar->scan.finish_queued = false;
 		spin_unlock_bh(&ar->data_lock);
 		goto exit;
 	}
@@ -5741,9 +5755,10 @@ static int ath12k_mac_initiate_hw_scan(struct ieee80211_hw *hw,
 	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac scan started");
 
 	/* Add a margin to account for event/command processing */
-	ieee80211_queue_delayed_work(ath12k_ar_to_hw(ar), &ar->scan.timeout,
-				     msecs_to_jiffies(arg->max_scan_time +
-						      ATH12K_MAC_SCAN_TIMEOUT_MSECS));
+	wiphy_delayed_work_queue(ath12k_ar_to_hw(ar)->wiphy,
+				 &ar->scan.timeout,
+				 msecs_to_jiffies(arg->max_scan_time +
+						  ATH12K_MAC_SCAN_TIMEOUT_MSECS));
 
 exit:
 	if (arg) {
@@ -5826,6 +5841,7 @@ int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
 				spin_lock_bh(&ar->data_lock);
 				ar->scan.arvif = NULL;
 				ar->scan.state = ATH12K_SCAN_IDLE;
+				ar->scan.finish_queued = false;
 				ar->scan_channel = NULL;
 				ar->scan.roc_freq = 0;
 				spin_unlock_bh(&ar->data_lock);
@@ -5861,7 +5877,7 @@ void ath12k_mac_op_cancel_hw_scan(struct ieee80211_hw *hw,
 
 		ath12k_scan_abort(ar);
 
-		cancel_delayed_work_sync(&ar->scan.timeout);
+		wiphy_delayed_work_cancel(hw->wiphy, &ar->scan.timeout);
 	}
 }
 EXPORT_SYMBOL(ath12k_mac_op_cancel_hw_scan);
@@ -10007,7 +10023,7 @@ static void ath12k_mac_stop(struct ath12k *ar)
 
 	clear_bit(ATH12K_FLAG_CAC_RUNNING, &ar->dev_flags);
 
-	cancel_delayed_work_sync(&ar->scan.timeout);
+	wiphy_delayed_work_cancel(ath12k_ar_to_hw(ar)->wiphy, &ar->scan.timeout);
 	wiphy_work_cancel(ath12k_ar_to_hw(ar)->wiphy, &ar->scan.vdev_clean_wk);
 	cancel_work_sync(&ar->regd_channel_update_work);
 	cancel_work_sync(&ar->regd_update_work);
@@ -11024,6 +11040,7 @@ void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
 			}
 
 			ar->scan.state = ATH12K_SCAN_IDLE;
+			ar->scan.finish_queued = false;
 			ar->scan_channel = NULL;
 			ar->scan.roc_freq = 0;
 			spin_unlock_bh(&ar->data_lock);
@@ -13943,7 +13960,7 @@ int ath12k_mac_op_cancel_remain_on_channel(struct ieee80211_hw *hw,
 
 	ath12k_scan_abort(ar);
 
-	cancel_delayed_work_sync(&ar->scan.timeout);
+	wiphy_delayed_work_cancel(hw->wiphy, &ar->scan.timeout);
 	wiphy_work_flush(hw->wiphy, &ar->scan.vdev_clean_wk);
 
 	return 0;
@@ -14023,6 +14040,7 @@ int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 		reinit_completion(&ar->scan.on_channel);
 		ar->scan.state = ATH12K_SCAN_STARTING;
 		ar->scan.is_roc = true;
+		ar->scan.finish_queued = false;
 		ar->scan.arvif = arvif;
 		ar->scan.roc_freq = chan->center_freq;
 		ar->scan.roc_notify = true;
@@ -14065,6 +14083,7 @@ int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 
 		spin_lock_bh(&ar->data_lock);
 		ar->scan.state = ATH12K_SCAN_IDLE;
+		ar->scan.finish_queued = false;
 		spin_unlock_bh(&ar->data_lock);
 		return ret;
 	}
@@ -14078,8 +14097,8 @@ int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 		return -ETIMEDOUT;
 	}
 
-	ieee80211_queue_delayed_work(hw, &ar->scan.timeout,
-				     msecs_to_jiffies(duration));
+	wiphy_delayed_work_queue(hw->wiphy, &ar->scan.timeout,
+				 msecs_to_jiffies(duration));
 
 	return 0;
 }
@@ -15113,6 +15132,7 @@ static void ath12k_mac_setup(struct ath12k *ar)
 	ar->num_tx_chains = hweight32(pdev->cap.tx_chain_mask);
 	ar->num_rx_chains = hweight32(pdev->cap.rx_chain_mask);
 	ar->scan.arvif = NULL;
+	ar->scan.finish_queued = false;
 	ar->vdev_id_11d_scan = ATH12K_11D_INVALID_VDEV_ID;
 
 	spin_lock_init(&ar->data_lock);
@@ -15138,7 +15158,7 @@ static void ath12k_mac_setup(struct ath12k *ar)
 	ar->thermal.temperature = 0;
 	ar->thermal.hwmon_dev = NULL;
 
-	INIT_DELAYED_WORK(&ar->scan.timeout, ath12k_scan_timeout_work);
+	wiphy_delayed_work_init(&ar->scan.timeout, ath12k_scan_timeout_work);
 	wiphy_work_init(&ar->scan.vdev_clean_wk, ath12k_scan_vdev_clean_work);
 	INIT_WORK(&ar->regd_channel_update_work, ath12k_regd_update_chan_list_work);
 	INIT_LIST_HEAD(&ar->regd_channel_update_queue);
-- 
2.34.1