From: "Paul E. McKenney" <paulmck@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Lai Jiangshan <jiangshanlai@gmail.com>,
linux-kernel@vger.kernel.org,
Valentin Schneider <valentin.schneider@arm.com>,
Qian Cai <cai@redhat.com>,
Vincent Donnefort <vincent.donnefort@arm.com>,
Dexuan Cui <decui@microsoft.com>,
Lai Jiangshan <laijs@linux.alibaba.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH -tip V3 0/8] workqueue: break affinity initiatively
Date: Mon, 11 Jan 2021 10:09:07 -0800 [thread overview]
Message-ID: <20210111180907.GE2743@paulmck-ThinkPad-P72> (raw)
In-Reply-To: <X/yH9+MGa1JCNZ8x@hirez.programming.kicks-ass.net>
On Mon, Jan 11, 2021 at 06:16:39PM +0100, Peter Zijlstra wrote:
>
> While thinking more about this, I'm thinking a big part of the problem
> is that we're not dinstinguishing between geniuine per-cpu kthreads and
> kthreads that just happen to be per-cpu.
>
> Geniuine per-cpu kthreads are kthread_bind() and have PF_NO_SETAFFINITY,
> but sadly a lot of non-per-cpu kthreads, that might happen to still be
> per-cpu also have that -- again workqueue does that even to it's unbound
> workers :-(
>
> Now, anything created by smpboot, is created through
> kthread_create_on_cpu() and that additionally sets to_kthread(p)->flags
> KTHREAD_IS_PER_CPU.
>
> And I'm thinking that might be sufficient, if we modify
> is_per_cpu_kthread() to check that, then we only match smpboot threads
> (which include the hotplug and stopper threads, but notably not the idle
> thread)
>
> Sadly it appears like io_uring() uses kthread_create_on_cpu() without
> then having any hotplug crud on, so that needs additinoal frobbing.
>
> Also, init_task is PF_KTHREAD but doesn't have a struct kthread on.. and
> I suppose bound workqueues don't go through this either.
>
> Let me rummage around a bit...
>
> This seems to not insta-explode... opinions?
It passes quick tests on -rcu both with and without the rcutorture fixes,
which is encouraging. I will start a more vigorous test in about an hour.
Thanx, Paul
> ---
> include/linux/kthread.h | 3 +++
> kernel/kthread.c | 25 ++++++++++++++++++++++++-
> kernel/sched/core.c | 2 +-
> kernel/sched/sched.h | 4 ++--
> kernel/smpboot.c | 1 +
> kernel/workqueue.c | 12 +++++++++---
> 6 files changed, 40 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/kthread.h b/include/linux/kthread.h
> index 65b81e0c494d..fdd5a52e35d8 100644
> --- a/include/linux/kthread.h
> +++ b/include/linux/kthread.h
> @@ -33,6 +33,9 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
> unsigned int cpu,
> const char *namefmt);
>
> +void kthread_set_per_cpu(struct task_struct *k, bool set);
> +bool kthread_is_per_cpu(struct task_struct *k);
> +
> /**
> * kthread_run - create and wake a thread.
> * @threadfn: the function to run until signal_pending(current).
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index a5eceecd4513..7f081530e459 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -493,11 +493,34 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
> return p;
> kthread_bind(p, cpu);
> /* CPU hotplug need to bind once again when unparking the thread. */
> - set_bit(KTHREAD_IS_PER_CPU, &to_kthread(p)->flags);
> to_kthread(p)->cpu = cpu;
> return p;
> }
>
> +void kthread_set_per_cpu(struct task_struct *k, bool set)
> +{
> + struct kthread *kthread = to_kthread(k);
> + if (!kthread)
> + return;
> +
> + if (set) {
> + WARN_ON_ONCE(!(k->flags & PF_NO_SETAFFINITY));
> + WARN_ON_ONCE(k->nr_cpus_allowed != 1);
> + set_bit(KTHREAD_IS_PER_CPU, &kthread->flags);
> + } else {
> + clear_bit(KTHREAD_IS_PER_CPU, &kthread->flags);
> + }
> +}
> +
> +bool kthread_is_per_cpu(struct task_struct *k)
> +{
> + struct kthread *kthread = to_kthread(k);
> + if (!kthread)
> + return false;
> +
> + return test_bit(KTHREAD_IS_PER_CPU, &kthread->flags);
> +}
> +
> /**
> * kthread_unpark - unpark a thread created by kthread_create().
> * @k: thread created by kthread_create().
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 15d2562118d1..e71f9e44789e 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -7277,7 +7277,7 @@ static void balance_push(struct rq *rq)
> * Both the cpu-hotplug and stop task are in this case and are
> * required to complete the hotplug process.
> */
> - if (is_per_cpu_kthread(push_task) || is_migration_disabled(push_task)) {
> + if (rq->idle == push_task || is_per_cpu_kthread(push_task) || is_migration_disabled(push_task)) {
> /*
> * If this is the idle task on the outgoing CPU try to wake
> * up the hotplug control thread which might wait for the
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 12ada79d40f3..3679f63e0aa2 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -2697,10 +2697,10 @@ static inline bool is_per_cpu_kthread(struct task_struct *p)
> if (!(p->flags & PF_KTHREAD))
> return false;
>
> - if (p->nr_cpus_allowed != 1)
> + if (!(p->flags & PF_NO_SETAFFINITY))
> return false;
>
> - return true;
> + return kthread_is_per_cpu(p);
> }
> #endif
>
> diff --git a/kernel/smpboot.c b/kernel/smpboot.c
> index 2efe1e206167..b0abe575a524 100644
> --- a/kernel/smpboot.c
> +++ b/kernel/smpboot.c
> @@ -188,6 +188,7 @@ __smpboot_create_thread(struct smp_hotplug_thread *ht, unsigned int cpu)
> kfree(td);
> return PTR_ERR(tsk);
> }
> + kthread_set_per_cpu(tsk, true);
> /*
> * Park the thread so that it could start right on the CPU
> * when it is available.
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 9880b6c0e272..824276e4fb2e 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1861,6 +1861,8 @@ static void worker_attach_to_pool(struct worker *worker,
> */
> if (pool->flags & POOL_DISASSOCIATED)
> worker->flags |= WORKER_UNBOUND;
> + else
> + kthread_set_per_cpu(worker->task, true);
>
> list_add_tail(&worker->node, &pool->workers);
> worker->pool = pool;
> @@ -4919,8 +4921,10 @@ static void unbind_workers(int cpu)
>
> raw_spin_unlock_irq(&pool->lock);
>
> - for_each_pool_worker(worker, pool)
> - WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task, cpu_active_mask) < 0);
> + for_each_pool_worker(worker, pool) {
> + kthread_set_per_cpu(worker->task, false);
> + WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task, cpu_possible_mask) < 0);
> + }
>
> mutex_unlock(&wq_pool_attach_mutex);
>
> @@ -4972,9 +4976,11 @@ static void rebind_workers(struct worker_pool *pool)
> * of all workers first and then clear UNBOUND. As we're called
> * from CPU_ONLINE, the following shouldn't fail.
> */
> - for_each_pool_worker(worker, pool)
> + for_each_pool_worker(worker, pool) {
> WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task,
> pool->attrs->cpumask) < 0);
> + kthread_set_per_cpu(worker->task, true);
> + }
>
> raw_spin_lock_irq(&pool->lock);
>
next prev parent reply other threads:[~2021-01-11 18:10 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-26 2:51 [PATCH -tip V3 0/8] workqueue: break affinity initiatively Lai Jiangshan
2020-12-26 2:51 ` [PATCH -tip V3 1/8] workqueue: use cpu_possible_mask instead of cpu_active_mask to break affinity Lai Jiangshan
2020-12-26 2:51 ` [PATCH -tip V3 2/8] workqueue: Manually break affinity on pool detachment Lai Jiangshan
2020-12-26 2:51 ` [PATCH -tip V3 3/8] workqueue: introduce wq_online_cpumask Lai Jiangshan
2021-01-04 13:56 ` Peter Zijlstra
2021-01-05 2:41 ` Lai Jiangshan
2021-01-05 2:53 ` Lai Jiangshan
2021-01-05 8:23 ` Lai Jiangshan
2021-01-05 13:17 ` Peter Zijlstra
2021-01-05 14:37 ` Lai Jiangshan
2021-01-05 14:40 ` Lai Jiangshan
2021-01-05 16:24 ` Peter Zijlstra
2020-12-26 2:51 ` [PATCH -tip V3 4/8] workqueue: use wq_online_cpumask in restore_unbound_workers_cpumask() Lai Jiangshan
2020-12-26 2:51 ` [PATCH -tip V3 5/8] workqueue: Manually break affinity on hotplug for unbound pool Lai Jiangshan
[not found] ` <20201226101631.5448-1-hdanton@sina.com>
2020-12-27 14:04 ` Lai Jiangshan
2020-12-26 2:51 ` [PATCH -tip V3 6/8] workqueue: reorganize workqueue_online_cpu() Lai Jiangshan
2020-12-26 2:51 ` [PATCH -tip V3 7/8] workqueue: reorganize workqueue_offline_cpu() unbind_workers() Lai Jiangshan
2020-12-26 2:51 ` [PATCH -tip V3 8/8] workqueue: Fix affinity of kworkers when attaching into pool Lai Jiangshan
[not found] ` <20201229100639.2086-1-hdanton@sina.com>
2020-12-29 10:13 ` Lai Jiangshan
2021-01-08 11:46 ` [PATCH -tip V3 0/8] workqueue: break affinity initiatively Peter Zijlstra
2021-01-11 10:07 ` Thomas Gleixner
2021-01-11 11:01 ` Peter Zijlstra
2021-01-11 15:00 ` Paul E. McKenney
2021-01-11 17:16 ` Peter Zijlstra
2021-01-11 18:09 ` Paul E. McKenney [this message]
2021-01-11 21:50 ` Paul E. McKenney
2021-01-12 17:14 ` Paul E. McKenney
2021-01-12 23:53 ` Paul E. McKenney
2021-01-15 9:11 ` Peter Zijlstra
2021-01-15 13:04 ` Peter Zijlstra
2021-01-16 6:00 ` Lai Jiangshan
2021-01-11 19:21 ` Valentin Schneider
2021-01-11 20:23 ` Peter Zijlstra
2021-01-11 22:47 ` Valentin Schneider
2021-01-12 4:33 ` Lai Jiangshan
2021-01-12 14:53 ` Peter Zijlstra
2021-01-12 15:38 ` Lai Jiangshan
2021-01-13 11:10 ` Peter Zijlstra
2021-01-13 12:00 ` Lai Jiangshan
2021-01-13 12:57 ` Lai Jiangshan
2021-01-12 17:52 ` Valentin Schneider
2021-01-12 14:57 ` Jens Axboe
2021-01-12 15:51 ` Peter Zijlstra
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=20210111180907.GE2743@paulmck-ThinkPad-P72 \
--to=paulmck@kernel.org \
--cc=cai@redhat.com \
--cc=decui@microsoft.com \
--cc=jiangshanlai@gmail.com \
--cc=laijs@linux.alibaba.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=valentin.schneider@arm.com \
--cc=vincent.donnefort@arm.com \
--cc=vincent.guittot@linaro.org \
/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.