Re: [PATCH v2 4/5] workqueue: Show all busy workers in stall diagnostics

[Petr Mladek <pmladek@suse.com>]

On Wed 2026-03-18 04:31:08, Breno Leitao wrote:
> On Fri, Mar 13, 2026 at 05:27:40PM +0100, Petr Mladek wrote:
> I agree. We should probably store the last woken worker in the worker_pool
> structure and print it later.
> 
> I've spent some time verifying that the locking and lifecycle management are
> correct. While I'm not completely certain, I believe it's getting closer. An
> extra pair of eyes would be helpful.
> 
> This is the new version of this patch:
> 
> commit feccca7e696ead3272669ee4d4dc02b6946d0faf
> Author: Breno Leitao <leitao@debian.org>
> Date:   Mon Mar 16 09:47:09 2026 -0700
> 
>     workqueue: print diagnostic info when no worker is in running state
>     
>     show_cpu_pool_busy_workers() iterates over busy workers but gives no
>     feedback when none are found in running state, which is a key indicator
>     that a pool may be stuck — unable to wake an idle worker to process
>     pending work.
>     
>     Add a diagnostic message when no running workers are found, reporting
>     pool id, CPU, idle state, and worker counts.  Also trigger a single-CPU
>     backtrace for the stalled CPU.
>     
>     To identify the task most likely responsible for the stall, add
>     last_woken_worker (L: pool->lock) to worker_pool and record it in
>     kick_pool() just before wake_up_process().  This captures the idle
>     worker that was kicked to take over when the last running worker went to
>     sleep; if the pool is now stuck with no running worker, that task is the
>     prime suspect and its backtrace is dumped.
>     
>     Using struct worker * rather than struct task_struct * avoids any
>     lifetime concern: workers are only destroyed via set_worker_dying()
>     which requires pool->lock, and set_worker_dying() clears
>     last_woken_worker when the dying worker matches.  show_cpu_pool_busy_workers()
>     holds pool->lock while calling sched_show_task(), so last_woken_worker
>     is either NULL or points to a live worker with a valid task.  More
>     precisely, set_worker_dying() clears last_woken_worker before setting
>     WORKER_DIE, so a non-NULL last_woken_worker means the kthread has not
>     yet exited and worker->task is still alive.
>     
>     The pool info message is printed inside pool->lock using
>     printk_deferred_enter/exit, the same pattern used by the existing
>     busy-worker loop, to avoid deadlocks with console drivers that queue
>     work while holding locks also taken in their write paths.
>     trigger_single_cpu_backtrace() is called after releasing the lock.
>     
>     Suggested-by: Petr Mladek <pmladek@suse.com>
>     Signed-off-by: Breno Leitao <leitao@debian.org>
> 
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index b77119d71641a..38aebf4514c03 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -7582,20 +7593,58 @@ module_param_named(panic_on_stall_time, wq_panic_on_stall_time, uint, 0644);
>  MODULE_PARM_DESC(panic_on_stall_time, "Panic if stall exceeds this many seconds (0=disabled)");
>  
>  /*
> - * Show workers that might prevent the processing of pending work items.
> - * A busy worker that is not running on the CPU (e.g. sleeping in
> - * wait_event_idle() with PF_WQ_WORKER cleared) can stall the pool just as
> - * effectively as a CPU-bound one, so dump every in-flight worker.
> + * Report that a pool has no worker in running state, which is a sign that the
> + * pool may be stuck. Print pool info. Must be called with pool->lock held and
> + * inside a printk_deferred_enter/exit region.
> + */
> +static void show_pool_no_running_worker(struct worker_pool *pool)
> +{
> +	lockdep_assert_held(&pool->lock);
> +
> +	printk_deferred_enter();
> +	pr_info("pool %d: no worker in running state, cpu=%d is %s (nr_workers=%d nr_idle=%d)\n",
> +		pool->id, pool->cpu,
> +		idle_cpu(pool->cpu) ? "idle" : "busy",
> +		pool->nr_workers, pool->nr_idle);
> +	pr_info("The pool might have trouble waking an idle worker.\n");
> +	/*
> +	 * last_woken_worker and its task are valid here: set_worker_dying()
> +	 * clears it under pool->lock before setting WORKER_DIE, so if
> +	 * last_woken_worker is non-NULL the kthread has not yet exited and
> +	 * worker->task is still alive.
> +	 */
> +	if (pool->last_woken_worker) {
> +		pr_info("Backtrace of last woken worker:\n");
> +		sched_show_task(pool->last_woken_worker->task);
> +	} else {
> +		pr_info("Last woken worker empty\n");

This is a bit ambiguous. It sounds like that the worker is idle.
I would write something like:

		pr_info("There is no info about the last woken worker\n");
		pr_info("Missing info about the last woken worker.\n");

> +	}
> +	printk_deferred_exit();
> +}
> +

Otherwise, I like this patch.

I still think what might be the reason that there is no worker
in the running state. Let's see if this patch brings some useful info.

One more idea. It might be useful to store a timestamp when the last
worker was woken. And then print either the timestamp or delta.
It would help to make sure that kick_pool() was really called
during the reported stall.

Best Regards,
Petr