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