diff mbox series

[3/3] ARM: exynos_defconfig: Enable Energy Model framework

Message ID 20200127215453.15144-4-lukasz.luba@arm.com (mailing list archive)
State Superseded
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>

Enable the Energy Model (EM) brings possibility to use Energy Aware
Scheduler (EAS). This compiles the EM but does not enable to run EAS in
default. The EAS only works with SchedUtil - a CPUFreq governor which
handles direct requests from the scheduler for the frequency change. Thus,
to make EAS working in default, the SchedUtil governor should be
configured as default CPUFreq governor. Although, the EAS might be enabled
in runtime, when the EM is present for CPUs, the SchedUtil is compiled and
then set as CPUFreq governor, i.e.:

echo schedutil > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
echo schedutil > /sys/devices/system/cpu/cpu4/cpufreq/scaling_governor

To check if EAS is ready to work, the read output from the command below
should show '1':
cat /proc/sys/kernel/sched_energy_aware

To disable EAS in runtime simply 'echo 0' to the file above.

Some test results, which stress the scheduler on Odroid-XU3:
hackbench -l 500 -s 4096
With mainline code and with this patch set.

The tests have been made with and without CONFIG_PROVE_LOCKING (PL)
(which is set to =y in default exynos_defconfig)

		|		this patch set			| mainline
		|-----------------------------------------------|---------------
		| performance	| SchedUtil	| SchedUtil	| performance
		| governor	| governor	| governor	| governor
		|		| w/o EAS	| w/ EAS	|
----------------|---------------|---------------|---------------|---------------
hackbench w/ PL | 12.7s		| 11.7s		| 12.0s		| 13.0s - 12.2s
hackbench w/o PL| 9.2s		| 8.1s		| 8.2s		| 9.2s - 8.4s

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, 1:16 p.m. UTC | #1
On Mon, 27 Jan 2020 at 22:55, <lukasz.luba@arm.com> wrote:
>
> From: Lukasz Luba <lukasz.luba@arm.com>
>
> Enable the Energy Model (EM) brings possibility to use Energy Aware
> Scheduler (EAS). This compiles the EM but does not enable to run EAS in
> default. The EAS only works with SchedUtil - a CPUFreq governor which
> handles direct requests from the scheduler for the frequency change. Thus,
> to make EAS working in default, the SchedUtil governor should be
> configured as default CPUFreq governor.

Full stop. That's enough of needed explanation of schedutil.

> Although, the EAS might be enabled
> in runtime, when the EM is present for CPUs, the SchedUtil is compiled and
> then set as CPUFreq governor, i.e.:
>
> echo schedutil > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
> echo schedutil > /sys/devices/system/cpu/cpu4/cpufreq/scaling_governor
>
> To check if EAS is ready to work, the read output from the command below
> should show '1':
> cat /proc/sys/kernel/sched_energy_aware
>
> To disable EAS in runtime simply 'echo 0' to the file above.

Not related to this commit. If you were implemeting here
schedutil/EAS, then it makes sense to post all this. However what's
the point to describe it in every defconfig change?

> Some test results, which stress the scheduler on Odroid-XU3:
> hackbench -l 500 -s 4096
> With mainline code and with this patch set.

Skip the last sentence - duplicated information.

>
> The tests have been made with and without CONFIG_PROVE_LOCKING (PL)
> (which is set to =y in default exynos_defconfig)
>
>                 |               this patch set                  | mainline

The commit will be applied on its own branch so the meaning of "this
patch set" will be lost. Maybe just "before/after"?

>                 |-----------------------------------------------|---------------
>                 | performance   | SchedUtil     | SchedUtil     | performance
>                 | governor      | governor      | governor      | governor
>                 |               | w/o EAS       | w/ EAS        |
> ----------------|---------------|---------------|---------------|---------------
> hackbench w/ PL | 12.7s         | 11.7s         | 12.0s         | 13.0s - 12.2s
> hackbench w/o PL| 9.2s          | 8.1s          | 8.2s          | 9.2s - 8.4s

Why does the performance different before and after this patch?

Mention - lower better (?). Space between number and unit... or better
mention [s] in column title.

And last but not least:
Why this patch is separate from 1/3? I don't get the need of splitting them.

Best regards,
Krzysztof
Bartlomiej Zolnierkiewicz Jan. 31, 2020, 1:30 p.m. UTC | #2
Hi,

On 1/27/20 10:54 PM, lukasz.luba@arm.com wrote:
> From: Lukasz Luba <lukasz.luba@arm.com>
> 
> Enable the Energy Model (EM) brings possibility to use Energy Aware
> Scheduler (EAS). This compiles the EM but does not enable to run EAS in
> default. The EAS only works with SchedUtil - a CPUFreq governor which
> handles direct requests from the scheduler for the frequency change. Thus,
> to make EAS working in default, the SchedUtil governor should be
> configured as default CPUFreq governor. Although, the EAS might be enabled
> in runtime, when the EM is present for CPUs, the SchedUtil is compiled and
> then set as CPUFreq governor, i.e.:
> 
> echo schedutil > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
> echo schedutil > /sys/devices/system/cpu/cpu4/cpufreq/scaling_governor
> 
> To check if EAS is ready to work, the read output from the command below
> should show '1':
> cat /proc/sys/kernel/sched_energy_aware
> 
> To disable EAS in runtime simply 'echo 0' to the file above.
> 
> Some test results, which stress the scheduler on Odroid-XU3:
> hackbench -l 500 -s 4096
> With mainline code and with this patch set.
> 
> The tests have been made with and without CONFIG_PROVE_LOCKING (PL)
> (which is set to =y in default exynos_defconfig)
> 
> 		|		this patch set			| mainline
> 		|-----------------------------------------------|---------------
> 		| performance	| SchedUtil	| SchedUtil	| performance
> 		| governor	| governor	| governor	| governor
> 		|		| w/o EAS	| w/ EAS	|
> ----------------|---------------|---------------|---------------|---------------
> hackbench w/ PL | 12.7s		| 11.7s		| 12.0s		| 13.0s - 12.2s
> hackbench w/o PL| 9.2s		| 8.1s		| 8.2s		| 9.2s - 8.4s

Would you happen to have measurements of how much power is
saved by running hackbench using "SchedUtil governor w/ EAS"
instead of "SchedUtil governor w/o EAS"?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> 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 1db857056992..c0f8ecabc607 100644
> --- a/arch/arm/configs/exynos_defconfig
> +++ b/arch/arm/configs/exynos_defconfig
> @@ -18,6 +18,7 @@ CONFIG_ZBOOT_ROM_BSS=0x0
>  CONFIG_ARM_APPENDED_DTB=y
>  CONFIG_ARM_ATAG_DTB_COMPAT=y
>  CONFIG_CMDLINE="root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc mem=256M"
> +CONFIG_ENERGY_MODEL=y
>  CONFIG_CPU_FREQ=y
>  CONFIG_CPU_FREQ_STAT=y
>  CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
Krzysztof Kozlowski Jan. 31, 2020, 1:31 p.m. UTC | #3
On Fri, 31 Jan 2020 at 14:30, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
>
> Hi,
>
> On 1/27/20 10:54 PM, lukasz.luba@arm.com wrote:
> > From: Lukasz Luba <lukasz.luba@arm.com>
> >
> > Enable the Energy Model (EM) brings possibility to use Energy Aware
> > Scheduler (EAS). This compiles the EM but does not enable to run EAS in
> > default. The EAS only works with SchedUtil - a CPUFreq governor which
> > handles direct requests from the scheduler for the frequency change. Thus,
> > to make EAS working in default, the SchedUtil governor should be
> > configured as default CPUFreq governor. Although, the EAS might be enabled
> > in runtime, when the EM is present for CPUs, the SchedUtil is compiled and
> > then set as CPUFreq governor, i.e.:
> >
> > echo schedutil > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
> > echo schedutil > /sys/devices/system/cpu/cpu4/cpufreq/scaling_governor
> >
> > To check if EAS is ready to work, the read output from the command below
> > should show '1':
> > cat /proc/sys/kernel/sched_energy_aware
> >
> > To disable EAS in runtime simply 'echo 0' to the file above.
> >
> > Some test results, which stress the scheduler on Odroid-XU3:
> > hackbench -l 500 -s 4096
> > With mainline code and with this patch set.
> >
> > The tests have been made with and without CONFIG_PROVE_LOCKING (PL)
> > (which is set to =y in default exynos_defconfig)
> >
> >               |               this patch set                  | mainline
> >               |-----------------------------------------------|---------------
> >               | performance   | SchedUtil     | SchedUtil     | performance
> >               | governor      | governor      | governor      | governor
> >               |               | w/o EAS       | w/ EAS        |
> > ----------------|---------------|---------------|---------------|---------------
> > hackbench w/ PL | 12.7s               | 11.7s         | 12.0s         | 13.0s - 12.2s
> > hackbench w/o PL| 9.2s                | 8.1s          | 8.2s          | 9.2s - 8.4s
>
> Would you happen to have measurements of how much power is
> saved by running hackbench using "SchedUtil governor w/ EAS"
> instead of "SchedUtil governor w/o EAS"?

That's a good point and quite important reason behind enabling (or not) EAS...

Best regards,
Krzysztof
Bartlomiej Zolnierkiewicz Jan. 31, 2020, 1:47 p.m. UTC | #4
On 1/31/20 2:31 PM, Krzysztof Kozlowski wrote:
> On Fri, 31 Jan 2020 at 14:30, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
>>
>>
>> Hi,
>>
>> On 1/27/20 10:54 PM, lukasz.luba@arm.com wrote:
>>> From: Lukasz Luba <lukasz.luba@arm.com>
>>>
>>> Enable the Energy Model (EM) brings possibility to use Energy Aware
>>> Scheduler (EAS). This compiles the EM but does not enable to run EAS in
>>> default. The EAS only works with SchedUtil - a CPUFreq governor which
>>> handles direct requests from the scheduler for the frequency change. Thus,
>>> to make EAS working in default, the SchedUtil governor should be
>>> configured as default CPUFreq governor. Although, the EAS might be enabled
>>> in runtime, when the EM is present for CPUs, the SchedUtil is compiled and
>>> then set as CPUFreq governor, i.e.:
>>>
>>> echo schedutil > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
>>> echo schedutil > /sys/devices/system/cpu/cpu4/cpufreq/scaling_governor
>>>
>>> To check if EAS is ready to work, the read output from the command below
>>> should show '1':
>>> cat /proc/sys/kernel/sched_energy_aware
>>>
>>> To disable EAS in runtime simply 'echo 0' to the file above.
>>>
>>> Some test results, which stress the scheduler on Odroid-XU3:
>>> hackbench -l 500 -s 4096
>>> With mainline code and with this patch set.
>>>
>>> The tests have been made with and without CONFIG_PROVE_LOCKING (PL)
>>> (which is set to =y in default exynos_defconfig)
>>>
>>>               |               this patch set                  | mainline
>>>               |-----------------------------------------------|---------------
>>>               | performance   | SchedUtil     | SchedUtil     | performance
>>>               | governor      | governor      | governor      | governor
>>>               |               | w/o EAS       | w/ EAS        |
>>> ----------------|---------------|---------------|---------------|---------------
>>> hackbench w/ PL | 12.7s               | 11.7s         | 12.0s         | 13.0s - 12.2s
>>> hackbench w/o PL| 9.2s                | 8.1s          | 8.2s          | 9.2s - 8.4s
>>
>> Would you happen to have measurements of how much power is
>> saved by running hackbench using "SchedUtil governor w/ EAS"
>> instead of "SchedUtil governor w/o EAS"?
> 
> That's a good point and quite important reason behind enabling (or not) EAS...

IIUC EAS is enabled by default if you use SchedUtil
governor and Energy Model is available on you platform.

[ SchedUtil governor is enabled in exynos_defconfig
  although not enabled by default currently. ]

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> Best regards,
> Krzysztof
Bartlomiej Zolnierkiewicz Jan. 31, 2020, 1:48 p.m. UTC | #5
On 1/31/20 2:47 PM, Bartlomiej Zolnierkiewicz wrote:
> 
> On 1/31/20 2:31 PM, Krzysztof Kozlowski wrote:
>> On Fri, 31 Jan 2020 at 14:30, Bartlomiej Zolnierkiewicz
>> <b.zolnierkie@samsung.com> wrote:
>>>
>>>
>>> Hi,
>>>
>>> On 1/27/20 10:54 PM, lukasz.luba@arm.com wrote:
>>>> From: Lukasz Luba <lukasz.luba@arm.com>
>>>>
>>>> Enable the Energy Model (EM) brings possibility to use Energy Aware
>>>> Scheduler (EAS). This compiles the EM but does not enable to run EAS in
>>>> default. The EAS only works with SchedUtil - a CPUFreq governor which
>>>> handles direct requests from the scheduler for the frequency change. Thus,
>>>> to make EAS working in default, the SchedUtil governor should be
>>>> configured as default CPUFreq governor. Although, the EAS might be enabled
>>>> in runtime, when the EM is present for CPUs, the SchedUtil is compiled and
>>>> then set as CPUFreq governor, i.e.:
>>>>
>>>> echo schedutil > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
>>>> echo schedutil > /sys/devices/system/cpu/cpu4/cpufreq/scaling_governor
>>>>
>>>> To check if EAS is ready to work, the read output from the command below
>>>> should show '1':
>>>> cat /proc/sys/kernel/sched_energy_aware
>>>>
>>>> To disable EAS in runtime simply 'echo 0' to the file above.
>>>>
>>>> Some test results, which stress the scheduler on Odroid-XU3:
>>>> hackbench -l 500 -s 4096
>>>> With mainline code and with this patch set.
>>>>
>>>> The tests have been made with and without CONFIG_PROVE_LOCKING (PL)
>>>> (which is set to =y in default exynos_defconfig)
>>>>
>>>>               |               this patch set                  | mainline
>>>>               |-----------------------------------------------|---------------
>>>>               | performance   | SchedUtil     | SchedUtil     | performance
>>>>               | governor      | governor      | governor      | governor
>>>>               |               | w/o EAS       | w/ EAS        |
>>>> ----------------|---------------|---------------|---------------|---------------
>>>> hackbench w/ PL | 12.7s               | 11.7s         | 12.0s         | 13.0s - 12.2s
>>>> hackbench w/o PL| 9.2s                | 8.1s          | 8.2s          | 9.2s - 8.4s
>>>
>>> Would you happen to have measurements of how much power is
>>> saved by running hackbench using "SchedUtil governor w/ EAS"
>>> instead of "SchedUtil governor w/o EAS"?
>>
>> That's a good point and quite important reason behind enabling (or not) EAS...
> 
> IIUC EAS is enabled by default if you use SchedUtil
> governor and Energy Model is available on you platform.
> 
> [ SchedUtil governor is enabled in exynos_defconfig
>   although not enabled by default currently. ]

s/enabled/used/

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Lukasz Luba Jan. 31, 2020, 5:30 p.m. UTC | #6
Hi Krzysztof,

On 1/31/20 1:16 PM, Krzysztof Kozlowski wrote:
> On Mon, 27 Jan 2020 at 22:55, <lukasz.luba@arm.com> wrote:
>>
>> From: Lukasz Luba <lukasz.luba@arm.com>
>>
>> Enable the Energy Model (EM) brings possibility to use Energy Aware
>> Scheduler (EAS). This compiles the EM but does not enable to run EAS in
>> default. The EAS only works with SchedUtil - a CPUFreq governor which
>> handles direct requests from the scheduler for the frequency change. Thus,
>> to make EAS working in default, the SchedUtil governor should be
>> configured as default CPUFreq governor.
> 
> Full stop. That's enough of needed explanation of schedutil.

OK

> 
>> Although, the EAS might be enabled
>> in runtime, when the EM is present for CPUs, the SchedUtil is compiled and
>> then set as CPUFreq governor, i.e.:
>>
>> echo schedutil > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
>> echo schedutil > /sys/devices/system/cpu/cpu4/cpufreq/scaling_governor
>>
>> To check if EAS is ready to work, the read output from the command below
>> should show '1':
>> cat /proc/sys/kernel/sched_energy_aware
>>
>> To disable EAS in runtime simply 'echo 0' to the file above.
> 
> Not related to this commit. If you were implemeting here
> schedutil/EAS, then it makes sense to post all this. However what's
> the point to describe it in every defconfig change?

I will drop it.

> 
>> Some test results, which stress the scheduler on Odroid-XU3:
>> hackbench -l 500 -s 4096
>> With mainline code and with this patch set.
> 
> Skip the last sentence - duplicated information.

OK

> 
>>
>> The tests have been made with and without CONFIG_PROVE_LOCKING (PL)
>> (which is set to =y in default exynos_defconfig)
>>
>>                  |               this patch set                  | mainline
> 
> The commit will be applied on its own branch so the meaning of "this
> patch set" will be lost. Maybe just "before/after"?

OK

> 
>>                  |-----------------------------------------------|---------------
>>                  | performance   | SchedUtil     | SchedUtil     | performance
>>                  | governor      | governor      | governor      | governor
>>                  |               | w/o EAS       | w/ EAS        |
>> ----------------|---------------|---------------|---------------|---------------
>> hackbench w/ PL | 12.7s         | 11.7s         | 12.0s         | 13.0s - 12.2s
>> hackbench w/o PL| 9.2s          | 8.1s          | 8.2s          | 9.2s - 8.4s
> 
> Why does the performance different before and after this patch?

Probably due to better locality and cache utilization. I can see that
there is ~700k context switches vs ~450k and ~160k migrations vs ~50k.
If you need to communicate two threads in different clusters, it will go
through CCI.

> 
> Mention - lower better (?). Space between number and unit... or better
> mention [s] in column title.

OK

> 
> And last but not least:
> Why this patch is separate from 1/3? I don't get the need of splitting them.

As mentioned in response to patch 1/3. The fist patch would create MC
domain, something different than Energy Model or EAS. The decisions in
the scheduler would be different.

I can merge 1/3 and 3/3 if you like, though.

Regards,
Lukasz

> 
> Best regards,
> Krzysztof
>
Lukasz Luba Jan. 31, 2020, 5:38 p.m. UTC | #7
Hi Bartek,

On 1/31/20 1:30 PM, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On 1/27/20 10:54 PM, lukasz.luba@arm.com wrote:
>> From: Lukasz Luba <lukasz.luba@arm.com>
>>
>> Enable the Energy Model (EM) brings possibility to use Energy Aware
>> Scheduler (EAS). This compiles the EM but does not enable to run EAS in
>> default. The EAS only works with SchedUtil - a CPUFreq governor which
>> handles direct requests from the scheduler for the frequency change. Thus,
>> to make EAS working in default, the SchedUtil governor should be
>> configured as default CPUFreq governor. Although, the EAS might be enabled
>> in runtime, when the EM is present for CPUs, the SchedUtil is compiled and
>> then set as CPUFreq governor, i.e.:
>>
>> echo schedutil > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
>> echo schedutil > /sys/devices/system/cpu/cpu4/cpufreq/scaling_governor
>>
>> To check if EAS is ready to work, the read output from the command below
>> should show '1':
>> cat /proc/sys/kernel/sched_energy_aware
>>
>> To disable EAS in runtime simply 'echo 0' to the file above.
>>
>> Some test results, which stress the scheduler on Odroid-XU3:
>> hackbench -l 500 -s 4096
>> With mainline code and with this patch set.
>>
>> The tests have been made with and without CONFIG_PROVE_LOCKING (PL)
>> (which is set to =y in default exynos_defconfig)
>>
>> 		|		this patch set			| mainline
>> 		|-----------------------------------------------|---------------
>> 		| performance	| SchedUtil	| SchedUtil	| performance
>> 		| governor	| governor	| governor	| governor
>> 		|		| w/o EAS	| w/ EAS	|
>> ----------------|---------------|---------------|---------------|---------------
>> hackbench w/ PL | 12.7s		| 11.7s		| 12.0s		| 13.0s - 12.2s
>> hackbench w/o PL| 9.2s		| 8.1s		| 8.2s		| 9.2s - 8.4s
> 
> Would you happen to have measurements of how much power is
> saved by running hackbench using "SchedUtil governor w/ EAS"
> instead of "SchedUtil governor w/o EAS"?

I need to check if this xu3 ina2xx can aggregate energy or
it's only drained-current-at-that-moment value.

Regards,
Lukasz

> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
>> 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 1db857056992..c0f8ecabc607 100644
>> --- a/arch/arm/configs/exynos_defconfig
>> +++ b/arch/arm/configs/exynos_defconfig
>> @@ -18,6 +18,7 @@ CONFIG_ZBOOT_ROM_BSS=0x0
>>   CONFIG_ARM_APPENDED_DTB=y
>>   CONFIG_ARM_ATAG_DTB_COMPAT=y
>>   CONFIG_CMDLINE="root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc mem=256M"
>> +CONFIG_ENERGY_MODEL=y
>>   CONFIG_CPU_FREQ=y
>>   CONFIG_CPU_FREQ_STAT=y
>>   CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
Krzysztof Kozlowski Jan. 31, 2020, 8:41 p.m. UTC | #8
On Fri, Jan 31, 2020 at 05:30:46PM +0000, Lukasz Luba wrote:
 
> > 
> > >                  |-----------------------------------------------|---------------
> > >                  | performance   | SchedUtil     | SchedUtil     | performance
> > >                  | governor      | governor      | governor      | governor
> > >                  |               | w/o EAS       | w/ EAS        |
> > > ----------------|---------------|---------------|---------------|---------------
> > > hackbench w/ PL | 12.7s         | 11.7s         | 12.0s         | 13.0s - 12.2s
> > > hackbench w/o PL| 9.2s          | 8.1s          | 8.2s          | 9.2s - 8.4s
> > 
> > Why does the performance different before and after this patch?
> 
> Probably due to better locality and cache utilization. I can see that
> there is ~700k context switches vs ~450k and ~160k migrations vs ~50k.
> If you need to communicate two threads in different clusters, it will go
> through CCI.

Mhmm... I was not specific - I mean, "performance governor". All this
you mentioned should not differ between performance governor before and
after. However once you have 12.7, then 13.0 - 12.2. Unless multi-core
scheduler affects it... but then these numbers here are not showing
only this change, but also the SCHED_MC effect.  In such case each of
commits should be coming with their own numbers.

> As mentioned in response to patch 1/3. The fist patch would create MC
> domain, something different than Energy Model or EAS. The decisions in
> the scheduler would be different.
> 
> I can merge 1/3 and 3/3 if you like, though.

I understand now that their independent. Still, they are part of one
goal to tune the scheduler for Exynos platform. Splitting these looks
too much, like enabling multiple drivers one after another.

However if you provide numbers for each of cases (before patches, multi
core scheduler, energy model with DTS), then I see benefit of splitting
it.  Each commit would have its own rationale.  I am not sure if it is
worth such investigation - that's just defconfig... distros might ignore
it anyway.

Best regards,
Krzysztof
Lukasz Luba Feb. 5, 2020, 12:49 p.m. UTC | #9
Hi Krzysztof,

On 1/31/20 8:41 PM, Krzysztof Kozlowski wrote:
> On Fri, Jan 31, 2020 at 05:30:46PM +0000, Lukasz Luba wrote:
>   
>>>
>>>>                   |-----------------------------------------------|---------------
>>>>                   | performance   | SchedUtil     | SchedUtil     | performance
>>>>                   | governor      | governor      | governor      | governor
>>>>                   |               | w/o EAS       | w/ EAS        |
>>>> ----------------|---------------|---------------|---------------|---------------
>>>> hackbench w/ PL | 12.7s         | 11.7s         | 12.0s         | 13.0s - 12.2s
>>>> hackbench w/o PL| 9.2s          | 8.1s          | 8.2s          | 9.2s - 8.4s
>>>
>>> Why does the performance different before and after this patch?
>>
>> Probably due to better locality and cache utilization. I can see that
>> there is ~700k context switches vs ~450k and ~160k migrations vs ~50k.
>> If you need to communicate two threads in different clusters, it will go
>> through CCI.
> 
> Mhmm... I was not specific - I mean, "performance governor". All this
> you mentioned should not differ between performance governor before and
> after. However once you have 12.7, then 13.0 - 12.2. Unless multi-core
> scheduler affects it... but then these numbers here are not showing
> only this change, but also the SCHED_MC effect.  In such case each of
> commits should be coming with their own numbers.

Agree, I should have not put 'this patch set' in the commit
msg. It should go into the cover letter and avoid this confusion.
You are right with ' Unless multi-core scheduler affects it...',
that's why when the SCHED_MC is missing, the decisions about task
placing might cause this variation and delay '13.0 - 12.2' seconds.

> 
>> As mentioned in response to patch 1/3. The fist patch would create MC
>> domain, something different than Energy Model or EAS. The decisions in
>> the scheduler would be different.
>>
>> I can merge 1/3 and 3/3 if you like, though.
> 
> I understand now that their independent. Still, they are part of one
> goal to tune the scheduler for Exynos platform. Splitting these looks
> too much, like enabling multiple drivers one after another.
> 
> However if you provide numbers for each of cases (before patches, multi
> core scheduler, energy model with DTS), then I see benefit of splitting
> it.  Each commit would have its own rationale.  I am not sure if it is
> worth such investigation - that's just defconfig... distros might ignore
> it anyway.

Good point, and I agree that it would require more investigation, for
which unfortunately I don't have currently spare cycles.

Should I merge patch 1/3 and 3/3 and send the v2 with a cover letter
which would have the test results?

Regards,
Lukasz
Krzysztof Kozlowski Feb. 6, 2020, 12:59 p.m. UTC | #10
On Wed, 5 Feb 2020 at 13:49, Lukasz Luba <lukasz.luba@arm.com> wrote:
> >> As mentioned in response to patch 1/3. The fist patch would create MC
> >> domain, something different than Energy Model or EAS. The decisions in
> >> the scheduler would be different.
> >>
> >> I can merge 1/3 and 3/3 if you like, though.
> >
> > I understand now that their independent. Still, they are part of one
> > goal to tune the scheduler for Exynos platform. Splitting these looks
> > too much, like enabling multiple drivers one after another.
> >
> > However if you provide numbers for each of cases (before patches, multi
> > core scheduler, energy model with DTS), then I see benefit of splitting
> > it.  Each commit would have its own rationale.  I am not sure if it is
> > worth such investigation - that's just defconfig... distros might ignore
> > it anyway.
>
> Good point, and I agree that it would require more investigation, for
> which unfortunately I don't have currently spare cycles.
>
> Should I merge patch 1/3 and 3/3 and send the v2 with a cover letter
> which would have the test results?

Yes, let's do this way.

Thanks for working on this!

Best regards,
Krzysztof
Lukasz Luba Feb. 6, 2020, 2:15 p.m. UTC | #11
On 2/6/20 12:59 PM, Krzysztof Kozlowski wrote:
> On Wed, 5 Feb 2020 at 13:49, Lukasz Luba <lukasz.luba@arm.com> wrote:
>>>> As mentioned in response to patch 1/3. The fist patch would create MC
>>>> domain, something different than Energy Model or EAS. The decisions in
>>>> the scheduler would be different.
>>>>
>>>> I can merge 1/3 and 3/3 if you like, though.
>>>
>>> I understand now that their independent. Still, they are part of one
>>> goal to tune the scheduler for Exynos platform. Splitting these looks
>>> too much, like enabling multiple drivers one after another.
>>>
>>> However if you provide numbers for each of cases (before patches, multi
>>> core scheduler, energy model with DTS), then I see benefit of splitting
>>> it.  Each commit would have its own rationale.  I am not sure if it is
>>> worth such investigation - that's just defconfig... distros might ignore
>>> it anyway.
>>
>> Good point, and I agree that it would require more investigation, for
>> which unfortunately I don't have currently spare cycles.
>>
>> Should I merge patch 1/3 and 3/3 and send the v2 with a cover letter
>> which would have the test results?
> 
> Yes, let's do this way.

Thank you, I will send the v2 then.

Regards,
Lukasz

> 
> Thanks for working on this!
> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
index 1db857056992..c0f8ecabc607 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -18,6 +18,7 @@  CONFIG_ZBOOT_ROM_BSS=0x0
 CONFIG_ARM_APPENDED_DTB=y
 CONFIG_ARM_ATAG_DTB_COMPAT=y
 CONFIG_CMDLINE="root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc mem=256M"
+CONFIG_ENERGY_MODEL=y
 CONFIG_CPU_FREQ=y
 CONFIG_CPU_FREQ_STAT=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y