Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Date:   Wed, 13 Dec 2023 09:59:42 +0100
From:   Juri Lelli <juri.lelli@redhat.com>
To:     Tejun Heo <tj@kernel.org>
Cc:     Aaron Tomlin <atomlin@atomlin.com>, linux-kernel@vger.kernel.org,
        jiangshanlai@gmail.com, peterz@infradead.org
Subject: Re: [RFC PATCH 0/2] workqueue: Introduce PF_WQ_RESCUE_WORKER
Message-ID: <ZXlyfjDsFGbYcMU6@localhost.localdomain>
References: <20230729135334.566138-1-atomlin@atomlin.com>
 <um77hym4t6zyypfbhwbaeqxpfdzc657oa7vgowdfah7cuctjak@pexots3mfb24>
 <ZXdXdBzvbkI4Y4fL@slm.duckdns.org>
 <ZXguMgcKLCLn16T4@localhost.localdomain>
 <ZXiVCOKk90Fjpmhw@slm.duckdns.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZXiVCOKk90Fjpmhw@slm.duckdns.org>
Precedence: bulk

On 12/12/23 07:14, Tejun Heo wrote:
> Hello, Juri.
> 
> On Tue, Dec 12, 2023 at 10:56:02AM +0100, Juri Lelli wrote:
> > Hummm, OK, but in terms of which CPU the rescuer is possibly woken up,
> > how are we making sure that the wake up is always happening on
> > housekeeping CPUs (assuming unbound workqueues have been restricted to
> > those)?
> > 
> > AFAICS, we have
> > 
> > send_mayday ->
> >   wake_up_process(wq->rescuer->task)
> > 
> > which is not affined to the workqueue cpumask it's called to rescue, so
> > in theory can be woken up anywhere?
> 
> Ah, was only thinking about work item execution. Yeah, it's not following
> the isolation rule there and we probably should affine it as we're waking it
> up.

Something like the following then maybe?

---
 kernel/workqueue.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 2989b57e154a7..ed73f7f80d57d 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -4405,6 +4405,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) {
+               kthread_bind_mask(ctx->wq->rescuer->task, ctx->attrs->cpumask);
+               wake_up_process(ctx->wq->rescuer->task);
+       }
+
        mutex_unlock(&ctx->wq->mutex);
 }