2010-08-12 17:25:34

by Heiko Carstens

[permalink] [raw]
Subject: [PATCH/RFC 2/5] [PATCH] sched: pass sched_domain_level to sched_power_savings_store

From: Heiko Carstens <[email protected]>

Pass the corresponding sched domain level to sched_power_savings_store instead
of a yes/no flag which indicates if the level is SMT or MC.
This is needed to easily extend the function so it can be used for a third
level.

Signed-off-by: Heiko Carstens <[email protected]>
---

kernel/sched.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)

diff -urpN linux-2.6/kernel/sched.c linux-2.6-patched/kernel/sched.c
--- linux-2.6/kernel/sched.c 2010-08-11 13:47:22.000000000 +0200
+++ linux-2.6-patched/kernel/sched.c 2010-08-11 13:47:22.000000000 +0200
@@ -7380,7 +7380,8 @@ static void arch_reinit_sched_domains(vo
put_online_cpus();
}

-static ssize_t sched_power_savings_store(const char *buf, size_t count, int smt)
+static ssize_t sched_power_savings_store(const char *buf, size_t count,
+ enum sched_domain_level sd_level)
{
unsigned int level = 0;

@@ -7397,10 +7398,16 @@ static ssize_t sched_power_savings_store
if (level >= MAX_POWERSAVINGS_BALANCE_LEVELS)
return -EINVAL;

- if (smt)
+ switch (sd_level) {
+ case SD_LV_SIBLING:
sched_smt_power_savings = level;
- else
+ break;
+ case SD_LV_MC:
sched_mc_power_savings = level;
+ break;
+ default:
+ break;
+ }

arch_reinit_sched_domains();

@@ -7418,7 +7425,7 @@ static ssize_t sched_mc_power_savings_st
struct sysdev_class_attribute *attr,
const char *buf, size_t count)
{
- return sched_power_savings_store(buf, count, 0);
+ return sched_power_savings_store(buf, count, SD_LV_MC);
}
static SYSDEV_CLASS_ATTR(sched_mc_power_savings, 0644,
sched_mc_power_savings_show,
@@ -7436,7 +7443,7 @@ static ssize_t sched_smt_power_savings_s
struct sysdev_class_attribute *attr,
const char *buf, size_t count)
{
- return sched_power_savings_store(buf, count, 1);
+ return sched_power_savings_store(buf, count, SD_LV_SIBLING);
}
static SYSDEV_CLASS_ATTR(sched_smt_power_savings, 0644,
sched_smt_power_savings_show,


2010-08-13 21:14:26

by Suresh Siddha

[permalink] [raw]
Subject: Re: [PATCH/RFC 2/5] [PATCH] sched: pass sched_domain_level to sched_power_savings_store

On Thu, 2010-08-12 at 10:25 -0700, Heiko Carstens wrote:
> From: Heiko Carstens <[email protected]>
>
> Pass the corresponding sched domain level to sched_power_savings_store instead
> of a yes/no flag which indicates if the level is SMT or MC.
> This is needed to easily extend the function so it can be used for a third
> level.
>
> Signed-off-by: Heiko Carstens <[email protected]>

Acked-by: Suresh Siddha <[email protected]>

> ---
>
> kernel/sched.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff -urpN linux-2.6/kernel/sched.c linux-2.6-patched/kernel/sched.c
> --- linux-2.6/kernel/sched.c 2010-08-11 13:47:22.000000000 +0200
> +++ linux-2.6-patched/kernel/sched.c 2010-08-11 13:47:22.000000000 +0200
> @@ -7380,7 +7380,8 @@ static void arch_reinit_sched_domains(vo
> put_online_cpus();
> }
>
> -static ssize_t sched_power_savings_store(const char *buf, size_t count, int smt)
> +static ssize_t sched_power_savings_store(const char *buf, size_t count,
> + enum sched_domain_level sd_level)
> {
> unsigned int level = 0;
>
> @@ -7397,10 +7398,16 @@ static ssize_t sched_power_savings_store
> if (level >= MAX_POWERSAVINGS_BALANCE_LEVELS)
> return -EINVAL;
>
> - if (smt)
> + switch (sd_level) {
> + case SD_LV_SIBLING:
> sched_smt_power_savings = level;
> - else
> + break;
> + case SD_LV_MC:
> sched_mc_power_savings = level;
> + break;
> + default:
> + break;
> + }
>
> arch_reinit_sched_domains();
>
> @@ -7418,7 +7425,7 @@ static ssize_t sched_mc_power_savings_st
> struct sysdev_class_attribute *attr,
> const char *buf, size_t count)
> {
> - return sched_power_savings_store(buf, count, 0);
> + return sched_power_savings_store(buf, count, SD_LV_MC);
> }
> static SYSDEV_CLASS_ATTR(sched_mc_power_savings, 0644,
> sched_mc_power_savings_show,
> @@ -7436,7 +7443,7 @@ static ssize_t sched_smt_power_savings_s
> struct sysdev_class_attribute *attr,
> const char *buf, size_t count)
> {
> - return sched_power_savings_store(buf, count, 1);
> + return sched_power_savings_store(buf, count, SD_LV_SIBLING);
> }
> static SYSDEV_CLASS_ATTR(sched_smt_power_savings, 0644,
> sched_smt_power_savings_show,
>

2010-08-16 08:30:08

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH/RFC 2/5] [PATCH] sched: pass sched_domain_level to sched_power_savings_store

On Thu, 2010-08-12 at 19:25 +0200, Heiko Carstens wrote:
> Pass the corresponding sched domain level to sched_power_savings_store instead
> of a yes/no flag which indicates if the level is SMT or MC.
> This is needed to easily extend the function so it can be used for a third
> level.

Ah, so the plan is to reduce the number of knobs, not create more.

Sysadmins really aren't interested in having a powersavings knob per
topology level.

Subject: Re: [PATCH/RFC 2/5] [PATCH] sched: pass sched_domain_level to sched_power_savings_store

On Fri, Aug 13, 2010 at 05:13:40PM -0400, Suresh Siddha wrote:
> On Thu, 2010-08-12 at 10:25 -0700, Heiko Carstens wrote:
> > From: Heiko Carstens <[email protected]>
> >
> > Pass the corresponding sched domain level to sched_power_savings_store instead
> > of a yes/no flag which indicates if the level is SMT or MC.
> > This is needed to easily extend the function so it can be used for a third
> > level.
> >
> > Signed-off-by: Heiko Carstens <[email protected]>
>
> Acked-by: Suresh Siddha <[email protected]>

Acked-by: Andreas Herrmann <[email protected]>

Subject: Re: [PATCH/RFC 2/5] [PATCH] sched: pass sched_domain_level to sched_power_savings_store

On Mon, Aug 16, 2010 at 04:29:57AM -0400, Peter Zijlstra wrote:
> On Thu, 2010-08-12 at 19:25 +0200, Heiko Carstens wrote:
> > Pass the corresponding sched domain level to sched_power_savings_store instead
> > of a yes/no flag which indicates if the level is SMT or MC.
> > This is needed to easily extend the function so it can be used for a third
> > level.
>
> Ah, so the plan is to reduce the number of knobs, not create more.

Don't think so.

> Sysadmins really aren't interested in having a powersavings knob per
> topology level.

It just allows to use the same store functions for three instead of
two different knobs.


Andreas


2010-08-19 12:35:27

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH/RFC 2/5] [PATCH] sched: pass sched_domain_level to sched_power_savings_store

On Thu, 2010-08-19 at 13:41 +0200, Andreas Herrmann wrote:
> It just allows to use the same store functions for three instead of
> two different knobs.

Creating more knobs for powersave scheduling is a fail.

We already have 2^3 powersave scheduling states, it should be decreased
to 2 (namely on/off), not increased to 3^3.

Subject: Re: [PATCH/RFC 2/5] [PATCH] sched: pass sched_domain_level to sched_power_savings_store

On Thu, Aug 19, 2010 at 08:35:11AM -0400, Peter Zijlstra wrote:
> On Thu, 2010-08-19 at 13:41 +0200, Andreas Herrmann wrote:
> > It just allows to use the same store functions for three instead of
> > two different knobs.
>
> Creating more knobs for powersave scheduling is a fail.
>
> We already have 2^3 powersave scheduling states, it should be decreased
> to 2 (namely on/off), not increased to 3^3.

I think it should be possible to select a domain level at which power
saving scheduling should happen (this would result in 3 states in the
z196 case).


Andreas

--
Operating | Advanced Micro Devices GmbH
System | Einsteinring 24, 85609 Dornach b. M?nchen, Germany
Research | Gesch?ftsf?hrer: Alberto Bozzo, Andrew Bowd
Center | Sitz: Dornach, Gemeinde Aschheim, Landkreis M?nchen
(OSRC) | Registergericht M?nchen, HRB Nr. 43632