Subject: Re: [PATCH 2/2] workqueue:Fix affinity of an unbound worker of a node with 1 online CPU

From: Peter Zijlstra <[email-masked]>
Date: Tue, 14 Jun 2016 13:22:34 +0200

On Tue, Jun 07, 2016 at 08:44:03PM +0530, Gautham R. Shenoy wrote:

I'm still puzzled why we don't see this on x86. Afaict there's nothing
PPC specific about this.

> This patch sets the affinity of the worker to
> a) the only online CPU in the cpumask of the worker pool when it comes
>    online.
> b) the cpumask of the worker pool when the second CPU in the pool's
>    cpumask comes online.

This basically works around the WARN conditions, which I suppose is fair
enough, but I would like a note here to revisit this once the whole cpu
hotplug rework has settled.

The real problem is that workqueues seem to want to create worker
threads before there's anybody who would use them or something like
that.

Or is that what PPC does funny? Use an unbound workqueue this early in
cpu bringup?

> @@ -4600,15 +4600,26 @@ static void restore_unbound_workers_cpumask(struct worker_pool *pool, int cpu)
>  	if (!cpumask_test_cpu(cpu, pool->attrs->cpumask))
>  		return;
>  
> -	/* is @cpu the only online CPU? */
>  	cpumask_and(&cpumask, pool->attrs->cpumask, cpu_online_mask);
> -	if (cpumask_weight(&cpumask) != 1)
> +
> +	/*
> +	 * The affinity needs to be set
> +	 * a) to @cpu when that is the only online CPU in
> +	 *    pool->attrs->cpumask.
> +	 * b) to pool->attrs->cpumask when exactly two CPUs in
> +	 *    pool->attrs->cpumask are online. This affinity will be
> +	 *    retained when subsequent CPUs come online.
> +	 */
> +	if (cpumask_weight(&cpumask) > 2)
>  		return;
>  
> +	if (cpumask_weight(&cpumask) == 2)
> +		cpumask_copy(&cpumask, pool->attrs->cpumask);
> +
>  	/* as we're called from CPU_ONLINE, the following shouldn't fail */
>  	for_each_pool_worker(worker, pool)
>  		WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task,
> -						  pool->attrs->cpumask) < 0);
> +						  &cpumask) < 0);
>  }