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

[Uladzislau Rezki <urezki@gmail.com>]

On Thu, Sep 17, 2026 at 08:07:37AM +0800, Hillf Danton wrote:
> On Wed, 16 Sep 2026 18:00:20 +0200 "Uladzislau Rezki (Sony)" wrote:
> > On Wed, Sep 16, 2026 at 08:12:27AM +0800, Hillf Danton wrote:
> > > On Mon, 14 Sep 2026 18:56:26 +0200 "Uladzislau Rezki (Sony)" wrote:
> > > > This patch does not use queue_work_on() semantic thus i do not want to
> > > > queue all helpers on current CPU. Instead scheduler does balancing and
> > > > that is it.
> > > > 
> > > [Fair queue in the Eric Dumazet accent]
> > > 
> > > +static bool
> > > +schedule_drain_vmap_work(struct workqueue_struct *wq,
> > > +		struct work_struct *work)
> > > +{
> > > +	if (wq)
> > > +		return queue_work(wq, work);
> > > +
> > > +	return false;
> > > +}
> > > +
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/workqueue.h#n697
> > > 
> > > static inline bool queue_work(struct workqueue_struct *wq,
> > > 			      struct work_struct *work)
> > > {
> > > 	return queue_work_on(WORK_CPU_UNBOUND, wq, work);
> > > }
> > > 
> > > queue_work_on
> > >   __queue_work
> > > 	if (req_cpu == WORK_CPU_UNBOUND) {
> > > 		if (wq->flags & WQ_UNBOUND)
> > > 			cpu = wq_select_unbound_cpu(raw_smp_processor_id());
> > > 		else
> > > 			cpu = raw_smp_processor_id();
> > > 	}
> > > /*
> > >  * When queueing an unbound work item to a wq, prefer local CPU if allowed
> > >  * by wq_unbound_cpumask.  Otherwise, round robin among the allowed ones to
> > >  * avoid perturbing sensitive tasks.
> > >  */
> > > static int wq_select_unbound_cpu(int cpu)
> > > {
> > >  	pr_warn_once("workqueue: round-robin CPU selection forced, expect performance impact\n");
> > > }
> > > 
> > /**
> >  * worker_attach_to_pool() - attach a worker to a pool
> >  * @worker: worker to be attached
> >  * @pool: the target pool
> >  *
> >  * Attach @worker to @pool.  Once attached, the %WORKER_UNBOUND flag and
> >  * cpu-binding of @worker are kept coordinated with the pool across
> >  * cpu-[un]hotplugs.
> >  */
> > static void worker_attach_to_pool(struct worker *worker,
> > 				  struct worker_pool *pool)
> > {
> > 	mutex_lock(&wq_pool_attach_mutex);
> > 
> > 	/*
> > 	 * The wq_pool_attach_mutex ensures %POOL_DISASSOCIATED remains stable
> > 	 * across this function. See the comments above the flag definition for
> > 	 * details. BH workers are, while per-CPU, always DISASSOCIATED.
> > 	 */
> > 	if (pool->flags & POOL_DISASSOCIATED) {
> > 		worker->flags |= WORKER_UNBOUND;
> > 	} else {
> > 		WARN_ON_ONCE(pool->flags & POOL_BH);
> > 		kthread_set_per_cpu(worker->task, pool->cpu);
> > 	}
> > ...
> > }
> > 
> > A local CPU preference for WQ_UNBOUND is not the same as executing on
> > the __bound__ per-CPU system kworker.
> >
> As Ulad, like Yu Zhao, is one of the couple black horses I saw in mm the
> past a couple years, lad, I make the difference between BOUND and UNBOUND
> workers as clear as it is.
>
This is very good :)

> 
> Given numa node1 including cpu8-15 without cpu hotplug cared, a bound worker
> for cpu9 can not migrate to any other cpu, while a unbound worker can run on
> any cpu of node1, that is all.
> 
Right. 

<snip>
	if (list_empty(&pwq->inactive_works) && pwq_tryinc_nr_active(pwq, false)) {
		if (list_empty(&pool->worklist))
			pool->last_progress_ts = jiffies;

		trace_workqueue_activate_work(work);
		insert_work(pwq, work, &pool->worklist, work_flags);
		kick_pool_pick(pool, &wake_task);
	} else {
		work_flags |= WORK_STRUCT_INACTIVE;
		insert_work(pwq, work, &pwq->inactive_works, work_flags);
	}

out:
	raw_spin_unlock(&pool->lock);
	if (wake_task)
		wake_up_process(wake_task);
<snip>

wake_up_process(wake_task) - this guy takes care about task placement.
For us it is TASK_FAIR thus select_task_rq_fair() decides the fate of
unbound kworker.

> Important UN/BOUND have nothing to do with eevdf (and balancing cpus) because
> of different layers.
> 
> And at best I suspect what you missed is the difference between drain_vmap_work
> and lru_add_drain_work, but I do not like the latter as it annoyed the RT/full
> nohz apps more than thought [11].
> 
> static DECLARE_WORK(drain_vmap_work, drain_vmap_area_work);
> static DEFINE_PER_CPU(struct work_struct, lru_add_drain_work);
> 
> [11] Subject: [PATCH v4 3/4] swap: apply new pw_queue_on() interface
> https://lore.kernel.org/lkml/20260519012754.240804-4-leobras.c@gmail.com/
>
I am not sure a drain_vmap_work should be per-cpu, IMO.

Thanks!

--
Uladzislau Rezki