[PATCH -tip V4 0/8] workqueue: break affinity initiatively

[PATCH -tip V4 0/8] workqueue: break affinity initiatively

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

06249738a41a ("workqueue: Manually break affinity on hotplug")
said that scheduler will not force break affinity for us.

But workqueue highly depends on the old behavior. Many parts of the codes
relies on it, 06249738a41a ("workqueue: Manually break affinity on hotplug")
is not enough to change it, and the commit has flaws in itself too.

It doesn't handle for worker detachment.
It doesn't handle for worker attachement, especially worker creation
  which is handled by Valentin Schneider's patch [1].
It doesn't handle for unbound workers which might be possible
per-cpu-kthread.

We need to thoroughly update the way workqueue handles affinity
in cpu hot[un]plug, what is this patchset intends to do and
replace the Valentin Schneider's patch [1].  The equivalent patch
is patch 8 here.

The patchset is based on tip/master rather than workqueue tree,
because the patchset is a complement for 06249738a41a ("workqueue:
Manually break affinity on hotplug") which is only in tip/master by now.

And TJ acked to route the series through tip.

Changed from V3:
	split hotplug callbacks

	introduce break_unbound_workers_cpumask() rather than resuing
	restore_unbound_workers_cpumask().

Changed from V2:
	Drop V2's patch4, which causes warning about setting cpumask
	online&!active to kthread reported by several people:
		Dexuan Cui <decui@microsoft.com>
		kernel test robot <oliver.sang@intel.com>

	Drop V2's patch 1, which can also cause warning about setting
	cpumask online&!active to kthread.  restore_unbound_workers_cpumask()
	is changed when we are bring cpu online.  And it cause V2's patch7
	(V3's patch5) to be changed accordingly.

	Marked patch8 Reviewed-by: Valentin Schneider <valentin.schneider@arm.com>

Changed from V1:
	Add TJ's acked-by for the whole patchset

	Add more words to the comments and the changelog, mainly derived
	from discussion with Peter.

	Update the comments as TJ suggested.
	
	Update a line of code as Valentin suggested.

	Add Valentin's ack for patch 10 because "Seems alright to me." and
	add Valentin's comments to the changelog which is integral.

[1]: https://lore.kernel.org/r/ff62e3ee994efb3620177bf7b19fab16f4866845.camel@redhat.com
[V1 patchset]: https://lore.kernel.org/lkml/20201214155457.3430-1-jiangshanlai@gmail.com/
[V2 patchset]: https://lore.kernel.org/lkml/20201218170919.2950-1-jiangshanlai@gmail.com/
[V3 patchset]: https://lore.kernel.org/lkml/20201226025117.2770-1-jiangshanlai@gmail.com/

Lai Jiangshan (8):
  workqueue: split cpuhotplug callbacks for unbound workqueue
  workqueue: set pool->attr->cpumask to workers when cpu online
  workqueue: use cpu_possible_mask instead of cpu_active_mask to break
    affinity
  workqueue: Manually break affinity on pool detachment
  workqueue: introduce wq_unbound_online_cpumask
  workqueue: use wq_unbound_online_cpumask in
    restore_unbound_workers_cpumask()
  workqueue: Manually break affinity on hotplug for unbound pool
  workqueue: Fix affinity of kworkers when attaching into pool

 include/linux/cpuhotplug.h |   4 +
 include/linux/workqueue.h  |   2 +
 kernel/cpu.c               |   5 +
 kernel/workqueue.c         | 192 +++++++++++++++++++++++++++++--------
 4 files changed, 165 insertions(+), 38 deletions(-)

-- 
2.19.1.6.gb485710b

[PATCH -tip V4 1/8] workqueue: split cpuhotplug callbacks for unbound workqueue

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

Unbound workers are normally non-per-cpu-kthread, but when cpu hotplug,
we also need to update the pools of unbound workqueues based on the info
that whether the relevant node has CPUs online or not for every workqueue.

The code reuses current cpu hotplug callbacks which are designed for
per-cpu workqueues and not well fit with unbound workqueues/pool/workers.

For example workqueue_offline_cpu() is very late, work items of unbound
workqueue might delay offline process or even worse it might cause
offline stopped due to back-to-back work items which are not really
needed to be per-cpu.

And it is also very bad when unbound worker are created after
sched_cpu_deactivate().  set_cpus_allowed_ptr() with online&!active
cpumasks (multi CPUs) will cause warning, and no one will deactivate
such late spawned workers and might cause later BUG_ON().

Similarly, workqueue_online_cpu is verly early, work items of unbound
workqueue might delay online process.  And it is also very bad when
unbound worker are created before sched_cpu_activate().
set_cpus_allowed_ptr() with online&!active cpumasks (multi CPUs) will
cause warning.  For example, the commit d945b5e9f0e("workqueue: Fix
setting affinity of unbound worker threads") fixed it in some cases
of the problem, leaving other cases unfixed and leaving the comment
does not match with the fixing code.

So we need to split cpuhotplug callback for unbound workqueue and
put the new cpuhotplug callbacks in proper places.

Normally, we can split them and put them to CPUHP_AP_ONLINE_DYN.  But it
doesn't solve the problem of set_cpus_allowed_ptr() with online&!active
cpumasks.  So we have to use an offline callback earlier than
sched_cpu_deactivate() and an online callbck later than sched_cpu_activate().

This patch just introduces CPUHP_AP_WORKQUEUE_UNBOUND_ONLINE and
splits the callbacks.  The follow-up fixes are in the later patches.

Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 include/linux/cpuhotplug.h |  4 ++++
 include/linux/workqueue.h  |  2 ++
 kernel/cpu.c               |  5 +++++
 kernel/workqueue.c         | 36 ++++++++++++++++++++++++++----------
 4 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 0042ef362511..ac2103deb20b 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -20,6 +20,9 @@
  *		  |				  ^
  *		  v				  |
  *              AP_ACTIVE			AP_ACTIVE
+ *		  |				  ^
+ *		  v				  |
+ *              ONLINE				ONLINE
  */
 
 enum cpuhp_state {
@@ -194,6 +197,7 @@ enum cpuhp_state {
 	CPUHP_AP_X86_HPET_ONLINE,
 	CPUHP_AP_X86_KVM_CLK_ONLINE,
 	CPUHP_AP_ACTIVE,
+	CPUHP_AP_WORKQUEUE_UNBOUND_ONLINE,
 	CPUHP_ONLINE,
 };
 
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 26de0cae2a0a..98300ddee308 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -663,6 +663,8 @@ static inline void wq_watchdog_touch(int cpu) { }
 int workqueue_prepare_cpu(unsigned int cpu);
 int workqueue_online_cpu(unsigned int cpu);
 int workqueue_offline_cpu(unsigned int cpu);
+int workqueue_unbound_online_cpu(unsigned int cpu);
+int workqueue_unbound_offline_cpu(unsigned int cpu);
 #endif
 
 void __init workqueue_init_early(void);
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 4e11e91010e1..f654ca0a104e 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1665,6 +1665,11 @@ static struct cpuhp_step cpuhp_hp_states[] = {
 		.startup.single		= sched_cpu_activate,
 		.teardown.single	= sched_cpu_deactivate,
 	},
+	[CPUHP_AP_WORKQUEUE_UNBOUND_ONLINE] = {
+		.name			= "workqueue_unbound:online",
+		.startup.single		= workqueue_unbound_online_cpu,
+		.teardown.single	= workqueue_unbound_offline_cpu,
+	},
 #endif
 
 	/* CPU is fully up and running. */
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 9880b6c0e272..d7bdb7885e55 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -5060,6 +5060,29 @@ int workqueue_prepare_cpu(unsigned int cpu)
 }
 
 int workqueue_online_cpu(unsigned int cpu)
+{
+	struct worker_pool *pool;
+
+	for_each_cpu_worker_pool(pool, cpu) {
+		mutex_lock(&wq_pool_attach_mutex);
+		rebind_workers(pool);
+		mutex_unlock(&wq_pool_attach_mutex);
+	}
+
+	return 0;
+}
+
+int workqueue_offline_cpu(unsigned int cpu)
+{
+	/* unbinding per-cpu workers should happen on the local CPU */
+	if (WARN_ON(cpu != smp_processor_id()))
+		return -1;
+
+	unbind_workers(cpu);
+	return 0;
+}
+
+int workqueue_unbound_online_cpu(unsigned int cpu)
 {
 	struct worker_pool *pool;
 	struct workqueue_struct *wq;
@@ -5067,12 +5090,11 @@ int workqueue_online_cpu(unsigned int cpu)
 
 	mutex_lock(&wq_pool_mutex);
 
+	/* update CPU affinity of workers of unbound pools */
 	for_each_pool(pool, pi) {
 		mutex_lock(&wq_pool_attach_mutex);
 
-		if (pool->cpu == cpu)
-			rebind_workers(pool);
-		else if (pool->cpu < 0)
+		if (pool->cpu < 0)
 			restore_unbound_workers_cpumask(pool, cpu);
 
 		mutex_unlock(&wq_pool_attach_mutex);
@@ -5086,16 +5108,10 @@ int workqueue_online_cpu(unsigned int cpu)
 	return 0;
 }
 
-int workqueue_offline_cpu(unsigned int cpu)
+int workqueue_unbound_offline_cpu(unsigned int cpu)
 {
 	struct workqueue_struct *wq;
 
-	/* unbinding per-cpu workers should happen on the local CPU */
-	if (WARN_ON(cpu != smp_processor_id()))
-		return -1;
-
-	unbind_workers(cpu);
-
 	/* update NUMA affinity of unbound workqueues */
 	mutex_lock(&wq_pool_mutex);
 	list_for_each_entry(wq, &workqueues, list)
-- 
2.19.1.6.gb485710b

[PATCH -tip V4 2/8] workqueue: set pool->attr->cpumask to workers when cpu online

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

The commit d945b5e9f0e("workqueue: Fix setting affinity of unbound worker
threads") fixed a problem of set_cpus_allowed_ptr() with online&!active
cpumasks (more than one CPUs) when restore_unbound_workers_cpumask() in
online callback.

But now the new online callback for unbound workqueue is called later
than sched_cpu_activate().  So the cpu is set in cpu_active_mask and
set_cpus_allowed_ptr() with pool->attrs->cpumask in the code won't
cause such warning any more.

The said commit also left comments outdated and causes confusing.
It is better to change the code back.

Cc: Hillf Danton <hdanton@sina.com>
Reported-by: Hillf Danton <hdanton@sina.com>
Acked-by: Tejun Heo <tj@kernel.org>
Tested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 kernel/workqueue.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index d7bdb7885e55..38628e2a622c 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -5039,11 +5039,15 @@ 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)
+		return;
 
 	/* 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, &cpumask) < 0);
+		WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task,
+						  pool->attrs->cpumask) < 0);
 }
 
 int workqueue_prepare_cpu(unsigned int cpu)
-- 
2.19.1.6.gb485710b

[PATCH -tip V4 3/8] workqueue: use cpu_possible_mask instead of cpu_active_mask to break affinity

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

The scheduler won't break affinity for us any more, and we should
"emulate" the same behavior when the scheduler breaks affinity for
us.  The behavior is "changing the cpumask to cpu_possible_mask".

And there might be some other CPUs online later while the worker is
still running with the pending work items.  The worker should be allowed
to use the later online CPUs as before and process the work items ASAP.
If we use cpu_active_mask here, we can't achieve this goal but
using cpu_possible_mask can.

Fixes: 06249738a41a ("workqueue: Manually break affinity on hotplug")
Acked-by: Tejun Heo <tj@kernel.org>
Tested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 kernel/workqueue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 38628e2a622c..3e92bc4f8a36 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -4920,7 +4920,7 @@ static void unbind_workers(int cpu)
 		raw_spin_unlock_irq(&pool->lock);
 
 		for_each_pool_worker(worker, pool)
-			WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task, cpu_active_mask) < 0);
+			WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task, cpu_possible_mask) < 0);
 
 		mutex_unlock(&wq_pool_attach_mutex);
 
-- 
2.19.1.6.gb485710b

[PATCH -tip V4 4/8] workqueue: Manually break affinity on pool detachment

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

The pool->attrs->cpumask might be a single CPU and it may go
down after detachment, and the scheduler won't force to break
affinity for us since it is a per-cpu-ktrhead.  So we have to
do it on our own and unbind this worker which can't be unbound
by workqueue_offline_cpu() since it doesn't belong to any pool
after detachment.  Do it unconditionally for there is no harm
to break affinity for non-per-cpu-ktrhead and we don't need to
rely on the scheduler's policy on when to break affinity.

Fixes: 06249738a41a ("workqueue: Manually break affinity on hotplug")
Acked-by: Tejun Heo <tj@kernel.org>
Tested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 kernel/workqueue.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 3e92bc4f8a36..aed08eddeb83 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1888,6 +1888,19 @@ static void worker_detach_from_pool(struct worker *worker)
 
 	if (list_empty(&pool->workers))
 		detach_completion = pool->detach_completion;
+
+	/*
+	 * The pool->attrs->cpumask might be a single CPU and it may go
+	 * down after detachment, and the scheduler won't force to break
+	 * affinity for us since it is a per-cpu-ktrhead.  So we have to
+	 * do it on our own and unbind this worker which can't be unbound
+	 * by workqueue_offline_cpu() since it doesn't belong to any pool
+	 * after detachment.  Do it unconditionally for there is no harm
+	 * to break affinity for non-per-cpu-ktrhead and we don't need to
+	 * rely on the scheduler's policy on when to break affinity.
+	 */
+	set_cpus_allowed_ptr(worker->task, cpu_possible_mask);
+
 	mutex_unlock(&wq_pool_attach_mutex);
 
 	/* clear leftover flags without pool->lock after it is detached */
-- 
2.19.1.6.gb485710b

[PATCH -tip V4 5/8] workqueue: introduce wq_unbound_online_cpumask

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

wq_unbound_online_cpumask is the cached result of cpu_online_mask with
the going-down cpu cleared before the cpu is cleared from cpu_active_mask.
It is used to track the cpu hotplug process so the creation/attachment
of unbound workers can know where it is in the process when there is
ongoing cpu hotplug and so that workqueue can cooperate with hotplug and
scheduler correctly in later patches for setting correct cpumask for
workers and break affinity initiatively.

The first usage of wq_unbound_online_cpumask is also in this patch.
wq_calc_node_cpumask() and wq_update_unbound_numa() can be simplified
a little.

Acked-by: Tejun Heo <tj@kernel.org>
Tested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 kernel/workqueue.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index aed08eddeb83..d01cca8e51f9 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -310,6 +310,9 @@ static bool workqueue_freezing;		/* PL: have wqs started freezing? */
 /* PL: allowable cpus for unbound wqs and work items */
 static cpumask_var_t wq_unbound_cpumask;
 
+/* PL: online cpus (cpu_online_mask with the going-down cpu cleared) */
+static cpumask_var_t wq_unbound_online_cpumask;
+
 /* CPU where unbound work was last round robin scheduled from this CPU */
 static DEFINE_PER_CPU(int, wq_rr_cpu_last);
 
@@ -3835,12 +3838,10 @@ static struct pool_workqueue *alloc_unbound_pwq(struct workqueue_struct *wq,
  * wq_calc_node_cpumask - calculate a wq_attrs' cpumask for the specified node
  * @attrs: the wq_attrs of the default pwq of the target workqueue
  * @node: the target NUMA node
- * @cpu_going_down: if >= 0, the CPU to consider as offline
  * @cpumask: outarg, the resulting cpumask
  *
- * Calculate the cpumask a workqueue with @attrs should use on @node.  If
- * @cpu_going_down is >= 0, that cpu is considered offline during
- * calculation.  The result is stored in @cpumask.
+ * Calculate the cpumask a workqueue with @attrs should use on @node.
+ * The result is stored in @cpumask.
  *
  * If NUMA affinity is not enabled, @attrs->cpumask is always used.  If
  * enabled and @node has online CPUs requested by @attrs, the returned
@@ -3854,15 +3855,14 @@ static struct pool_workqueue *alloc_unbound_pwq(struct workqueue_struct *wq,
  * %false if equal.
  */
 static bool wq_calc_node_cpumask(const struct workqueue_attrs *attrs, int node,
-				 int cpu_going_down, cpumask_t *cpumask)
+				 cpumask_t *cpumask)
 {
 	if (!wq_numa_enabled || attrs->no_numa)
 		goto use_dfl;
 
 	/* does @node have any online CPUs @attrs wants? */
 	cpumask_and(cpumask, cpumask_of_node(node), attrs->cpumask);
-	if (cpu_going_down >= 0)
-		cpumask_clear_cpu(cpu_going_down, cpumask);
+	cpumask_and(cpumask, cpumask, wq_unbound_online_cpumask);
 
 	if (cpumask_empty(cpumask))
 		goto use_dfl;
@@ -3971,7 +3971,7 @@ apply_wqattrs_prepare(struct workqueue_struct *wq,
 		goto out_free;
 
 	for_each_node(node) {
-		if (wq_calc_node_cpumask(new_attrs, node, -1, tmp_attrs->cpumask)) {
+		if (wq_calc_node_cpumask(new_attrs, node, tmp_attrs->cpumask)) {
 			ctx->pwq_tbl[node] = alloc_unbound_pwq(wq, tmp_attrs);
 			if (!ctx->pwq_tbl[node])
 				goto out_free;
@@ -4096,7 +4096,6 @@ int apply_workqueue_attrs(struct workqueue_struct *wq,
  * wq_update_unbound_numa - update NUMA affinity of a wq for CPU hot[un]plug
  * @wq: the target workqueue
  * @cpu: the CPU coming up or going down
- * @online: whether @cpu is coming up or going down
  *
  * This function is to be called from %CPU_DOWN_PREPARE, %CPU_ONLINE and
  * %CPU_DOWN_FAILED.  @cpu is being hot[un]plugged, update NUMA affinity of
@@ -4114,11 +4113,9 @@ int apply_workqueue_attrs(struct workqueue_struct *wq,
  * affinity, it's the user's responsibility to flush the work item from
  * CPU_DOWN_PREPARE.
  */
-static void wq_update_unbound_numa(struct workqueue_struct *wq, int cpu,
-				   bool online)
+static void wq_update_unbound_numa(struct workqueue_struct *wq, int cpu)
 {
 	int node = cpu_to_node(cpu);
-	int cpu_off = online ? -1 : cpu;
 	struct pool_workqueue *old_pwq = NULL, *pwq;
 	struct workqueue_attrs *target_attrs;
 	cpumask_t *cpumask;
@@ -4146,7 +4143,7 @@ static void wq_update_unbound_numa(struct workqueue_struct *wq, int cpu,
 	 * and create a new one if they don't match.  If the target cpumask
 	 * equals the default pwq's, the default pwq should be used.
 	 */
-	if (wq_calc_node_cpumask(wq->dfl_pwq->pool->attrs, node, cpu_off, cpumask)) {
+	if (wq_calc_node_cpumask(wq->dfl_pwq->pool->attrs, node, cpumask)) {
 		if (cpumask_equal(cpumask, pwq->pool->attrs->cpumask))
 			return;
 	} else {
@@ -5106,6 +5103,7 @@ int workqueue_unbound_online_cpu(unsigned int cpu)
 	int pi;
 
 	mutex_lock(&wq_pool_mutex);
+	cpumask_set_cpu(cpu, wq_unbound_online_cpumask);
 
 	/* update CPU affinity of workers of unbound pools */
 	for_each_pool(pool, pi) {
@@ -5119,7 +5117,7 @@ int workqueue_unbound_online_cpu(unsigned int cpu)
 
 	/* update NUMA affinity of unbound workqueues */
 	list_for_each_entry(wq, &workqueues, list)
-		wq_update_unbound_numa(wq, cpu, true);
+		wq_update_unbound_numa(wq, cpu);
 
 	mutex_unlock(&wq_pool_mutex);
 	return 0;
@@ -5131,8 +5129,9 @@ int workqueue_unbound_offline_cpu(unsigned int cpu)
 
 	/* update NUMA affinity of unbound workqueues */
 	mutex_lock(&wq_pool_mutex);
+	cpumask_clear_cpu(cpu, wq_unbound_online_cpumask);
 	list_for_each_entry(wq, &workqueues, list)
-		wq_update_unbound_numa(wq, cpu, false);
+		wq_update_unbound_numa(wq, cpu);
 	mutex_unlock(&wq_pool_mutex);
 
 	return 0;
@@ -5969,6 +5968,9 @@ void __init workqueue_init_early(void)
 
 	BUILD_BUG_ON(__alignof__(struct pool_workqueue) < __alignof__(long long));
 
+	BUG_ON(!alloc_cpumask_var(&wq_unbound_online_cpumask, GFP_KERNEL));
+	cpumask_copy(wq_unbound_online_cpumask, cpu_online_mask);
+
 	BUG_ON(!alloc_cpumask_var(&wq_unbound_cpumask, GFP_KERNEL));
 	cpumask_copy(wq_unbound_cpumask, housekeeping_cpumask(hk_flags));
 
@@ -6065,7 +6067,7 @@ void __init workqueue_init(void)
 	}
 
 	list_for_each_entry(wq, &workqueues, list) {
-		wq_update_unbound_numa(wq, smp_processor_id(), true);
+		wq_update_unbound_numa(wq, smp_processor_id());
 		WARN(init_rescuer(wq),
 		     "workqueue: failed to create early rescuer for %s",
 		     wq->name);
-- 
2.19.1.6.gb485710b

[PATCH -tip V4 6/8] workqueue: use wq_unbound_online_cpumask in restore_unbound_workers_cpumask()

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

restore_unbound_workers_cpumask() is called when CPU_ONLINE, where
wq_online_cpumask equals to cpu_online_mask.  So no fucntionality
changed.

Acked-by: Tejun Heo <tj@kernel.org>
Tested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 kernel/workqueue.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index d01cca8e51f9..f2793749bd97 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -5043,6 +5043,7 @@ static void restore_unbound_workers_cpumask(struct worker_pool *pool, int cpu)
 	static cpumask_t cpumask;
 	struct worker *worker;
 
+	lockdep_assert_held(&wq_pool_mutex);
 	lockdep_assert_held(&wq_pool_attach_mutex);
 
 	/* is @cpu allowed for @pool? */
@@ -5050,7 +5051,7 @@ static void restore_unbound_workers_cpumask(struct worker_pool *pool, int cpu)
 		return;
 
 	/* is @cpu the only online CPU? */
-	cpumask_and(&cpumask, pool->attrs->cpumask, cpu_online_mask);
+	cpumask_and(&cpumask, pool->attrs->cpumask, wq_unbound_online_cpumask);
 	if (cpumask_weight(&cpumask) != 1)
 		return;
 
-- 
2.19.1.6.gb485710b

[PATCH -tip V4 7/8] workqueue: Manually break affinity on hotplug for unbound pool

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

There is possible that a per-node pool/woker's affinity is a single
CPU.  It can happen when the workqueue user changes the cpumask of the
workqueue or when wq_unbound_cpumask is changed by system adim via
/sys/devices/virtual/workqueue/cpumask.  And pool->attrs->cpumask
is workqueue's cpumask & wq_unbound_cpumask & possible_cpumask_of_the_node,
which can be a single CPU and makes the pool's workers to be "per cpu
kthread".

And the scheduler won't break affinity on the "per cpu kthread" workers
when the CPU is going down, so we have to do it by ourselves.

We do it by introducing new break_unbound_workers_cpumask() which is a
symmetric version of restore_unbound_workers_cpumask().   When the last
online CPU of the pool goes down, it is time to break the affinity.

The way to break affinity is to set the workers' affinity to
cpu_possible_mask, so that we preserve the same behavisor when
the scheduler breaks affinity for us.

Fixes: 06249738a41a ("workqueue: Manually break affinity on hotplug")
Acked-by: Tejun Heo <tj@kernel.org>
Tested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 kernel/workqueue.c | 65 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 62 insertions(+), 3 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index f2793749bd97..b012adbeff9f 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -5035,8 +5035,9 @@ static void rebind_workers(struct worker_pool *pool)
  *
  * An unbound pool may end up with a cpumask which doesn't have any online
  * CPUs.  When a worker of such pool get scheduled, the scheduler resets
- * its cpus_allowed.  If @cpu is in @pool's cpumask which didn't have any
- * online CPU before, cpus_allowed of all its workers should be restored.
+ * its cpus_allowed or we had reset it earlier in break_unbound_workers_cpumask().
+ * If @cpu is in @pool's cpumask which didn't have any online CPU before,
+ * cpus_allowed of all its workers should be restored.
  */
 static void restore_unbound_workers_cpumask(struct worker_pool *pool, int cpu)
 {
@@ -5061,6 +5062,50 @@ static void restore_unbound_workers_cpumask(struct worker_pool *pool, int cpu)
 						  pool->attrs->cpumask) < 0);
 }
 
+/**
+ * break_unbound_workers_cpumask - break cpumask of unbound workers
+ * @pool: unbound pool of interest
+ * @cpu: the CPU which is going down
+ *
+ * An unbound pool may end up with a cpumask which doesn't have any online
+ * CPUs.  When a worker of such pool get scheduled, the scheduler resets
+ * its cpus_allowed unless there is only one CPU in the cpus_allowed which
+ * is the special case we need to handle it on our own and avoid blocking
+ * the hotplug process or causing further harms.
+ */
+static void break_unbound_workers_cpumask(struct worker_pool *pool, int cpu)
+{
+	struct worker *worker;
+
+	lockdep_assert_held(&wq_pool_mutex);
+	lockdep_assert_held(&wq_pool_attach_mutex);
+
+	/* is @cpu allowed for @pool? */
+	if (!cpumask_test_cpu(cpu, pool->attrs->cpumask))
+		return;
+
+	/*
+	 * is @cpu the last online for @pool?  If so, the scheduler or we
+	 * need to break affinity for the workers.
+	 */
+	if (cpumask_intersects(pool->attrs->cpumask, wq_unbound_online_cpumask))
+		return;
+
+	/*
+	 * is @cpu the only possible CPU for @pool?  If not, scheduler
+	 * will take care of breaking affinity for the workers since the
+	 * workers are all non-per-cpu-kthread.  It is the usual case
+	 * for unbound pools/workers and we don't need to bother to do it.
+	 */
+	if (cpumask_weight(pool->attrs->cpumask) > 1)
+		return;
+
+	/* as we're setting it to cpu_possible_mask, the following shouldn't fail */
+	for_each_pool_worker(worker, pool)
+		WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task,
+						  cpu_possible_mask) < 0);
+}
+
 int workqueue_prepare_cpu(unsigned int cpu)
 {
 	struct worker_pool *pool;
@@ -5126,13 +5171,27 @@ int workqueue_unbound_online_cpu(unsigned int cpu)
 
 int workqueue_unbound_offline_cpu(unsigned int cpu)
 {
+	struct worker_pool *pool;
 	struct workqueue_struct *wq;
+	int pi;
 
-	/* update NUMA affinity of unbound workqueues */
 	mutex_lock(&wq_pool_mutex);
 	cpumask_clear_cpu(cpu, wq_unbound_online_cpumask);
+
+	/* update CPU affinity of workers of unbound pools */
+	for_each_pool(pool, pi) {
+		mutex_lock(&wq_pool_attach_mutex);
+
+		if (pool->cpu < 0)
+			break_unbound_workers_cpumask(pool, cpu);
+
+		mutex_unlock(&wq_pool_attach_mutex);
+	}
+
+	/* update NUMA affinity of unbound workqueues */
 	list_for_each_entry(wq, &workqueues, list)
 		wq_update_unbound_numa(wq, cpu);
+
 	mutex_unlock(&wq_pool_mutex);
 
 	return 0;
-- 
2.19.1.6.gb485710b

[PATCH -tip V4 8/8] workqueue: Fix affinity of kworkers when attaching into pool

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

When worker_attach_to_pool() is called, we should not put the workers
to pool->attrs->cpumask when there is or will be no CPU online in it.

Otherwise, it may cause BUG_ON(): (quote from Valentin:)
  Per-CPU kworkers forcefully migrated away by hotplug via
  workqueue_offline_cpu() can end up spawning more kworkers via

    manage_workers() -> maybe_create_worker()

  Workers created at this point will be bound using

    pool->attrs->cpumask

  which in this case is wrong, as the hotplug state machine already
  migrated all pinned kworkers away from this CPU. This ends up
  triggering the BUG_ON condition is sched_cpu_dying() (i.e. there's
  a kworker enqueued on the dying rq).
(end of quote)

We need to find out where it is in the hotplug stages to determind
whether pool->attrs->cpumask is valid.  So we have to check
%POOL_DISASSOCIATED and wq_unbound_online_cpumask which are indications
for the hotplug stages.

So for per-CPU kworker case, %POOL_DISASSOCIATED marks the kworkers
of the pool are bound or unboud, so it is used to detect whether
pool->attrs->cpumask is valid to use when attachment.

For unbound workers, we should not set online&!active cpumask to workers.
Just introduced wq_unound_online_cpumask has the features that going-down
cpu is cleared earlier in it than in cpu_active_mask and bring-up cpu
is set later in it than cpu_active_mask.  So it is perfect to be used to
detect whether the pool->attrs->cpumask is valid to use.

To use wq_unound_online_cpumask in worker_attach_to_pool(), we need to protect
wq_unbound_online_cpumask in wq_pool_attach_mutex.

Cc: Qian Cai <cai@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Vincent Donnefort <vincent.donnefort@arm.com>
Link: https://lore.kernel.org/lkml/20201210163830.21514-3-valentin.schneider@arm.com/
Link: https://lore.kernel.org/r/ff62e3ee994efb3620177bf7b19fab16f4866845.camel@redhat.com
Reported-by: Qian Cai <cai@redhat.com>
Reviewed-by: Valentin Schneider <valentin.schneider@arm.com>
Acked-by: Tejun Heo <tj@kernel.org>
Tested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 kernel/workqueue.c | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index b012adbeff9f..d1f1b863c52a 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -310,7 +310,7 @@ static bool workqueue_freezing;		/* PL: have wqs started freezing? */
 /* PL: allowable cpus for unbound wqs and work items */
 static cpumask_var_t wq_unbound_cpumask;
 
-/* PL: online cpus (cpu_online_mask with the going-down cpu cleared) */
+/* PL&A: online cpus (cpu_online_mask with the going-down cpu cleared) */
 static cpumask_var_t wq_unbound_online_cpumask;
 
 /* CPU where unbound work was last round robin scheduled from this CPU */
@@ -1849,19 +1849,36 @@ static struct worker *alloc_worker(int node)
 static void worker_attach_to_pool(struct worker *worker,
 				   struct worker_pool *pool)
 {
+	bool pool_cpumask_active;
+
 	mutex_lock(&wq_pool_attach_mutex);
 
 	/*
-	 * 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.
+	 * The wq_pool_attach_mutex ensures %POOL_DISASSOCIATED and
+	 * wq_unbound_online_cpumask remain stable across this function.
+	 * See the comments above the definitions of the flag and
+	 * wq_unbound_online_cpumask for details.
+	 *
+	 * For percpu pools, whether pool->attrs->cpumask is legitimate
+	 * for @worker task depends on where it is in the hotplug stages
+	 * divided by workqueue_online/offline_cpu().  Refer the functions
+	 * to see how they toggle %POOL_DISASSOCIATED and update cpumask
+	 * of the workers.
+	 *
+	 * For unbound pools, whether pool->attrs->cpumask is legitimate
+	 * for @worker task depends on where it is in the hotplug stages
+	 * divided by workqueue_unbound_online/offline_cpu().  Refer the
+	 * functions to see how they update wq_unbound_online_cpumask and
+	 * update cpumask of the workers.
 	 */
-	set_cpus_allowed_ptr(worker->task, pool->attrs->cpumask);
+	pool_cpumask_active = pool->cpu >= 0 ? !(pool->flags & POOL_DISASSOCIATED) :
+		cpumask_intersects(pool->attrs->cpumask, wq_unbound_online_cpumask);
+
+	if (pool_cpumask_active)
+		WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task, pool->attrs->cpumask) < 0);
+	else
+		WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task, cpu_possible_mask) < 0);
 
-	/*
-	 * The wq_pool_attach_mutex ensures %POOL_DISASSOCIATED remains
-	 * stable across this function.  See the comments above the flag
-	 * definition for details.
-	 */
 	if (pool->flags & POOL_DISASSOCIATED)
 		worker->flags |= WORKER_UNBOUND;
 
@@ -5149,7 +5166,9 @@ int workqueue_unbound_online_cpu(unsigned int cpu)
 	int pi;
 
 	mutex_lock(&wq_pool_mutex);
+	mutex_lock(&wq_pool_attach_mutex);
 	cpumask_set_cpu(cpu, wq_unbound_online_cpumask);
+	mutex_unlock(&wq_pool_attach_mutex);
 
 	/* update CPU affinity of workers of unbound pools */
 	for_each_pool(pool, pi) {
@@ -5176,7 +5195,9 @@ int workqueue_unbound_offline_cpu(unsigned int cpu)
 	int pi;
 
 	mutex_lock(&wq_pool_mutex);
+	mutex_lock(&wq_pool_attach_mutex);
 	cpumask_clear_cpu(cpu, wq_unbound_online_cpumask);
+	mutex_unlock(&wq_pool_attach_mutex);
 
 	/* update CPU affinity of workers of unbound pools */
 	for_each_pool(pool, pi) {
-- 
2.19.1.6.gb485710b

[tip: sched/urgent] workqueue: Use cpu_possible_mask instead of cpu_active_mask to break affinity

[tip-bot2 for Lai Jiangshan]

The following commit has been merged into the sched/urgent branch of tip:

Commit-ID:     547a77d02f8cfb345631ce23b5b548d27afa0fc4
Gitweb:        https://git.kernel.org/tip/547a77d02f8cfb345631ce23b5b548d27afa0fc4
Author:        Lai Jiangshan <laijs@linux.alibaba.com>
AuthorDate:    Mon, 11 Jan 2021 23:26:33 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 22 Jan 2021 15:09:41 +01:00

workqueue: Use cpu_possible_mask instead of cpu_active_mask to break affinity

The scheduler won't break affinity for us any more, and we should
"emulate" the same behavior when the scheduler breaks affinity for
us.  The behavior is "changing the cpumask to cpu_possible_mask".

And there might be some other CPUs online later while the worker is
still running with the pending work items.  The worker should be allowed
to use the later online CPUs as before and process the work items ASAP.
If we use cpu_active_mask here, we can't achieve this goal but
using cpu_possible_mask can.

Fixes: 06249738a41a ("workqueue: Manually break affinity on hotplug")
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Valentin Schneider <valentin.schneider@arm.com>
Acked-by: Tejun Heo <tj@kernel.org>
Tested-by: Paul E. McKenney <paulmck@kernel.org>
Tested-by: Valentin Schneider <valentin.schneider@arm.com>
Link: https://lkml.kernel.org/r/20210111152638.2417-4-jiangshanlai@gmail.com
---
 kernel/workqueue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 9880b6c..1646331 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -4920,7 +4920,7 @@ static void unbind_workers(int cpu)
 		raw_spin_unlock_irq(&pool->lock);
 
 		for_each_pool_worker(worker, pool)
-			WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task, cpu_active_mask) < 0);
+			WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task, cpu_possible_mask) < 0);
 
 		mutex_unlock(&wq_pool_attach_mutex);