Skip to content

Commit e36bce4

Browse files
Lai Jiangshanhtejun
authored andcommitted
workqueue: Update the rescuer's affinity only when it is detached
When a rescuer is attached to a pool, its affinity should be only managed by the pool. But updating the detached rescuer's affinity is still meaningful so that it will not disrupt isolated CPUs when it is to be waken up. But the commit d64f2fa ("kernel/workqueue: Let rescuers follow unbound wq cpumask changes") updates the affinity unconditionally, and causes some issues 1) it also changes the affinity when the rescuer is already attached to a pool, which violates the affinity management. 2) the said commit tries to update the affinity of the rescuers, but it misses the rescuers of the PERCPU workqueues, and isolated CPUs can be possibly disrupted by these rescuers when they are summoned. 3) The affinity to set to the rescuers should be consistent in all paths when a rescuer is in detached state. The affinity could be either wq_unbound_cpumask or unbound_effective_cpumask(wq). Related paths: rescuer's worker_detach_from_pool() update wq_unbound_cpumask update wq's cpumask init_rescuer() Both affinities are Ok as long as they are consistent in all paths. But using unbound_effective_cpumask(wq) requres much more code to maintain the consistency, and it doesn't make much sense since the affinity is only effective when the rescuer is not processing works. wq_unbound_cpumask is more favorable. Fix the 1) issue by testing rescuer->pool before updating with wq_pool_attach_mutex held. Fix the 2) issue by moving the rescuer's affinity updating code to the place updating wq_unbound_cpumask and make it also update for PERCPU workqueues. Partially cleanup the 3) consistency issue by using wq_unbound_cpumask. So that the path of "updating wq's cpumask" doesn't need to maintain it. and both the paths of "updating wq_unbound_cpumask" and "rescuer's worker_detach_from_pool()" use wq_unbound_cpumask. Cleanup for init_rescuer()'s consistency for affinity can be done in future. Cc: Juri Lelli <[email protected]> Cc: Waiman Long <[email protected]> Signed-off-by: Lai Jiangshan <[email protected]> Signed-off-by: Tejun Heo <[email protected]>
1 parent 4457265 commit e36bce4

File tree

1 file changed

+5
-5
lines changed

1 file changed

+5
-5
lines changed

kernel/workqueue.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5370,11 +5370,6 @@ static void apply_wqattrs_commit(struct apply_wqattrs_ctx *ctx)
53705370
/* update node_nr_active->max */
53715371
wq_update_node_max_active(ctx->wq, -1);
53725372

5373-
/* rescuer needs to respect wq cpumask changes */
5374-
if (ctx->wq->rescuer)
5375-
set_cpus_allowed_ptr(ctx->wq->rescuer->task,
5376-
unbound_effective_cpumask(ctx->wq));
5377-
53785373
mutex_unlock(&ctx->wq->mutex);
53795374
}
53805375

@@ -6933,6 +6928,11 @@ static int workqueue_apply_unbound_cpumask(const cpumask_var_t unbound_cpumask)
69336928
if (!ret) {
69346929
mutex_lock(&wq_pool_attach_mutex);
69356930
cpumask_copy(wq_unbound_cpumask, unbound_cpumask);
6931+
/* rescuer needs to respect cpumask changes when it is not attached */
6932+
list_for_each_entry(wq, &workqueues, list) {
6933+
if (wq->rescuer && !wq->rescuer->pool)
6934+
unbind_worker(wq->rescuer);
6935+
}
69366936
mutex_unlock(&wq_pool_attach_mutex);
69376937
}
69386938
return ret;

0 commit comments

Comments
 (0)