All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lai Jiangshan <jiangshanlai@gmail.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Lai Jiangshan <laijs@linux.alibaba.com>,
	Hillf Danton <hdanton@sina.com>,
	Valentin Schneider <valentin.schneider@arm.com>,
	Qian Cai <cai@redhat.com>,
	Vincent Donnefort <vincent.donnefort@arm.com>,
	Tejun Heo <tj@kernel.org>
Subject: Re: [PATCH 00/10] workqueue: break affinity initiatively
Date: Tue, 15 Dec 2020 13:44:53 +0800	[thread overview]
Message-ID: <CAJhGHyDYbU2=dQyiyx-AC+wu9=B7mTvsbU=QYPyV4xTE8PFPog@mail.gmail.com> (raw)
In-Reply-To: <X9eirnnrD/+VqqEo@hirez.programming.kicks-ass.net>

On Tue, Dec 15, 2020 at 1:36 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Dec 14, 2020 at 11:54:47PM +0800, Lai Jiangshan wrote:
> > From: Lai Jiangshan <laijs@linux.alibaba.com>
> >
> > 06249738a41a ("workqueue: Manually break affinity on hotplug")
> > said that scheduler will not force break affinity for us.
> >
> > But workqueue highly depends on the old behavior. Many parts of the codes
> > relies on it, 06249738a41a ("workqueue: Manually break affinity on hotplug")
> > is not enough to change it, and the commit has flaws in itself too.
> >
> > We need to thoroughly update the way workqueue handles affinity
> > in cpu hot[un]plug, what is this patchset intends to do and
> > replace the Valentin Schneider's patch [1].
>
> So the actual problem is with per-cpu kthreads, the new assumption is
> that hot-un-plug will make all per-cpu kthreads for the dying CPU go
> away.

Hello, Peter

"new assumption" is all needed to be aligned. I haven't read the code.
I thought I understood to some extent which is enough for me to know
that workqueue does violate that.

Workqueue does not break affinity for all per-cpu kthreads in several
cases such as hot-un-plug and workers detaching from pool (those workers
will not be searchable from pools and should be handled alike to hot-un-plug).

But workqueue has not only per-cpu kthreads but also per-node threads.
And per-node threads may be bound to multiple CPUs or may be bound to
a single CPU. I don't know how the scheduler distinguishes all these
different cases under the "new assumption". But at least workqueue
handle these different cases at the same few places.  Since workqueue
have to "break affinity" for per-cpu kthreads, it can also "break affinity"
for other cases. Making workqueue totally do not rely on scheduler's
work to "break affinity" is worth doing since we have to do it for the
most parts.

I haven't read the code about "new assumption", if possible, I'll first
try to find out how will scheduler handle these cases:

If a per-node thread has only cpu 4, and when it goes down, does
workqueue need to "break affinity" for it?

If a per-node thread has only cpu 41,42, and when both go down, does
workqueue need to "break affinity" for it?

Thanks
Lai

>
> Workqueues violated that. I fixed the obvious site, and Valentin's patch
> avoids workqueues from quickly creating new ones while we're not
> looking.
>
> What other problems did you find?

  reply	other threads:[~2020-12-15  5:46 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-14 15:54 [PATCH 00/10] workqueue: break affinity initiatively Lai Jiangshan
2020-12-14 15:54 ` [PATCH 01/10] workqueue: restore unbound_workers' cpumask correctly Lai Jiangshan
2020-12-14 15:54 ` [PATCH 02/10] workqueue: use cpu_possible_mask instead of cpu_active_mask to break affinity Lai Jiangshan
2020-12-14 17:25   ` Peter Zijlstra
2020-12-15  8:33     ` Lai Jiangshan
2020-12-15  8:40     ` Peter Zijlstra
2020-12-16 14:32   ` Tejun Heo
2020-12-14 15:54 ` [PATCH 03/10] workqueue: Manually break affinity on pool detachment Lai Jiangshan
2020-12-14 15:54 ` [PATCH 04/10] workqueue: don't set the worker's cpumask when kthread_bind_mask() Lai Jiangshan
2020-12-16 14:39   ` Tejun Heo
2020-12-14 15:54 ` [PATCH 05/10] workqueue: introduce wq_online_cpumask Lai Jiangshan
2020-12-14 15:54 ` [PATCH 06/10] workqueue: use wq_online_cpumask in restore_unbound_workers_cpumask() Lai Jiangshan
2020-12-14 15:54 ` [PATCH 07/10] workqueue: Manually break affinity on hotplug for unbound pool Lai Jiangshan
2020-12-16 14:50   ` Tejun Heo
2020-12-14 15:54 ` [PATCH 08/10] workqueue: reorganize workqueue_online_cpu() Lai Jiangshan
2020-12-14 15:54 ` [PATCH 09/10] workqueue: reorganize workqueue_offline_cpu() unbind_workers() Lai Jiangshan
2020-12-14 15:54 ` [PATCH 10/10] workqueue: Fix affinity of kworkers when attaching into pool Lai Jiangshan
2020-12-15 15:03   ` Valentin Schneider
2020-12-14 17:36 ` [PATCH 00/10] workqueue: break affinity initiatively Peter Zijlstra
2020-12-15  5:44   ` Lai Jiangshan [this message]
2020-12-15  7:50     ` Peter Zijlstra
2020-12-15  8:14       ` Lai Jiangshan
2020-12-15  8:49         ` Peter Zijlstra
2020-12-15  9:46           ` Lai Jiangshan
2020-12-16 14:30 ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJhGHyDYbU2=dQyiyx-AC+wu9=B7mTvsbU=QYPyV4xTE8PFPog@mail.gmail.com' \
    --to=jiangshanlai@gmail.com \
    --cc=cai@redhat.com \
    --cc=hdanton@sina.com \
    --cc=laijs@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tj@kernel.org \
    --cc=valentin.schneider@arm.com \
    --cc=vincent.donnefort@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.