Re: [PATCH v3] mm/vmalloc: Use dedicated unbound workqueues for vmap drain

Baoquan He
2026-04-01T09:48:07Z

On 03/31/26 at 10:23pm, Uladzislau Rezki (Sony) wrote:
> drain_vmap_area_work() function can take >10ms to complete
> when there are many accumulated vmap areas in a system with
> high CPU count, causing workqueue watchdog warnings when run
> via schedule_work():
>
> workqueue: drain_vmap_area_work hogged CPU for >10000us
>
> Move the top-level drain work to a dedicated WQ_UNBOUND
> workqueue so the scheduler can run this background work
> on any available CPU, improving responsiveness. Use the
> WQ_MEM_RECLAIM to ensure forward progress under memory
> pressure.
>
> Move purge helpers to separate WQ_UNBOUND | WQ_MEM_RECLAIM
> workqueue. This allows drain_vmap_work to wait for helpers
> completion without creating dependency on the same rescuer
> thread and avoid a potential parent/child deadlock.
...snip...
> @@ -2385,29 +2390,31 @@ static bool __purge_vmap_area_lazy(unsigned long start, unsigned long end,
> nr_purge_helpers = atomic_long_read(&vmap_lazy_nr) / lazy_max_pages();
> nr_purge_helpers = clamp(nr_purge_helpers, 1U, nr_purge_nodes) - 1;
>
> - for_each_cpu(i, &purge_nodes) {
> - vn = &vmap_nodes[i];
> + for_each_vmap_node(vn) {
> + vn->work_queued = false;
> +
> + if (list_empty(&vn->purge_list))
> + continue;
>
> if (nr_purge_helpers > 0) {
> INIT_WORK(&vn->purge_work, purge_vmap_node);
> + vn->work_queued = schedule_drain_vmap_work(
> + READ_ONCE(drain_vmap_helpers_wq), &vn->purge_work);

The new schedule_drain_vmap_work() could submit all purge_work on one
CPU, do we need use queue_work_on(cpu, wq, work) instead?

>
> - if (cpumask_test_cpu(i, cpu_online_mask))
> - schedule_work_on(i, &vn->purge_work);
> - else
> - schedule_work(&vn->purge_work);
> -
> - nr_purge_helpers--;
> - } else {
> - vn->purge_work.func = NULL;
> - purge_vmap_node(&vn->purge_work);
> - nr_purged_areas += vn->nr_purged;
> + if (vn->work_queued) {
> + nr_purge_helpers--;
> + continue;
> + }
> }
> - }
>
> - for_each_cpu(i, &purge_nodes) {
> - vn = &vmap_nodes[i];
> + /* Sync path. Process locally. */
> + purge_vmap_node(&vn->purge_work);
> + nr_purged_areas += vn->nr_purged;
> + }
>
> - if (vn->purge_work.func) {
> + /* Wait for completion if queued any. */
> + for_each_vmap_node(vn) {
> + if (vn->work_queued) {
> flush_work(&vn->purge_work);
> nr_purged_areas += vn->nr_purged;
> }
...snip...
> +
> +static int __init vmalloc_init_workqueue(void)
> +{
> + struct workqueue_struct *drain_wq, *helpers_wq;

Maybe there's one local variable is enough like below:

struct workqueue_struct *wq;
unsigned int flags = WQ_UNBOUND | WQ_MEM_RECLAIM;

wq = alloc_workqueue("vmap_drain", flags, 0);
WARN_ON_ONCE(wq == NULL);
WRITE_ONCE(drain_vmap_wq, wq);

wq = alloc_workqueue("vmap_drain_helpers", flags, 0);
WARN_ON_ONCE(wq == NULL);
WRITE_ONCE(drain_vmap_helpers_wq, wq);

return 0;
}

Just personal preference on nitpick, not strong opionion.