From: Peter Zijlstra <peterz@infradead.org>
To: Valentin Schneider <valentin.schneider@arm.com>
Cc: tglx@linutronix.de, mingo@kernel.org, bigeasy@linutronix.de,
swood@redhat.com, juri.lelli@redhat.com,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
bristot@redhat.com, vincent.donnefort@arm.com,
qais.yousef@arm.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback
Date: Thu, 15 Apr 2021 10:59:13 +0200 [thread overview]
Message-ID: <YHgAYef83VQhKdC2@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <YHU/a9HvGLYpOLKZ@hirez.programming.kicks-ass.net>
On Tue, Apr 13, 2021 at 08:51:23AM +0200, Peter Zijlstra wrote:
> > I'm afraid I don't follow; we're replacing a read of rq->balance_push with
> > cpu_dying(), and those are still written on the same side of the
> > synchronize_rcu(). What am I missing?
>
> Yeah, I'm not sure anymnore either; I tried to work out why I'd done
> that but upon closer examination everything fell flat.
>
> Let me try again today :-)
Can't make sense of what I did.. I've removed that hunk. Patch now looks
like this.
---
Subject: sched: Use cpu_dying() to fix balance_push vs hotplug-rollback
From: Peter Zijlstra <peterz@infradead.org>
Date: Thu Jan 21 16:09:32 CET 2021
Use the new cpu_dying() state to simplify and fix the balance_push()
vs CPU hotplug rollback state.
Specifically, we currently rely on notifiers sched_cpu_dying() /
sched_cpu_activate() to terminate balance_push, however if the
cpu_down() fails when we're past sched_cpu_deactivate(), it should
terminate balance_push at that point and not wait until we hit
sched_cpu_activate().
Similarly, when cpu_up() fails and we're going back down, balance_push
should be active, where it currently is not.
So instead, make sure balance_push is enabled between
sched_cpu_deactivate() and sched_cpu_activate() (eg. when
!cpu_active()), and gate it's utility with cpu_dying().
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/sched/core.c | 26 +++++++++++++++-----------
kernel/sched/sched.h | 1 -
2 files changed, 15 insertions(+), 12 deletions(-)
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1811,7 +1811,7 @@ static inline bool is_cpu_allowed(struct
return cpu_online(cpu);
/* Regular kernel threads don't get to stay during offline. */
- if (cpu_rq(cpu)->balance_push)
+ if (cpu_dying(cpu))
return false;
/* But are allowed during online. */
@@ -7639,6 +7639,9 @@ static DEFINE_PER_CPU(struct cpu_stop_wo
/*
* Ensure we only run per-cpu kthreads once the CPU goes !active.
+ *
+ * This is active/set between sched_cpu_deactivate() / sched_cpu_activate().
+ * But only effective when the hotplug motion is down.
*/
static void balance_push(struct rq *rq)
{
@@ -7646,12 +7649,19 @@ static void balance_push(struct rq *rq)
lockdep_assert_held(&rq->lock);
SCHED_WARN_ON(rq->cpu != smp_processor_id());
+
/*
* Ensure the thing is persistent until balance_push_set(.on = false);
*/
rq->balance_callback = &balance_push_callback;
/*
+ * Only active while going offline.
+ */
+ if (!cpu_dying(rq->cpu))
+ return;
+
+ /*
* Both the cpu-hotplug and stop task are in this case and are
* required to complete the hotplug process.
*
@@ -7704,7 +7714,6 @@ static void balance_push_set(int cpu, bo
struct rq_flags rf;
rq_lock_irqsave(rq, &rf);
- rq->balance_push = on;
if (on) {
WARN_ON_ONCE(rq->balance_callback);
rq->balance_callback = &balance_push_callback;
@@ -7829,8 +7838,8 @@ int sched_cpu_activate(unsigned int cpu)
struct rq_flags rf;
/*
- * Make sure that when the hotplug state machine does a roll-back
- * we clear balance_push. Ideally that would happen earlier...
+ * Clear the balance_push callback and prepare to schedule
+ * regular tasks.
*/
balance_push_set(cpu, false);
@@ -8015,12 +8024,6 @@ int sched_cpu_dying(unsigned int cpu)
}
rq_unlock_irqrestore(rq, &rf);
- /*
- * Now that the CPU is offline, make sure we're welcome
- * to new tasks once we come back up.
- */
- balance_push_set(cpu, false);
-
calc_load_migrate(rq);
update_max_interval();
hrtick_clear(rq);
@@ -8205,7 +8208,7 @@ void __init sched_init(void)
rq->sd = NULL;
rq->rd = NULL;
rq->cpu_capacity = rq->cpu_capacity_orig = SCHED_CAPACITY_SCALE;
- rq->balance_callback = NULL;
+ rq->balance_callback = &balance_push_callback;
rq->active_balance = 0;
rq->next_balance = jiffies;
rq->push_cpu = 0;
@@ -8252,6 +8255,7 @@ void __init sched_init(void)
#ifdef CONFIG_SMP
idle_thread_set_boot_cpu();
+ balance_push_set(smp_processor_id(), false);
#endif
init_sched_fair_class();
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -983,7 +983,6 @@ struct rq {
unsigned long cpu_capacity_orig;
struct callback_head *balance_callback;
- unsigned char balance_push;
unsigned char nohz_idle_balance;
unsigned char idle_balance;
next prev parent reply other threads:[~2021-04-15 8:59 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-10 14:52 [PATCH 0/3] sched: Fix remaining balance_push vs hotplug hole Peter Zijlstra
2021-03-10 14:52 ` [PATCH 1/3] cpumask: Make cpu_{online,possible,present,active}() inline Peter Zijlstra
2021-04-16 15:53 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2021-03-10 14:53 ` [PATCH 2/3] cpumask: Introduce DYING mask Peter Zijlstra
2021-03-21 19:30 ` Qais Yousef
2021-03-22 15:07 ` Steven Rostedt
2021-04-12 10:55 ` Peter Zijlstra
2021-04-12 11:16 ` Qais Yousef
2021-04-16 15:53 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2021-03-10 14:53 ` [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback Peter Zijlstra
2021-03-11 15:13 ` Valentin Schneider
2021-03-11 16:42 ` Peter Zijlstra
2021-04-12 12:03 ` Peter Zijlstra
2021-04-12 17:22 ` Valentin Schneider
2021-04-13 6:51 ` Peter Zijlstra
2021-04-15 8:59 ` Peter Zijlstra [this message]
2021-04-15 14:32 ` Valentin Schneider
2021-04-15 15:29 ` Peter Zijlstra
2021-04-15 15:34 ` Valentin Schneider
2021-04-19 10:56 ` Vincent Donnefort
2021-04-20 9:46 ` Vincent Donnefort
2021-04-20 14:20 ` Peter Zijlstra
2021-04-20 14:39 ` Peter Zijlstra
2021-04-20 14:58 ` Peter Zijlstra
2021-04-20 16:53 ` Vincent Donnefort
2021-04-20 18:07 ` Peter Zijlstra
2021-04-21 9:32 ` Valentin Schneider
2021-04-22 7:36 ` [tip: sched/core] cpumask/hotplug: Fix cpu_dying() state tracking tip-bot2 for Peter Zijlstra
2021-04-16 15:53 ` [tip: sched/core] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback tip-bot2 for 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=YHgAYef83VQhKdC2@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=bigeasy@linutronix.de \
--cc=bristot@redhat.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@kernel.org \
--cc=qais.yousef@arm.com \
--cc=rostedt@goodmis.org \
--cc=swood@redhat.com \
--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.