[RFC PATCH 4/4] kernel/workqueue: Let rescuers follow unbound wq cpumask changes

Juri Lelli posted 4 patches 1 year, 11 months ago
[RFC PATCH 4/4] kernel/workqueue: Let rescuers follow unbound wq cpumask changes
Posted by Juri Lelli 1 year, 11 months ago
When workqueue cpumask changes are committed the associated rescuer (if
one exists) affinity is not touched and this might be a problem down the
line for isolated setups.

Make sure rescuers affinity is updated every time a workqueue cpumask
changes, so that rescuers can't break isolation.

Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
---
 kernel/workqueue.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 2ef6573909070..df7f2f2bfd0c8 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -4416,6 +4416,12 @@ static void apply_wqattrs_commit(struct apply_wqattrs_ctx *ctx)
 	link_pwq(ctx->dfl_pwq);
 	swap(ctx->wq->dfl_pwq, ctx->dfl_pwq);
 
+	/* rescuer needs to respect wq cpumask changes */
+	if (ctx->wq->rescuer) {
+		set_cpus_allowed_ptr(ctx->wq->rescuer->task, ctx->attrs->cpumask);
+		wake_up_process(ctx->wq->rescuer->task);
+	}
+
 	mutex_unlock(&ctx->wq->mutex);
 }
 
-- 
2.43.0
Re: [RFC PATCH 4/4] kernel/workqueue: Let rescuers follow unbound wq cpumask changes
Posted by Lai Jiangshan 1 year, 11 months ago
Hello, Juri

On Wed, Jan 17, 2024 at 12:20 AM Juri Lelli <juri.lelli@redhat.com> wrote:

> +       /* rescuer needs to respect wq cpumask changes */
> +       if (ctx->wq->rescuer) {
> +               set_cpus_allowed_ptr(ctx->wq->rescuer->task, ctx->attrs->cpumask);
> +               wake_up_process(ctx->wq->rescuer->task);
> +       }
> +

What's the reason to wake up the rescuer?

I support this patch except for the wakeup:
Reviewed-by: Lai Jiangshan <jiangshanlai@gmail.com>

Thanks

Lai
Re: [RFC PATCH 4/4] kernel/workqueue: Let rescuers follow unbound wq cpumask changes
Posted by Juri Lelli 1 year, 11 months ago
Hello Lai,

On 17/01/24 11:56, Lai Jiangshan wrote:
> Hello, Juri
> 
> On Wed, Jan 17, 2024 at 12:20 AM Juri Lelli <juri.lelli@redhat.com> wrote:
> 
> > +       /* rescuer needs to respect wq cpumask changes */
> > +       if (ctx->wq->rescuer) {
> > +               set_cpus_allowed_ptr(ctx->wq->rescuer->task, ctx->attrs->cpumask);
> > +               wake_up_process(ctx->wq->rescuer->task);
> > +       }
> > +
> 
> What's the reason to wake up the rescuer?

I believe we want to wake it up so that it can possibly be moved
"instantly" to a cpu inside its new cpumask affinity. If we don't wake
it up it might be sleeping on a cpu outside its affinity which might
have become isolated and this cpu could be affected by that wakeup if
that only happens later on when possibly the rescuer needs to perform
some work.

Does is make more sense to you?

> I support this patch except for the wakeup:
> Reviewed-by: Lai Jiangshan <jiangshanlai@gmail.com>

Thanks for looking at this!

Juri