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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160614112234.GF30154@twins.programming.kicks-ass.net>
Date:	Tue, 14 Jun 2016 13:22:34 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	"Gautham R. Shenoy" <ego@...ux.vnet.ibm.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>, Tejun Heo <htejun@...il.com>,
	Michael Ellerman <mpe@...erman.id.au>,
	Abdul Haleem <abdhalee@...ux.vnet.ibm.com>,
	Aneesh Kumar <aneesh.kumar@...ux.vnet.ibm.com>,
	linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] workqueue:Fix affinity of an unbound worker of a
 node with 1 online CPU

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);
>  }