workqueue: Protects wq_unbound_cpumask with wq_pool_attach_mutex
authorLai Jiangshan <jiangshan.ljs@antgroup.com>
Thu, 12 Jan 2023 16:14:27 +0000 (16:14 +0000)
committerTejun Heo <tj@kernel.org>
Thu, 12 Jan 2023 16:21:48 +0000 (06:21 -1000)
When unbind_workers() reads wq_unbound_cpumask to set the affinity of
freshly-unbound kworkers, it only holds wq_pool_attach_mutex. This isn't
sufficient as wq_unbound_cpumask is only protected by wq_pool_mutex.

Make wq_unbound_cpumask protected with wq_pool_attach_mutex and also
remove the need of temporary saved_cpumask.

Fixes: 10a5a651e3af ("workqueue: Restrict kworker in the offline CPU pool running on housekeeping CPUs")
Reported-by: Valentin Schneider <vschneid@redhat.com>
Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
kernel/workqueue.c

index 76b4185..55cca6c 100644 (file)
@@ -326,7 +326,7 @@ static struct rcuwait manager_wait = __RCUWAIT_INITIALIZER(manager_wait);
 static LIST_HEAD(workqueues);          /* PR: list of all workqueues */
 static bool workqueue_freezing;                /* PL: have wqs started freezing? */
 
-/* PL: allowable cpus for unbound wqs and work items */
+/* PL&A: allowable cpus for unbound wqs and work items */
 static cpumask_var_t wq_unbound_cpumask;
 
 /* CPU where unbound work was last round robin scheduled from this CPU */
@@ -3956,7 +3956,8 @@ static void apply_wqattrs_cleanup(struct apply_wqattrs_ctx *ctx)
 /* allocate the attrs and pwqs for later installation */
 static struct apply_wqattrs_ctx *
 apply_wqattrs_prepare(struct workqueue_struct *wq,
-                     const struct workqueue_attrs *attrs)
+                     const struct workqueue_attrs *attrs,
+                     const cpumask_var_t unbound_cpumask)
 {
        struct apply_wqattrs_ctx *ctx;
        struct workqueue_attrs *new_attrs, *tmp_attrs;
@@ -3972,14 +3973,15 @@ apply_wqattrs_prepare(struct workqueue_struct *wq,
                goto out_free;
 
        /*
-        * Calculate the attrs of the default pwq.
+        * Calculate the attrs of the default pwq with unbound_cpumask
+        * which is wq_unbound_cpumask or to set to wq_unbound_cpumask.
         * If the user configured cpumask doesn't overlap with the
         * wq_unbound_cpumask, we fallback to the wq_unbound_cpumask.
         */
        copy_workqueue_attrs(new_attrs, attrs);
-       cpumask_and(new_attrs->cpumask, new_attrs->cpumask, wq_unbound_cpumask);
+       cpumask_and(new_attrs->cpumask, new_attrs->cpumask, unbound_cpumask);
        if (unlikely(cpumask_empty(new_attrs->cpumask)))
-               cpumask_copy(new_attrs->cpumask, wq_unbound_cpumask);
+               cpumask_copy(new_attrs->cpumask, unbound_cpumask);
 
        /*
         * We may create multiple pwqs with differing cpumasks.  Make a
@@ -4076,7 +4078,7 @@ static int apply_workqueue_attrs_locked(struct workqueue_struct *wq,
                wq->flags &= ~__WQ_ORDERED;
        }
 
-       ctx = apply_wqattrs_prepare(wq, attrs);
+       ctx = apply_wqattrs_prepare(wq, attrs, wq_unbound_cpumask);
        if (!ctx)
                return -ENOMEM;
 
@@ -5377,7 +5379,7 @@ out_unlock:
 }
 #endif /* CONFIG_FREEZER */
 
-static int workqueue_apply_unbound_cpumask(void)
+static int workqueue_apply_unbound_cpumask(const cpumask_var_t unbound_cpumask)
 {
        LIST_HEAD(ctxs);
        int ret = 0;
@@ -5393,7 +5395,7 @@ static int workqueue_apply_unbound_cpumask(void)
                if (wq->flags & __WQ_ORDERED)
                        continue;
 
-               ctx = apply_wqattrs_prepare(wq, wq->unbound_attrs);
+               ctx = apply_wqattrs_prepare(wq, wq->unbound_attrs, unbound_cpumask);
                if (!ctx) {
                        ret = -ENOMEM;
                        break;
@@ -5408,6 +5410,11 @@ static int workqueue_apply_unbound_cpumask(void)
                apply_wqattrs_cleanup(ctx);
        }
 
+       if (!ret) {
+               mutex_lock(&wq_pool_attach_mutex);
+               cpumask_copy(wq_unbound_cpumask, unbound_cpumask);
+               mutex_unlock(&wq_pool_attach_mutex);
+       }
        return ret;
 }
 
@@ -5426,7 +5433,6 @@ static int workqueue_apply_unbound_cpumask(void)
 int workqueue_set_unbound_cpumask(cpumask_var_t cpumask)
 {
        int ret = -EINVAL;
-       cpumask_var_t saved_cpumask;
 
        /*
         * Not excluding isolated cpus on purpose.
@@ -5440,23 +5446,8 @@ int workqueue_set_unbound_cpumask(cpumask_var_t cpumask)
                        goto out_unlock;
                }
 
-               if (!zalloc_cpumask_var(&saved_cpumask, GFP_KERNEL)) {
-                       ret = -ENOMEM;
-                       goto out_unlock;
-               }
-
-               /* save the old wq_unbound_cpumask. */
-               cpumask_copy(saved_cpumask, wq_unbound_cpumask);
-
-               /* update wq_unbound_cpumask at first and apply it to wqs. */
-               cpumask_copy(wq_unbound_cpumask, cpumask);
-               ret = workqueue_apply_unbound_cpumask();
-
-               /* restore the wq_unbound_cpumask when failed. */
-               if (ret < 0)
-                       cpumask_copy(wq_unbound_cpumask, saved_cpumask);
+               ret = workqueue_apply_unbound_cpumask(cpumask);
 
-               free_cpumask_var(saved_cpumask);
 out_unlock:
                apply_wqattrs_unlock();
        }