diff mbox series

[1/3] ARM: exynos_defconfig: Enable SCHED_MC

Message ID 20200127215453.15144-2-lukasz.luba@arm.com (mailing list archive)
State New, archived
Headers show
Series Enable Odroid-XU3/4 to use Energy Model and Energy Aware Scheduler | expand

Commit Message

Lukasz Luba Jan. 27, 2020, 9:54 p.m. UTC 
From: Lukasz Luba <lukasz.luba@arm.com>

Since the 'capacities-dmips-mhz' are present in the CPU nodes, make use of
this knowledge in smarter decisions during scheduling.

The values in 'capacities-dmips-mhz' are normilized, this means that i.e.
when CPU0's capacities-dmips-mhz=100 and CPU1's 'capacities-dmips-mhz'=50,
cpu0 is twice fast as CPU1, at the same frequency. The proper hirarchy
in sched_domain topology could exploit the SoC architecture advantages
like big.LITTLE. Enabling the SCHED_MC will create two levels in
sched_domain hierarchy, which might be observed in:

grep . /proc/sys/kernel/sched_domain/cpu*/domain*/{name,flags}
/proc/sys/kernel/sched_domain/cpu0/domain0/name:MC
/proc/sys/kernel/sched_domain/cpu0/domain1/name:DIE
...
/proc/sys/kernel/sched_domain/cpu0/domain0/flags:575
/proc/sys/kernel/sched_domain/cpu0/domain1/flags:4223

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 arch/arm/configs/exynos_defconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Krzysztof Kozlowski Jan. 31, 2020, 12:47 p.m. UTC | #1 
On Mon, 27 Jan 2020 at 22:55, <lukasz.luba@arm.com> wrote:
>
> From: Lukasz Luba <lukasz.luba@arm.com>
>
> Since the 'capacities-dmips-mhz' are present in the CPU nodes, make use of
> this knowledge in smarter decisions during scheduling.
>
> The values in 'capacities-dmips-mhz' are normilized, this means that i.e.
> when CPU0's capacities-dmips-mhz=100 and CPU1's 'capacities-dmips-mhz'=50,
> cpu0 is twice fast as CPU1, at the same frequency. The proper hirarchy
> in sched_domain topology could exploit the SoC architecture advantages
> like big.LITTLE.

I do not quite get how this is related to rationale behind changing defconfig...

> Enabling the SCHED_MC will create two levels in
> sched_domain hierarchy, which might be observed in:

This is looks more convincing... but still what is the need? To work with EAS?

> grep . /proc/sys/kernel/sched_domain/cpu*/domain*/{name,flags}
> /proc/sys/kernel/sched_domain/cpu0/domain0/name:MC
> /proc/sys/kernel/sched_domain/cpu0/domain1/name:DIE
> ...
> /proc/sys/kernel/sched_domain/cpu0/domain0/flags:575
> /proc/sys/kernel/sched_domain/cpu0/domain1/flags:4223

Not related to defconfig change and not visible after this commit.

Best regards,
Krzysztof

>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  arch/arm/configs/exynos_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
> index e7e4bb5ad8d5..1db857056992 100644
> --- a/arch/arm/configs/exynos_defconfig
> +++ b/arch/arm/configs/exynos_defconfig
> @@ -8,6 +8,7 @@ CONFIG_PERF_EVENTS=y
>  CONFIG_ARCH_EXYNOS=y
>  CONFIG_CPU_ICACHE_MISMATCH_WORKAROUND=y
>  CONFIG_SMP=y
> +CONFIG_SCHED_MC=y
>  CONFIG_BIG_LITTLE=y
>  CONFIG_NR_CPUS=8
>  CONFIG_HIGHMEM=y
> --
> 2.17.1
>
Lukasz Luba Jan. 31, 2020, 3:59 p.m. UTC | #2 
Hi Krzysztof,

Thank you for your review, please see my comments below.

On 1/31/20 12:47 PM, Krzysztof Kozlowski wrote:
> On Mon, 27 Jan 2020 at 22:55, <lukasz.luba@arm.com> wrote:
>>
>> From: Lukasz Luba <lukasz.luba@arm.com>
>>
>> Since the 'capacities-dmips-mhz' are present in the CPU nodes, make use of
>> this knowledge in smarter decisions during scheduling.
>>
>> The values in 'capacities-dmips-mhz' are normilized, this means that i.e.
>> when CPU0's capacities-dmips-mhz=100 and CPU1's 'capacities-dmips-mhz'=50,
>> cpu0 is twice fast as CPU1, at the same frequency. The proper hirarchy
>> in sched_domain topology could exploit the SoC architecture advantages
>> like big.LITTLE.
> 
> I do not quite get how this is related to rationale behind changing defconfig...

It is not strictly about EAS, it is useful in general for big.LITTLE
platform with clusters.

> 
>> Enabling the SCHED_MC will create two levels in
>> sched_domain hierarchy, which might be observed in:
> 
> This is looks more convincing... but still what is the need? To work with EAS?

It is not only for EAS, but in general for the scheduler (load balance,
task's wake-up path, etc). The scheduler algorithms iterate CPUs in the
sched groups. To make better decisions, the information about MC domain
is needed. More about the scheduler domains and i.e. load_balance()
you can find here:

https://www.kernel.org/doc/html/latest/scheduler/sched-domains.html


> 
>> grep . /proc/sys/kernel/sched_domain/cpu*/domain*/{name,flags}
>> /proc/sys/kernel/sched_domain/cpu0/domain0/name:MC
>> /proc/sys/kernel/sched_domain/cpu0/domain1/name:DIE
>> ...
>> /proc/sys/kernel/sched_domain/cpu0/domain0/flags:575
>> /proc/sys/kernel/sched_domain/cpu0/domain1/flags:4223
> 
> Not related to defconfig change and not visible after this commit.

Without this patch there is only one domain: 'domain0' -> 'DIE'
cat /proc/sys/kernel/sched_domain/cpu0/domain0/name
DIE

When you apply this patch you will get two: 'domain0, 'domain1'
grep . /proc/sys/kernel/sched_domain/cpu0/domain?/name 

/proc/sys/kernel/sched_domain/cpu0/domain0/name:MC
/proc/sys/kernel/sched_domain/cpu0/domain1/name:DIE

I can remove it this information, but it is the most important
to spot this difference out.

This is also the main reason I haven't merge the patch 1 + 3.

Regards,
Lukasz

> 
> Best regards,
> Krzysztof
> 
>>
>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>> ---
>>   arch/arm/configs/exynos_defconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
>> index e7e4bb5ad8d5..1db857056992 100644
>> --- a/arch/arm/configs/exynos_defconfig
>> +++ b/arch/arm/configs/exynos_defconfig
>> @@ -8,6 +8,7 @@ CONFIG_PERF_EVENTS=y
>>   CONFIG_ARCH_EXYNOS=y
>>   CONFIG_CPU_ICACHE_MISMATCH_WORKAROUND=y
>>   CONFIG_SMP=y
>> +CONFIG_SCHED_MC=y
>>   CONFIG_BIG_LITTLE=y
>>   CONFIG_NR_CPUS=8
>>   CONFIG_HIGHMEM=y
>> --
>> 2.17.1
>>
Krzysztof Kozlowski Jan. 31, 2020, 8:33 p.m. UTC | #3 
On Fri, Jan 31, 2020 at 03:59:30PM +0000, Lukasz Luba wrote:
> Hi Krzysztof,
> 
> Thank you for your review, please see my comments below.
> 
> On 1/31/20 12:47 PM, Krzysztof Kozlowski wrote:
> > On Mon, 27 Jan 2020 at 22:55, <lukasz.luba@arm.com> wrote:
> > > 
> > > From: Lukasz Luba <lukasz.luba@arm.com>
> > > 
> > > Since the 'capacities-dmips-mhz' are present in the CPU nodes, make use of
> > > this knowledge in smarter decisions during scheduling.
> > > 
> > > The values in 'capacities-dmips-mhz' are normilized, this means that i.e.
> > > when CPU0's capacities-dmips-mhz=100 and CPU1's 'capacities-dmips-mhz'=50,
> > > cpu0 is twice fast as CPU1, at the same frequency. The proper hirarchy
> > > in sched_domain topology could exploit the SoC architecture advantages
> > > like big.LITTLE.
> > 
> > I do not quite get how this is related to rationale behind changing defconfig...
> 
> It is not strictly about EAS, it is useful in general for big.LITTLE
> platform with clusters.
> 
> > 
> > > Enabling the SCHED_MC will create two levels in
> > > sched_domain hierarchy, which might be observed in:
> > 
> > This is looks more convincing... but still what is the need? To work with EAS?
> 
> It is not only for EAS, but in general for the scheduler (load balance,
> task's wake-up path, etc). The scheduler algorithms iterate CPUs in the
> sched groups. To make better decisions, the information about MC domain
> is needed. More about the scheduler domains and i.e. load_balance()
> you can find here:
> 
> https://www.kernel.org/doc/html/latest/scheduler/sched-domains.html

Ahhh, I see, it's independent of later patches. Somehow I had impression
it is a prerequisite...

> 
> > 
> > > grep . /proc/sys/kernel/sched_domain/cpu*/domain*/{name,flags}
> > > /proc/sys/kernel/sched_domain/cpu0/domain0/name:MC
> > > /proc/sys/kernel/sched_domain/cpu0/domain1/name:DIE
> > > ...
> > > /proc/sys/kernel/sched_domain/cpu0/domain0/flags:575
> > > /proc/sys/kernel/sched_domain/cpu0/domain1/flags:4223
> > 
> > Not related to defconfig change and not visible after this commit.
> 
> Without this patch there is only one domain: 'domain0' -> 'DIE'
> cat /proc/sys/kernel/sched_domain/cpu0/domain0/name
> DIE
> 
> When you apply this patch you will get two: 'domain0, 'domain1'
> grep . /proc/sys/kernel/sched_domain/cpu0/domain?/name
> 
> /proc/sys/kernel/sched_domain/cpu0/domain0/name:MC
> /proc/sys/kernel/sched_domain/cpu0/domain1/name:DIE
> 
> I can remove it this information, but it is the most important
> to spot this difference out.
> 
> This is also the main reason I haven't merge the patch 1 + 3.

Indeed. I thought that these will pop up at the end of the patchset, my
bad.

I do not see big benefits of adding these outputs as proofs of working
SCHED_MC because they are kind of obvious. It is not a measurement but
report of current system state. However they don't harm neither, so I am
fine with it.

However please us in commit msg also the name of turned on option, next
or instead of SCHED_MC.  The options might be sometimes cryptic or too
vague and the name actually easily expresses what you want enable.
diff mbox series

Patch

diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
index e7e4bb5ad8d5..1db857056992 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -8,6 +8,7 @@  CONFIG_PERF_EVENTS=y
 CONFIG_ARCH_EXYNOS=y
 CONFIG_CPU_ICACHE_MISMATCH_WORKAROUND=y
 CONFIG_SMP=y
+CONFIG_SCHED_MC=y
 CONFIG_BIG_LITTLE=y
 CONFIG_NR_CPUS=8
 CONFIG_HIGHMEM=y