[PATCH ath-next v3 01/10] wifi: ath10k: fix SDIO aux workqueue starvation in recovery detection
Loic Poulain
loic.poulain at oss.qualcomm.com
Fri Nov 13 01:24:25 PST 2026
On Thu, Nov 12, 2026 at 12:26 PM Kang Yang <kang.yang at oss.qualcomm.com> wrote:
>
> Recovery monitoring runs on workqueue_aux, which is also
> used by SDIO RX dispatch and WMI SERVICE_READY handling.
>
> A single firmware crash may generate multiple recovery
> triggers. Recovery detection currently handles each one by
> waiting for recovery completion on workqueue_aux. On SDIO,
> this blocks the same workqueue needed to deliver recovery
> traffic, preventing recovery from completing.
>
> As a result, otherwise recoverable firmware crashes fail
> with:
> ctl_resp never came in (-110)
>
> Treat duplicate triggers as part of the same recovery attempt. Use an
> atomic compare-and-exchange on pending_recovery so only the first
> trigger starts restart_work.
>
> Convert the recovery monitor to self-rearming delayed work. While
> recovery remains pending, the monitor periodically checks its state
> without blocking workqueue_aux. Cancel the monitor when recovery
> succeeds or the device is stopped.
>
> Tested-on: QCA6174 hw3.2 PCI WLAN.RM.4.4.1-00288-QCARMSWPZ-1
> Tested-on: QCA6174 hw3.2 SDIO WLAN.RMH.4.4.1-00189
>
> Fixes: f35a07a4842a ("wifi: ath10k: move recovery check logic into a new work")
> Signed-off-by: Kang Yang <kang.yang at oss.qualcomm.com>
> ---
> drivers/net/wireless/ath/ath10k/core.c | 71 +++++++++++++++++++---------------
> drivers/net/wireless/ath/ath10k/core.h | 2 +-
> drivers/net/wireless/ath/ath10k/mac.c | 13 ++++++-
> 3 files changed, 53 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> index 7c2939cbde5f..82bb0bb72dc7 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -2493,48 +2493,54 @@ static int ath10k_init_hw_params(struct ath10k *ar)
>
> static void ath10k_core_recovery_check_work(struct work_struct *work)
> {
> - struct ath10k *ar = container_of(work, struct ath10k, recovery_check_work);
> - long time_left;
> + struct ath10k *ar = container_of(work, struct ath10k,
> + recovery_check_work.work);
> + int fail_count;
>
> - /* Sometimes the recovery will fail and then the next all recovery fail,
> - * so avoid infinite recovery.
> - */
> - if (atomic_read(&ar->fail_cont_count) >= ATH10K_RECOVERY_MAX_FAIL_COUNT) {
> - ath10k_err(ar, "consecutive fail %d times, will shutdown driver!",
> - atomic_read(&ar->fail_cont_count));
> - ar->state = ATH10K_STATE_WEDGED;
> + if (ar->state == ATH10K_STATE_WEDGED)
> return;
> - }
> -
> - ath10k_dbg(ar, ATH10K_DBG_BOOT, "total recovery count: %d", ++ar->recovery_count);
>
> - if (atomic_read(&ar->pending_recovery)) {
> - /* Sometimes it happened another recovery work before the previous one
> - * completed, then the second recovery work will destroy the previous
> - * one, thus below is to avoid that.
> - */
> - time_left = wait_for_completion_timeout(&ar->driver_recovery,
> - ATH10K_RECOVERY_TIMEOUT_HZ);
> - if (time_left) {
> - ath10k_warn(ar, "previous recovery succeeded, skip this!\n");
> - return;
> - }
> + /*
> + * Pure monitor: only ath10k_core_start_recovery() starts a fresh
> + * cycle. Bail if there is nothing in flight.
> + */
> + if (!atomic_read(&ar->pending_recovery))
> + return;
>
> - /* Record the continuous recovery fail count when recovery failed. */
> - atomic_inc(&ar->fail_cont_count);
> + /* Record the continuous recovery fail count when recovery is late. */
> + fail_count = atomic_inc_return(&ar->fail_cont_count);
I don't understand why we need so much complication and the need to
count and restart this work again, why not simply scheduling the work
or a timer at the final expected timeout instead, then if we reach the
timeout the device is simply marked as wedged.
>
> - /* Avoid having multiple recoveries at the same time. */
> + if (fail_count >= ATH10K_RECOVERY_MAX_FAIL_COUNT) {
> + ath10k_err(ar, "consecutive fail %d times, will shutdown driver!",
> + fail_count);
> + ar->state = ATH10K_STATE_WEDGED;
> return;
> }
>
> - atomic_inc(&ar->pending_recovery);
> - queue_work(ar->workqueue, &ar->restart_work);
> + /* Recovery still in flight; check again later. */
> + queue_delayed_work(ar->workqueue_aux, &ar->recovery_check_work,
> + ATH10K_RECOVERY_TIMEOUT_HZ);
> }
>
> void ath10k_core_start_recovery(struct ath10k *ar)
> {
> - /* Use workqueue_aux to avoid blocking recovery tracking */
> - queue_work(ar->workqueue_aux, &ar->recovery_check_work);
> + if (ar->state == ATH10K_STATE_WEDGED)
> + return;
> +
> + /*
> + * SDIO raises several triggers within microseconds of one crash;
> + * only the first winner starts a fresh cycle. Losers do nothing
> + * while the owner runs recovery.
> + */
> + if (atomic_cmpxchg(&ar->pending_recovery, 0, 1) != 0)
> + return;
> + ath10k_dbg(ar, ATH10K_DBG_BOOT, "total recovery count: %d",
> + ++ar->recovery_count);
> + /* Start a fresh failure budget; the last cycle may not have completed. */
> + atomic_set(&ar->fail_cont_count, 0);
> + queue_work(ar->workqueue, &ar->restart_work);
> + queue_delayed_work(ar->workqueue_aux, &ar->recovery_check_work,
> + ATH10K_RECOVERY_TIMEOUT_HZ);
> }
> EXPORT_SYMBOL(ath10k_core_start_recovery);
>
> @@ -3727,7 +3733,7 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
>
> INIT_WORK(&ar->register_work, ath10k_core_register_work);
> INIT_WORK(&ar->restart_work, ath10k_core_restart);
> - INIT_WORK(&ar->recovery_check_work, ath10k_core_recovery_check_work);
> + INIT_DELAYED_WORK(&ar->recovery_check_work, ath10k_core_recovery_check_work);
> INIT_WORK(&ar->set_coverage_class_work,
> ath10k_core_set_coverage_class_work);
>
> @@ -3764,6 +3770,9 @@ EXPORT_SYMBOL(ath10k_core_create);
>
> void ath10k_core_destroy(struct ath10k *ar)
> {
> + cancel_work_sync(&ar->restart_work);
> + cancel_delayed_work_sync(&ar->recovery_check_work);
> +
> destroy_workqueue(ar->workqueue);
>
> destroy_workqueue(ar->workqueue_aux);
> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> index dfee432615eb..e8fcbef38ca6 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -1207,7 +1207,7 @@ struct ath10k {
>
> struct work_struct register_work;
> struct work_struct restart_work;
> - struct work_struct recovery_check_work;
> + struct delayed_work recovery_check_work;
> struct work_struct bundle_tx_work;
> struct work_struct tx_complete_work;
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 8e604697d6c2..3c45e5483b60 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -5427,7 +5427,15 @@ static void ath10k_stop(struct ieee80211_hw *hw, bool suspend)
> cancel_work_sync(&ar->set_coverage_class_work);
> cancel_delayed_work_sync(&ar->scan.timeout);
> cancel_work_sync(&ar->restart_work);
> - cancel_work_sync(&ar->recovery_check_work);
> + cancel_delayed_work_sync(&ar->recovery_check_work);
> +
> + /* ath10k_stop() can race with an in-flight recovery. Clear the
> + * recovery state after the works are cancelled so a later
> + * ath10k_start() can start a fresh cycle instead of being blocked
> + * by a stale pending_recovery that nothing will clear.
> + */
> + atomic_set(&ar->fail_cont_count, 0);
> + atomic_set(&ar->pending_recovery, 0);
> }
>
> static int ath10k_config_ps(struct ath10k *ar)
> @@ -8170,6 +8178,9 @@ static void ath10k_reconfig_complete(struct ieee80211_hw *hw,
> ar->state = ATH10K_STATE_ON;
> ieee80211_wake_queues(ar->hw);
>
> + /* Cancel the monitor first so it cannot bump the count after reset. */
> + cancel_delayed_work_sync(&ar->recovery_check_work);
> +
> /* Clear recovery state. */
> complete(&ar->driver_recovery);
> atomic_set(&ar->fail_cont_count, 0);
>
> --
> 2.34.1
>
More information about the ath10k
mailing list