* Re: [PATCH v3 1/2] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates
2021-08-23 11:16 ` [PATCH v3 1/2] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates Valentin Schneider
@ 2021-08-23 11:59 ` Peter Zijlstra
2021-08-23 12:57 ` Valentin Schneider
2021-08-24 9:08 ` Vincent Guittot
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Peter Zijlstra @ 2021-08-23 11:59 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, Vincent Guittot, Ingo Molnar, Dietmar Eggemann
On Mon, Aug 23, 2021 at 12:16:59PM +0100, Valentin Schneider wrote:
> Gate NOHZ blocked load
> update by the presence of NOHZ_STATS_KICK - currently all NOHZ balance
> kicks will have the NOHZ_STATS_KICK flag set, so no change in behaviour is
> expected.
> @@ -10572,7 +10572,8 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
> * setting the flag, we are sure to not clear the state and not
> * check the load of an idle cpu.
> */
> - WRITE_ONCE(nohz.has_blocked, 0);
> + if (flags & NOHZ_STATS_KICK)
> + WRITE_ONCE(nohz.has_blocked, 0);
>
> /*
> * Ensures that if we miss the CPU, we must see the has_blocked
> @@ -10594,13 +10595,15 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
> * balancing owner will pick it up.
> */
> if (need_resched()) {
> - has_blocked_load = true;
> + if (flags & NOHZ_STATS_KICK)
> + has_blocked_load = true;
> goto abort;
> }
>
> rq = cpu_rq(balance_cpu);
>
> - has_blocked_load |= update_nohz_stats(rq);
> + if (flags & NOHZ_STATS_KICK)
> + has_blocked_load |= update_nohz_stats(rq);
>
> /*
> * If time for next balance is due,
> @@ -10631,8 +10634,9 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
> if (likely(update_next_balance))
> nohz.next_balance = next_balance;
>
> - WRITE_ONCE(nohz.next_blocked,
> - now + msecs_to_jiffies(LOAD_AVG_PERIOD));
> + if (flags & NOHZ_STATS_KICK)
> + WRITE_ONCE(nohz.next_blocked,
> + now + msecs_to_jiffies(LOAD_AVG_PERIOD));
>
> abort:
> /* There is still blocked load, enable periodic update */
I'm a bit puzzled by this; that function has:
SCHED_WARN_ON((flags & NOHZ_KICK_MASK) == NOHZ_BALANCE_KICK);
Which:
- isn't updated
- implies STATS must be set when BALANCE
the latter gives rise to my confusion; why add that gate on STATS? It
just doesn't make sense to do a BALANCE and not update STATS.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/2] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates
2021-08-23 11:59 ` Peter Zijlstra
@ 2021-08-23 12:57 ` Valentin Schneider
2021-08-23 13:53 ` Dietmar Eggemann
0 siblings, 1 reply; 13+ messages in thread
From: Valentin Schneider @ 2021-08-23 12:57 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Vincent Guittot, Ingo Molnar, Dietmar Eggemann
On 23/08/21 13:59, Peter Zijlstra wrote:
> On Mon, Aug 23, 2021 at 12:16:59PM +0100, Valentin Schneider wrote:
>
>> Gate NOHZ blocked load
>> update by the presence of NOHZ_STATS_KICK - currently all NOHZ balance
>> kicks will have the NOHZ_STATS_KICK flag set, so no change in behaviour is
>> expected.
>
>> @@ -10572,7 +10572,8 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
>> * setting the flag, we are sure to not clear the state and not
>> * check the load of an idle cpu.
>> */
>> - WRITE_ONCE(nohz.has_blocked, 0);
>> + if (flags & NOHZ_STATS_KICK)
>> + WRITE_ONCE(nohz.has_blocked, 0);
>>
>> /*
>> * Ensures that if we miss the CPU, we must see the has_blocked
>> @@ -10594,13 +10595,15 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
>> * balancing owner will pick it up.
>> */
>> if (need_resched()) {
>> - has_blocked_load = true;
>> + if (flags & NOHZ_STATS_KICK)
>> + has_blocked_load = true;
>> goto abort;
>> }
>>
>> rq = cpu_rq(balance_cpu);
>>
>> - has_blocked_load |= update_nohz_stats(rq);
>> + if (flags & NOHZ_STATS_KICK)
>> + has_blocked_load |= update_nohz_stats(rq);
>>
>> /*
>> * If time for next balance is due,
>> @@ -10631,8 +10634,9 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
>> if (likely(update_next_balance))
>> nohz.next_balance = next_balance;
>>
>> - WRITE_ONCE(nohz.next_blocked,
>> - now + msecs_to_jiffies(LOAD_AVG_PERIOD));
>> + if (flags & NOHZ_STATS_KICK)
>> + WRITE_ONCE(nohz.next_blocked,
>> + now + msecs_to_jiffies(LOAD_AVG_PERIOD));
>>
>> abort:
>> /* There is still blocked load, enable periodic update */
>
> I'm a bit puzzled by this; that function has:
>
> SCHED_WARN_ON((flags & NOHZ_KICK_MASK) == NOHZ_BALANCE_KICK);
>
> Which:
>
> - isn't updated
> - implies STATS must be set when BALANCE
Yup
>
> the latter gives rise to my confusion; why add that gate on STATS? It
> just doesn't make sense to do a BALANCE and not update STATS.
AFAIA that warning was only there to catch BALANCE && !STATS, so I didn't
tweak it.
Now, you could still end up with
flags == NOHZ_NEXT_KICK
(e.g. nohz.next_balance is in the future, but a new CPU entered NOHZ-idle
and needs its own rq.next_balance collated into the nohz struct)
in which case you don't do any blocked load update, hence the
gate. In v1 I had that piggyback on NOHZ_STATS_KICK, but Vincent noted
that might not be the best given blocked load updates can be time
consuming - hence the separate flag.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/2] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates
2021-08-23 12:57 ` Valentin Schneider
@ 2021-08-23 13:53 ` Dietmar Eggemann
2021-08-24 8:11 ` Peter Zijlstra
0 siblings, 1 reply; 13+ messages in thread
From: Dietmar Eggemann @ 2021-08-23 13:53 UTC (permalink / raw)
To: Valentin Schneider, Peter Zijlstra
Cc: linux-kernel, Vincent Guittot, Ingo Molnar
On 23/08/2021 14:57, Valentin Schneider wrote:
> On 23/08/21 13:59, Peter Zijlstra wrote:
>> On Mon, Aug 23, 2021 at 12:16:59PM +0100, Valentin Schneider wrote:
>>
>>> Gate NOHZ blocked load
>>> update by the presence of NOHZ_STATS_KICK - currently all NOHZ balance
>>> kicks will have the NOHZ_STATS_KICK flag set, so no change in behaviour is
>>> expected.
>>
>>> @@ -10572,7 +10572,8 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
>>> * setting the flag, we are sure to not clear the state and not
>>> * check the load of an idle cpu.
>>> */
>>> - WRITE_ONCE(nohz.has_blocked, 0);
>>> + if (flags & NOHZ_STATS_KICK)
>>> + WRITE_ONCE(nohz.has_blocked, 0);
>>>
>>> /*
>>> * Ensures that if we miss the CPU, we must see the has_blocked
>>> @@ -10594,13 +10595,15 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
>>> * balancing owner will pick it up.
>>> */
>>> if (need_resched()) {
>>> - has_blocked_load = true;
>>> + if (flags & NOHZ_STATS_KICK)
>>> + has_blocked_load = true;
>>> goto abort;
>>> }
>>>
>>> rq = cpu_rq(balance_cpu);
>>>
>>> - has_blocked_load |= update_nohz_stats(rq);
>>> + if (flags & NOHZ_STATS_KICK)
>>> + has_blocked_load |= update_nohz_stats(rq);
>>>
>>> /*
>>> * If time for next balance is due,
>>> @@ -10631,8 +10634,9 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
>>> if (likely(update_next_balance))
>>> nohz.next_balance = next_balance;
>>>
>>> - WRITE_ONCE(nohz.next_blocked,
>>> - now + msecs_to_jiffies(LOAD_AVG_PERIOD));
>>> + if (flags & NOHZ_STATS_KICK)
>>> + WRITE_ONCE(nohz.next_blocked,
>>> + now + msecs_to_jiffies(LOAD_AVG_PERIOD));
>>>
>>> abort:
>>> /* There is still blocked load, enable periodic update */
>>
>> I'm a bit puzzled by this; that function has:
>>
>> SCHED_WARN_ON((flags & NOHZ_KICK_MASK) == NOHZ_BALANCE_KICK);
>>
>> Which:
>>
>> - isn't updated
>> - implies STATS must be set when BALANCE
>
> Yup
>
>>
>> the latter gives rise to my confusion; why add that gate on STATS? It
>> just doesn't make sense to do a BALANCE and not update STATS.
>
> AFAIA that warning was only there to catch BALANCE && !STATS, so I didn't
> tweak it.
>
> Now, you could still end up with
>
> flags == NOHZ_NEXT_KICK
>
> (e.g. nohz.next_balance is in the future, but a new CPU entered NOHZ-idle
> and needs its own rq.next_balance collated into the nohz struct)
>
> in which case you don't do any blocked load update, hence the
> gate. In v1 I had that piggyback on NOHZ_STATS_KICK, but Vincent noted
> that might not be the best given blocked load updates can be time
> consuming - hence the separate flag.
Maybe the confusion stems from the fact that the NOHZ_NEXT_KICK-set
changes are only introduced in 2/2?
@@ -10417,6 +10418,9 @@ static void nohz_balancer_kick(struct rq *rq)
unlock:
rcu_read_unlock();
out:
+ if (READ_ONCE(nohz.needs_update))
+ flags |= NOHZ_NEXT_KICK;
+
@@ -10513,12 +10517,13 @@ void nohz_balance_enter_idle(int cpu)
...
+ WRITE_ONCE(nohz.needs_update, 1);
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/2] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates
2021-08-23 13:53 ` Dietmar Eggemann
@ 2021-08-24 8:11 ` Peter Zijlstra
0 siblings, 0 replies; 13+ messages in thread
From: Peter Zijlstra @ 2021-08-24 8:11 UTC (permalink / raw)
To: Dietmar Eggemann
Cc: Valentin Schneider, linux-kernel, Vincent Guittot, Ingo Molnar
On Mon, Aug 23, 2021 at 03:53:16PM +0200, Dietmar Eggemann wrote:
> >> I'm a bit puzzled by this; that function has:
> >>
> >> SCHED_WARN_ON((flags & NOHZ_KICK_MASK) == NOHZ_BALANCE_KICK);
> >>
> >> Which:
> >>
> >> - isn't updated
> >> - implies STATS must be set when BALANCE
> >
> > Yup
> >
> >>
> >> the latter gives rise to my confusion; why add that gate on STATS? It
> >> just doesn't make sense to do a BALANCE and not update STATS.
> >
> > AFAIA that warning was only there to catch BALANCE && !STATS, so I didn't
> > tweak it.
> >
> > Now, you could still end up with
> >
> > flags == NOHZ_NEXT_KICK
> >
> > (e.g. nohz.next_balance is in the future, but a new CPU entered NOHZ-idle
> > and needs its own rq.next_balance collated into the nohz struct)
> >
> > in which case you don't do any blocked load update, hence the
> > gate. In v1 I had that piggyback on NOHZ_STATS_KICK, but Vincent noted
> > that might not be the best given blocked load updates can be time
> > consuming - hence the separate flag.
>
> Maybe the confusion stems from the fact that the NOHZ_NEXT_KICK-set
> changes are only introduced in 2/2?
>
> @@ -10417,6 +10418,9 @@ static void nohz_balancer_kick(struct rq *rq)
> unlock:
> rcu_read_unlock();
> out:
> + if (READ_ONCE(nohz.needs_update))
> + flags |= NOHZ_NEXT_KICK;
> +
The confusion was about how we'd ever get there and not have STATS set,
but i guess having it all nicely gated does make it saner.
Thanks!
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/2] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates
2021-08-23 11:16 ` [PATCH v3 1/2] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates Valentin Schneider
2021-08-23 11:59 ` Peter Zijlstra
@ 2021-08-24 9:08 ` Vincent Guittot
2021-09-09 11:18 ` [tip: sched/core] " tip-bot2 for Valentin Schneider
2021-10-05 14:12 ` tip-bot2 for Valentin Schneider
3 siblings, 0 replies; 13+ messages in thread
From: Vincent Guittot @ 2021-08-24 9:08 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Dietmar Eggemann
On Mon, 23 Aug 2021 at 13:17, Valentin Schneider
<valentin.schneider@arm.com> wrote:
>
> A following patch will trigger NOHZ idle balances as a means to update
> nohz.next_balance. Vincent noted that blocked load updates can have
> non-negligible overhead, which should be avoided if the intent is to only
> update nohz.next_balance.
>
> Add a new NOHZ balance kick flag, NOHZ_NEXT_KICK. Gate NOHZ blocked load
> update by the presence of NOHZ_STATS_KICK - currently all NOHZ balance
> kicks will have the NOHZ_STATS_KICK flag set, so no change in behaviour is
> expected.
>
> Suggested-by: Vincent Guittot <vincent.guittot@linaro.org>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
> kernel/sched/fair.c | 24 ++++++++++++++----------
> kernel/sched/sched.h | 8 +++++++-
> 2 files changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 6cd05f1d77ef..4a91f3027c92 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -10342,7 +10342,7 @@ static void nohz_balancer_kick(struct rq *rq)
> goto out;
>
> if (rq->nr_running >= 2) {
> - flags = NOHZ_KICK_MASK;
> + flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
> goto out;
> }
>
> @@ -10356,7 +10356,7 @@ static void nohz_balancer_kick(struct rq *rq)
> * on.
> */
> if (rq->cfs.h_nr_running >= 1 && check_cpu_capacity(rq, sd)) {
> - flags = NOHZ_KICK_MASK;
> + flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
> goto unlock;
> }
> }
> @@ -10370,7 +10370,7 @@ static void nohz_balancer_kick(struct rq *rq)
> */
> for_each_cpu_and(i, sched_domain_span(sd), nohz.idle_cpus_mask) {
> if (sched_asym_prefer(i, cpu)) {
> - flags = NOHZ_KICK_MASK;
> + flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
> goto unlock;
> }
> }
> @@ -10383,7 +10383,7 @@ static void nohz_balancer_kick(struct rq *rq)
> * to run the misfit task on.
> */
> if (check_misfit_status(rq, sd)) {
> - flags = NOHZ_KICK_MASK;
> + flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
> goto unlock;
> }
>
> @@ -10410,7 +10410,7 @@ static void nohz_balancer_kick(struct rq *rq)
> */
> nr_busy = atomic_read(&sds->nr_busy_cpus);
> if (nr_busy > 1) {
> - flags = NOHZ_KICK_MASK;
> + flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
> goto unlock;
> }
> }
> @@ -10572,7 +10572,8 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
> * setting the flag, we are sure to not clear the state and not
> * check the load of an idle cpu.
> */
> - WRITE_ONCE(nohz.has_blocked, 0);
> + if (flags & NOHZ_STATS_KICK)
> + WRITE_ONCE(nohz.has_blocked, 0);
>
> /*
> * Ensures that if we miss the CPU, we must see the has_blocked
> @@ -10594,13 +10595,15 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
> * balancing owner will pick it up.
> */
> if (need_resched()) {
> - has_blocked_load = true;
> + if (flags & NOHZ_STATS_KICK)
> + has_blocked_load = true;
> goto abort;
> }
>
> rq = cpu_rq(balance_cpu);
>
> - has_blocked_load |= update_nohz_stats(rq);
> + if (flags & NOHZ_STATS_KICK)
> + has_blocked_load |= update_nohz_stats(rq);
>
> /*
> * If time for next balance is due,
> @@ -10631,8 +10634,9 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
> if (likely(update_next_balance))
> nohz.next_balance = next_balance;
>
> - WRITE_ONCE(nohz.next_blocked,
> - now + msecs_to_jiffies(LOAD_AVG_PERIOD));
> + if (flags & NOHZ_STATS_KICK)
> + WRITE_ONCE(nohz.next_blocked,
> + now + msecs_to_jiffies(LOAD_AVG_PERIOD));
>
> abort:
> /* There is still blocked load, enable periodic update */
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index e7e2bba5b520..30b7bd2ef25d 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -2706,12 +2706,18 @@ extern void cfs_bandwidth_usage_dec(void);
> #define NOHZ_BALANCE_KICK_BIT 0
> #define NOHZ_STATS_KICK_BIT 1
> #define NOHZ_NEWILB_KICK_BIT 2
> +#define NOHZ_NEXT_KICK_BIT 3
>
> +/* Run rebalance_domains() */
> #define NOHZ_BALANCE_KICK BIT(NOHZ_BALANCE_KICK_BIT)
> +/* Update blocked load */
> #define NOHZ_STATS_KICK BIT(NOHZ_STATS_KICK_BIT)
> +/* Update blocked load when entering idle */
> #define NOHZ_NEWILB_KICK BIT(NOHZ_NEWILB_KICK_BIT)
> +/* Update nohz.next_balance */
> +#define NOHZ_NEXT_KICK BIT(NOHZ_NEXT_KICK_BIT)
>
> -#define NOHZ_KICK_MASK (NOHZ_BALANCE_KICK | NOHZ_STATS_KICK)
> +#define NOHZ_KICK_MASK (NOHZ_BALANCE_KICK | NOHZ_STATS_KICK | NOHZ_NEXT_KICK)
>
> #define nohz_flags(cpu) (&cpu_rq(cpu)->nohz_flags)
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread* [tip: sched/core] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates
2021-08-23 11:16 ` [PATCH v3 1/2] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates Valentin Schneider
2021-08-23 11:59 ` Peter Zijlstra
2021-08-24 9:08 ` Vincent Guittot
@ 2021-09-09 11:18 ` tip-bot2 for Valentin Schneider
2021-10-05 14:12 ` tip-bot2 for Valentin Schneider
3 siblings, 0 replies; 13+ messages in thread
From: tip-bot2 for Valentin Schneider @ 2021-09-09 11:18 UTC (permalink / raw)
To: linux-tip-commits
Cc: Vincent Guittot, Valentin Schneider, Peter Zijlstra (Intel), x86,
linux-kernel
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 013ce5ed58f799a2f035b732f904f6ebd8e8d881
Gitweb: https://git.kernel.org/tip/013ce5ed58f799a2f035b732f904f6ebd8e8d881
Author: Valentin Schneider <valentin.schneider@arm.com>
AuthorDate: Mon, 23 Aug 2021 12:16:59 +01:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Thu, 09 Sep 2021 11:27:29 +02:00
sched/fair: Add NOHZ balancer flag for nohz.next_balance updates
A following patch will trigger NOHZ idle balances as a means to update
nohz.next_balance. Vincent noted that blocked load updates can have
non-negligible overhead, which should be avoided if the intent is to only
update nohz.next_balance.
Add a new NOHZ balance kick flag, NOHZ_NEXT_KICK. Gate NOHZ blocked load
update by the presence of NOHZ_STATS_KICK - currently all NOHZ balance
kicks will have the NOHZ_STATS_KICK flag set, so no change in behaviour is
expected.
Suggested-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lkml.kernel.org/r/20210823111700.2842997-2-valentin.schneider@arm.com
---
kernel/sched/fair.c | 24 ++++++++++++++----------
kernel/sched/sched.h | 8 +++++++-
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 7b3e859..48ce754 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -10342,7 +10342,7 @@ static void nohz_balancer_kick(struct rq *rq)
goto out;
if (rq->nr_running >= 2) {
- flags = NOHZ_KICK_MASK;
+ flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto out;
}
@@ -10356,7 +10356,7 @@ static void nohz_balancer_kick(struct rq *rq)
* on.
*/
if (rq->cfs.h_nr_running >= 1 && check_cpu_capacity(rq, sd)) {
- flags = NOHZ_KICK_MASK;
+ flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto unlock;
}
}
@@ -10370,7 +10370,7 @@ static void nohz_balancer_kick(struct rq *rq)
*/
for_each_cpu_and(i, sched_domain_span(sd), nohz.idle_cpus_mask) {
if (sched_asym_prefer(i, cpu)) {
- flags = NOHZ_KICK_MASK;
+ flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto unlock;
}
}
@@ -10383,7 +10383,7 @@ static void nohz_balancer_kick(struct rq *rq)
* to run the misfit task on.
*/
if (check_misfit_status(rq, sd)) {
- flags = NOHZ_KICK_MASK;
+ flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto unlock;
}
@@ -10410,7 +10410,7 @@ static void nohz_balancer_kick(struct rq *rq)
*/
nr_busy = atomic_read(&sds->nr_busy_cpus);
if (nr_busy > 1) {
- flags = NOHZ_KICK_MASK;
+ flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto unlock;
}
}
@@ -10572,7 +10572,8 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
* setting the flag, we are sure to not clear the state and not
* check the load of an idle cpu.
*/
- WRITE_ONCE(nohz.has_blocked, 0);
+ if (flags & NOHZ_STATS_KICK)
+ WRITE_ONCE(nohz.has_blocked, 0);
/*
* Ensures that if we miss the CPU, we must see the has_blocked
@@ -10594,13 +10595,15 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
* balancing owner will pick it up.
*/
if (need_resched()) {
- has_blocked_load = true;
+ if (flags & NOHZ_STATS_KICK)
+ has_blocked_load = true;
goto abort;
}
rq = cpu_rq(balance_cpu);
- has_blocked_load |= update_nohz_stats(rq);
+ if (flags & NOHZ_STATS_KICK)
+ has_blocked_load |= update_nohz_stats(rq);
/*
* If time for next balance is due,
@@ -10631,8 +10634,9 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
if (likely(update_next_balance))
nohz.next_balance = next_balance;
- WRITE_ONCE(nohz.next_blocked,
- now + msecs_to_jiffies(LOAD_AVG_PERIOD));
+ if (flags & NOHZ_STATS_KICK)
+ WRITE_ONCE(nohz.next_blocked,
+ now + msecs_to_jiffies(LOAD_AVG_PERIOD));
abort:
/* There is still blocked load, enable periodic update */
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index e7e2bba..30b7bd2 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2706,12 +2706,18 @@ extern void cfs_bandwidth_usage_dec(void);
#define NOHZ_BALANCE_KICK_BIT 0
#define NOHZ_STATS_KICK_BIT 1
#define NOHZ_NEWILB_KICK_BIT 2
+#define NOHZ_NEXT_KICK_BIT 3
+/* Run rebalance_domains() */
#define NOHZ_BALANCE_KICK BIT(NOHZ_BALANCE_KICK_BIT)
+/* Update blocked load */
#define NOHZ_STATS_KICK BIT(NOHZ_STATS_KICK_BIT)
+/* Update blocked load when entering idle */
#define NOHZ_NEWILB_KICK BIT(NOHZ_NEWILB_KICK_BIT)
+/* Update nohz.next_balance */
+#define NOHZ_NEXT_KICK BIT(NOHZ_NEXT_KICK_BIT)
-#define NOHZ_KICK_MASK (NOHZ_BALANCE_KICK | NOHZ_STATS_KICK)
+#define NOHZ_KICK_MASK (NOHZ_BALANCE_KICK | NOHZ_STATS_KICK | NOHZ_NEXT_KICK)
#define nohz_flags(cpu) (&cpu_rq(cpu)->nohz_flags)
^ permalink raw reply related [flat|nested] 13+ messages in thread* [tip: sched/core] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates
2021-08-23 11:16 ` [PATCH v3 1/2] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates Valentin Schneider
` (2 preceding siblings ...)
2021-09-09 11:18 ` [tip: sched/core] " tip-bot2 for Valentin Schneider
@ 2021-10-05 14:12 ` tip-bot2 for Valentin Schneider
3 siblings, 0 replies; 13+ messages in thread
From: tip-bot2 for Valentin Schneider @ 2021-10-05 14:12 UTC (permalink / raw)
To: linux-tip-commits
Cc: Vincent Guittot, Valentin Schneider, Peter Zijlstra (Intel), x86,
linux-kernel
The following commit has been merged into the sched/core branch of tip:
Commit-ID: efd984c481abb516fab8bafb25bf41fd9397a43c
Gitweb: https://git.kernel.org/tip/efd984c481abb516fab8bafb25bf41fd9397a43c
Author: Valentin Schneider <valentin.schneider@arm.com>
AuthorDate: Mon, 23 Aug 2021 12:16:59 +01:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 Oct 2021 15:51:30 +02:00
sched/fair: Add NOHZ balancer flag for nohz.next_balance updates
A following patch will trigger NOHZ idle balances as a means to update
nohz.next_balance. Vincent noted that blocked load updates can have
non-negligible overhead, which should be avoided if the intent is to only
update nohz.next_balance.
Add a new NOHZ balance kick flag, NOHZ_NEXT_KICK. Gate NOHZ blocked load
update by the presence of NOHZ_STATS_KICK - currently all NOHZ balance
kicks will have the NOHZ_STATS_KICK flag set, so no change in behaviour is
expected.
Suggested-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lkml.kernel.org/r/20210823111700.2842997-2-valentin.schneider@arm.com
---
kernel/sched/fair.c | 24 ++++++++++++++----------
kernel/sched/sched.h | 8 +++++++-
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index f6a05d9..f4de7f5 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -10375,7 +10375,7 @@ static void nohz_balancer_kick(struct rq *rq)
goto out;
if (rq->nr_running >= 2) {
- flags = NOHZ_KICK_MASK;
+ flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto out;
}
@@ -10389,7 +10389,7 @@ static void nohz_balancer_kick(struct rq *rq)
* on.
*/
if (rq->cfs.h_nr_running >= 1 && check_cpu_capacity(rq, sd)) {
- flags = NOHZ_KICK_MASK;
+ flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto unlock;
}
}
@@ -10403,7 +10403,7 @@ static void nohz_balancer_kick(struct rq *rq)
*/
for_each_cpu_and(i, sched_domain_span(sd), nohz.idle_cpus_mask) {
if (sched_asym_prefer(i, cpu)) {
- flags = NOHZ_KICK_MASK;
+ flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto unlock;
}
}
@@ -10416,7 +10416,7 @@ static void nohz_balancer_kick(struct rq *rq)
* to run the misfit task on.
*/
if (check_misfit_status(rq, sd)) {
- flags = NOHZ_KICK_MASK;
+ flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto unlock;
}
@@ -10443,7 +10443,7 @@ static void nohz_balancer_kick(struct rq *rq)
*/
nr_busy = atomic_read(&sds->nr_busy_cpus);
if (nr_busy > 1) {
- flags = NOHZ_KICK_MASK;
+ flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto unlock;
}
}
@@ -10605,7 +10605,8 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
* setting the flag, we are sure to not clear the state and not
* check the load of an idle cpu.
*/
- WRITE_ONCE(nohz.has_blocked, 0);
+ if (flags & NOHZ_STATS_KICK)
+ WRITE_ONCE(nohz.has_blocked, 0);
/*
* Ensures that if we miss the CPU, we must see the has_blocked
@@ -10627,13 +10628,15 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
* balancing owner will pick it up.
*/
if (need_resched()) {
- has_blocked_load = true;
+ if (flags & NOHZ_STATS_KICK)
+ has_blocked_load = true;
goto abort;
}
rq = cpu_rq(balance_cpu);
- has_blocked_load |= update_nohz_stats(rq);
+ if (flags & NOHZ_STATS_KICK)
+ has_blocked_load |= update_nohz_stats(rq);
/*
* If time for next balance is due,
@@ -10664,8 +10667,9 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
if (likely(update_next_balance))
nohz.next_balance = next_balance;
- WRITE_ONCE(nohz.next_blocked,
- now + msecs_to_jiffies(LOAD_AVG_PERIOD));
+ if (flags & NOHZ_STATS_KICK)
+ WRITE_ONCE(nohz.next_blocked,
+ now + msecs_to_jiffies(LOAD_AVG_PERIOD));
abort:
/* There is still blocked load, enable periodic update */
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 3d3e579..1fec313 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2709,12 +2709,18 @@ extern void cfs_bandwidth_usage_dec(void);
#define NOHZ_BALANCE_KICK_BIT 0
#define NOHZ_STATS_KICK_BIT 1
#define NOHZ_NEWILB_KICK_BIT 2
+#define NOHZ_NEXT_KICK_BIT 3
+/* Run rebalance_domains() */
#define NOHZ_BALANCE_KICK BIT(NOHZ_BALANCE_KICK_BIT)
+/* Update blocked load */
#define NOHZ_STATS_KICK BIT(NOHZ_STATS_KICK_BIT)
+/* Update blocked load when entering idle */
#define NOHZ_NEWILB_KICK BIT(NOHZ_NEWILB_KICK_BIT)
+/* Update nohz.next_balance */
+#define NOHZ_NEXT_KICK BIT(NOHZ_NEXT_KICK_BIT)
-#define NOHZ_KICK_MASK (NOHZ_BALANCE_KICK | NOHZ_STATS_KICK)
+#define NOHZ_KICK_MASK (NOHZ_BALANCE_KICK | NOHZ_STATS_KICK | NOHZ_NEXT_KICK)
#define nohz_flags(cpu) (&cpu_rq(cpu)->nohz_flags)
^ permalink raw reply related [flat|nested] 13+ messages in thread