* [PATCH] sched/topology: Don't set SD_BALANCE_WAKE on cpuset domain relax
@ 2019-10-14 16:44 Valentin Schneider
2019-10-15 11:34 ` Peter Zijlstra
2019-10-18 12:48 ` [tip: sched/core] " tip-bot2 for Valentin Schneider
0 siblings, 2 replies; 5+ messages in thread
From: Valentin Schneider @ 2019-10-14 16:44 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, peterz, vincent.guittot, juri.lelli, Dietmar.Eggemann,
morten.rasmussen, seto.hidetoshi, qperret
As pointed out in commit
182a85f8a119 ("sched: Disable wakeup balancing")
SD_BALANCE_WAKE is a tad too aggressive, and is usually left unset.
However, it turns out cpuset domain relaxation will unconditionally set it
on domains below the relaxation level. This made sense back when
SD_BALANCE_WAKE was set unconditionally, but it no longer is the case.
We can improve things slightly by noticing that set_domain_attribute() is
always called after sd_init(), so rather than setting flags we can rely on
whatever sd_init() is doing and only clear certain flags when above the
relaxation level.
While at it, slightly clean up the function and flip the relax level
check to be more human readable.
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
I was tempted to put a
Fixes: 182a85f8a119 ("sched: Disable wakeup balancing")
but the SD setup code back then was a mess of SD_INIT() macros which I'm
not familiar with. It *looks* like the sequence was roughly the same as it
is now (i.e. set up domain flags, *then* call set_domain_attribute()) but
I'm not completely sure.
---
kernel/sched/topology.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index b5667a273bf6..3623ffe85d18 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1201,17 +1201,13 @@ static void set_domain_attribute(struct sched_domain *sd,
if (!attr || attr->relax_domain_level < 0) {
if (default_relax_domain_level < 0)
return;
- else
- request = default_relax_domain_level;
+ request = default_relax_domain_level;
} else
request = attr->relax_domain_level;
- if (request < sd->level) {
+
+ if (sd->level > request) {
/* Turn off idle balance on this domain: */
sd->flags &= ~(SD_BALANCE_WAKE|SD_BALANCE_NEWIDLE);
- } else {
- /* Turn on idle balance on this domain: */
- sd->flags |= (SD_BALANCE_WAKE|SD_BALANCE_NEWIDLE);
}
}
--
2.22.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] sched/topology: Don't set SD_BALANCE_WAKE on cpuset domain relax
2019-10-14 16:44 [PATCH] sched/topology: Don't set SD_BALANCE_WAKE on cpuset domain relax Valentin Schneider
@ 2019-10-15 11:34 ` Peter Zijlstra
2019-10-15 11:37 ` Valentin Schneider
2019-10-18 12:48 ` [tip: sched/core] " tip-bot2 for Valentin Schneider
1 sibling, 1 reply; 5+ messages in thread
From: Peter Zijlstra @ 2019-10-15 11:34 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, mingo, vincent.guittot, juri.lelli,
Dietmar.Eggemann, morten.rasmussen, seto.hidetoshi, qperret
On Mon, Oct 14, 2019 at 05:44:08PM +0100, Valentin Schneider wrote:
> As pointed out in commit
>
> 182a85f8a119 ("sched: Disable wakeup balancing")
>
> SD_BALANCE_WAKE is a tad too aggressive, and is usually left unset.
>
> However, it turns out cpuset domain relaxation will unconditionally set it
> on domains below the relaxation level. This made sense back when
> SD_BALANCE_WAKE was set unconditionally, but it no longer is the case.
>
> We can improve things slightly by noticing that set_domain_attribute() is
> always called after sd_init(), so rather than setting flags we can rely on
> whatever sd_init() is doing and only clear certain flags when above the
> relaxation level.
>
> While at it, slightly clean up the function and flip the relax level
> check to be more human readable.
>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> ---
> I was tempted to put a
>
> Fixes: 182a85f8a119 ("sched: Disable wakeup balancing")
>
> but the SD setup code back then was a mess of SD_INIT() macros which I'm
> not familiar with. It *looks* like the sequence was roughly the same as it
> is now (i.e. set up domain flags, *then* call set_domain_attribute()) but
> I'm not completely sure.
This 'relax' thing is on my list of regrets. It is a terrible thing that
should never have existed.
Are you actually using it or did you just stumble upon it while looking
around there?
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] sched/topology: Don't set SD_BALANCE_WAKE on cpuset domain relax
2019-10-15 11:34 ` Peter Zijlstra
@ 2019-10-15 11:37 ` Valentin Schneider
2019-10-16 8:28 ` Peter Zijlstra
0 siblings, 1 reply; 5+ messages in thread
From: Valentin Schneider @ 2019-10-15 11:37 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, mingo, vincent.guittot, juri.lelli,
Dietmar.Eggemann, morten.rasmussen, seto.hidetoshi, qperret
On 15/10/2019 12:34, Peter Zijlstra wrote:
> This 'relax' thing is on my list of regrets. It is a terrible thing that
> should never have existed.
>
> Are you actually using it or did you just stumble upon it while looking
> around there?
>
Just staring around, I don't use cpuset stuff unless I really need to...
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sched/topology: Don't set SD_BALANCE_WAKE on cpuset domain relax
2019-10-15 11:37 ` Valentin Schneider
@ 2019-10-16 8:28 ` Peter Zijlstra
0 siblings, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2019-10-16 8:28 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, mingo, vincent.guittot, juri.lelli,
Dietmar.Eggemann, morten.rasmussen, seto.hidetoshi, qperret
On Tue, Oct 15, 2019 at 12:37:11PM +0100, Valentin Schneider wrote:
> On 15/10/2019 12:34, Peter Zijlstra wrote:
> > This 'relax' thing is on my list of regrets. It is a terrible thing that
> > should never have existed.
> >
> > Are you actually using it or did you just stumble upon it while looking
> > around there?
> >
>
> Just staring around, I don't use cpuset stuff unless I really need to...
Fair enough. Applied it.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip: sched/core] sched/topology: Don't set SD_BALANCE_WAKE on cpuset domain relax
2019-10-14 16:44 [PATCH] sched/topology: Don't set SD_BALANCE_WAKE on cpuset domain relax Valentin Schneider
2019-10-15 11:34 ` Peter Zijlstra
@ 2019-10-18 12:48 ` tip-bot2 for Valentin Schneider
1 sibling, 0 replies; 5+ messages in thread
From: tip-bot2 for Valentin Schneider @ 2019-10-18 12:48 UTC (permalink / raw)
To: linux-tip-commits
Cc: Valentin Schneider, Peter Zijlstra (Intel),
mingo, vincent.guittot, juri.lelli, seto.hidetoshi, qperret,
Dietmar.Eggemann, morten.rasmussen, Borislav Petkov,
linux-kernel
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 9ae7ab20b4835dbea0e5fc6a5c70171dc354a72e
Gitweb: https://git.kernel.org/tip/9ae7ab20b4835dbea0e5fc6a5c70171dc354a72e
Author: Valentin Schneider <valentin.schneider@arm.com>
AuthorDate: Mon, 14 Oct 2019 17:44:08 +01:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Thu, 17 Oct 2019 21:31:54 +02:00
sched/topology: Don't set SD_BALANCE_WAKE on cpuset domain relax
As pointed out in commit
182a85f8a119 ("sched: Disable wakeup balancing")
SD_BALANCE_WAKE is a tad too aggressive, and is usually left unset.
However, it turns out cpuset domain relaxation will unconditionally set it
on domains below the relaxation level. This made sense back when
SD_BALANCE_WAKE was set unconditionally, but it no longer is the case.
We can improve things slightly by noticing that set_domain_attribute() is
always called after sd_init(), so rather than setting flags we can rely on
whatever sd_init() is doing and only clear certain flags when above the
relaxation level.
While at it, slightly clean up the function and flip the relax level
check to be more human readable.
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: mingo@kernel.org
Cc: vincent.guittot@linaro.org
Cc: juri.lelli@redhat.com
Cc: seto.hidetoshi@jp.fujitsu.com
Cc: qperret@google.com
Cc: Dietmar.Eggemann@arm.com
Cc: morten.rasmussen@arm.com
Link: https://lkml.kernel.org/r/20191014164408.32596-1-valentin.schneider@arm.com
---
kernel/sched/topology.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index b5667a2..3623ffe 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1201,16 +1201,13 @@ static void set_domain_attribute(struct sched_domain *sd,
if (!attr || attr->relax_domain_level < 0) {
if (default_relax_domain_level < 0)
return;
- else
- request = default_relax_domain_level;
+ request = default_relax_domain_level;
} else
request = attr->relax_domain_level;
- if (request < sd->level) {
+
+ if (sd->level > request) {
/* Turn off idle balance on this domain: */
sd->flags &= ~(SD_BALANCE_WAKE|SD_BALANCE_NEWIDLE);
- } else {
- /* Turn on idle balance on this domain: */
- sd->flags |= (SD_BALANCE_WAKE|SD_BALANCE_NEWIDLE);
}
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-18 12:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14 16:44 [PATCH] sched/topology: Don't set SD_BALANCE_WAKE on cpuset domain relax Valentin Schneider
2019-10-15 11:34 ` Peter Zijlstra
2019-10-15 11:37 ` Valentin Schneider
2019-10-16 8:28 ` Peter Zijlstra
2019-10-18 12:48 ` [tip: sched/core] " tip-bot2 for Valentin Schneider
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.