History

workqueue: Factor out actual cpumask calculation to reduce subtlety in wq_update_pod()

For an unbound pool, multiple cpumasks are involved. U: The user-specified cpumask (may be filtered with cpu_possible_mask). A: The actual cpumask filtered by wq_unbound_cpumask. If the filtering leaves no CPU, wq_unbound_cpumask is used. P: Per-pod subsets of #A. wq->attrs stores #U, wq->dfl_pwq->pool->attrs->cpumask #A, and wq->cpu_pwq[CPU]->pool->attrs->cpumask #P. wq_update_pod() is called to update per-pod pwq's during CPU hotplug. To calculate the new #P for each workqueue, it needs to call wq_calc_pod_cpumask() with @attrs that contains #A. Currently, wq_update_pod() achieves this by calling wq_calc_pod_cpumask() with wq->dfl_pwq->pool->attrs. This is rather fragile because we're calling wq_calc_pod_cpumask() with @attrs of a worker_pool rather than the workqueue's actual attrs when what we want to calculate is the workqueue's cpumask on the pod. While this works fine currently, future changes will add fields which are used differently between workqueues and worker_pools and this subtlety will bite us. This patch factors out #U -> #A calculation from apply_wqattrs_prepare() into wqattrs_actualize_cpumask and updates wq_update_pod() to copy wq->unbound_attrs and use the new helper to obtain #A freshly instead of abusing wq->dfl_pwq->pool_attrs. This shouldn't cause any behavior changes in the current code.

Tejun Heo committed 3 years ago

Commit 0f36ee21 parent 29301551 file · +29 −20Browse files

Showing the changes to kernel/workqueue.c (1 of 1 file). Show all

Unified · Split

kernel/workqueue.c+29−20
@@ -348,6 +348,7 @@ static bool wq_pod_enabled; /* unbound CPU pod affinity enabled */ · 347 unmodified lines
348348
349349 /* buf for wq_update_unbound_pod_attrs(), protected by CPU hotplug exclusion */
350350 static struct workqueue_attrs *wq_update_pod_attrs_buf;
351+static cpumask_var_t wq_update_pod_cpumask_buf;
351352
352353 static DEFINE_MUTEX(wq_pool_mutex); /* protects pools and workqueues list */
353354 static DEFINE_MUTEX(wq_pool_attach_mutex); /* protects worker attach/detach */
@@ -3699,6 +3700,20 @@ static bool wqattrs_equal(const struct workqueue_attrs *a, · 3345 unmodified lines
36993700 return true;
37003701 }
37013702
3703+/* Update @attrs with actually available CPUs */
3704+static void wqattrs_actualize_cpumask(struct workqueue_attrs *attrs,
3705+ const cpumask_t *unbound_cpumask)
3706+{
3707+ /*
3708+ * Calculate the effective CPU mask of @attrs given @unbound_cpumask. If
3709+ * @attrs->cpumask doesn't overlap with @unbound_cpumask, we fallback to
3710+ * @unbound_cpumask.
3711+ */
3712+ cpumask_and(attrs->cpumask, attrs->cpumask, unbound_cpumask);
3713+ if (unlikely(cpumask_empty(attrs->cpumask)))
3714+ cpumask_copy(attrs->cpumask, unbound_cpumask);
3715+}
3716+
37023717 /**
37033718 * init_worker_pool - initialize a newly zalloc'd worker_pool
37043719 * @pool: worker_pool to initialize
@@ -4221,33 +4236,23 @@ apply_wqattrs_prepare(struct workqueue_struct *wq, · 516 unmodified lines
42214236 if (!ctx || !new_attrs || !tmp_attrs)
42224237 goto out_free;
42234238
4224− /*
4225− * Calculate the attrs of the default pwq with unbound_cpumask
4226− * which is wq_unbound_cpumask or to set to wq_unbound_cpumask.
4227− * If the user configured cpumask doesn't overlap with the
4228− * wq_unbound_cpumask, we fallback to the wq_unbound_cpumask.
4229− */
4230− copy_workqueue_attrs(new_attrs, attrs);
4231− cpumask_and(new_attrs->cpumask, new_attrs->cpumask, unbound_cpumask);
4232− if (unlikely(cpumask_empty(new_attrs->cpumask)))
4233− cpumask_copy(new_attrs->cpumask, unbound_cpumask);
4234−
4235− /*
4236− * We may create multiple pwqs with differing cpumasks. Make a
4237− * copy of @new_attrs which will be modified and used to obtain
4238− * pools.
4239− */
4240− copy_workqueue_attrs(tmp_attrs, new_attrs);
4241−
42424239 /*
42434240 * If something goes wrong during CPU up/down, we'll fall back to
42444241 * the default pwq covering whole @attrs->cpumask. Always create
42454242 * it even if we don't use it immediately.
42464243 */
4244+ copy_workqueue_attrs(new_attrs, attrs);
4245+ wqattrs_actualize_cpumask(new_attrs, unbound_cpumask);
42474246 ctx->dfl_pwq = alloc_unbound_pwq(wq, new_attrs);
42484247 if (!ctx->dfl_pwq)
42494248 goto out_free;
42504249
4250+ /*
4251+ * We may create multiple pwqs with differing cpumasks. Make a copy of
4252+ * @new_attrs which will be modified and used to obtain pools.
4253+ */
4254+ copy_workqueue_attrs(tmp_attrs, new_attrs);
4255+
42514256 for_each_possible_cpu(cpu) {
42524257 if (new_attrs->ordered) {
42534258 ctx->dfl_pwq->refcnt++;
@@ -4414,18 +4419,20 @@ static void wq_update_pod(struct workqueue_struct *wq, int cpu, · 160 unmodified lines
44144419 * CPU hotplug exclusion.
44154420 */
44164421 target_attrs = wq_update_pod_attrs_buf;
4417− cpumask = target_attrs->cpumask;
4422+ cpumask = wq_update_pod_cpumask_buf;
44184423
44194424 copy_workqueue_attrs(target_attrs, wq->unbound_attrs);
4425+ wqattrs_actualize_cpumask(target_attrs, wq_unbound_cpumask);
44204426
44214427 /* nothing to do if the target cpumask matches the current pwq */
4422− wq_calc_pod_cpumask(wq->dfl_pwq->pool->attrs, pod, off_cpu, cpumask);
4428+ wq_calc_pod_cpumask(target_attrs, pod, off_cpu, cpumask);
44234429 pwq = rcu_dereference_protected(*per_cpu_ptr(wq->cpu_pwq, cpu),
44244430 lockdep_is_held(&wq_pool_mutex));
44254431 if (cpumask_equal(cpumask, pwq->pool->attrs->cpumask))
44264432 return;
44274433
44284434 /* create a new pwq */
4435+ cpumask_copy(target_attrs->cpumask, cpumask);
44294436 pwq = alloc_unbound_pwq(wq, target_attrs);
44304437 if (!pwq) {
44314438 pr_warn("workqueue: allocation failed while updating CPU pod affinity of \"%s\"\n",
@@ -6285,6 +6292,8 @@ void __init workqueue_init_early(void) · 1853 unmodified lines
62856292 wq_update_pod_attrs_buf = alloc_workqueue_attrs();
62866293 BUG_ON(!wq_update_pod_attrs_buf);
62876294
6295+ BUG_ON(!alloc_cpumask_var(&wq_update_pod_cpumask_buf, GFP_KERNEL));
6296+
62886297 /* initialize CPU pools */
62896298 for_each_possible_cpu(cpu) {
62906299 struct worker_pool *pool;