Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 11 Dec 2020 13:51:01 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Vincent Donnefort <vincent.donnefort@arm.com>
Cc:     Valentin Schneider <valentin.schneider@arm.com>,
        linux-kernel@vger.kernel.org, Qian Cai <cai@redhat.com>,
        tglx@linutronix.de, mingo@kernel.org, bigeasy@linutronix.de,
        qais.yousef@arm.com, swood@redhat.com, juri.lelli@redhat.com,
        vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
        rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
        bristot@redhat.com, tj@kernel.org, ouwen210@hotmail.com
Subject: Re: [PATCH 2/2] workqueue: Fix affinity of kworkers attached during
 late hotplug
Message-ID: <20201211125101.GX2414@hirez.programming.kicks-ass.net>
References: <20201210163830.21514-1-valentin.schneider@arm.com>
 <20201210163830.21514-3-valentin.schneider@arm.com>
 <20201211113920.GA75974@e120877-lin.cambridge.arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201211113920.GA75974@e120877-lin.cambridge.arm.com>
Precedence: bulk

On Fri, Dec 11, 2020 at 11:39:21AM +0000, Vincent Donnefort wrote:
> On Thu, Dec 10, 2020 at 04:38:30PM +0000, Valentin Schneider wrote:
> > +	if (pool->flags & POOL_DISASSOCIATED) {
> >  		worker->flags |= WORKER_UNBOUND;
> > +		set_cpus_allowed_ptr(worker->task, cpu_active_mask);
> > +	} else {
> > +		/*
> > +		 * set_cpus_allowed_ptr() will fail if the cpumask doesn't have any
> > +		 * online CPUs. It'll be re-applied when any of the CPUs come up.
> > +		 */
> 
> Does this comment still stand ? IIUC, we should always be in the
> POOL_DISASSOCIATED case if the CPU from cpumask is offline. Unless a
> pool->attrs->cpumask can have several CPUs. In that case maybe we should check
> for the cpu_active_mask here too ?

IIUC it can be a numa mask, and would still be valid in that case.

> > +		set_cpus_allowed_ptr(worker->task, pool->attrs->cpumask);
> > +	}