On 19 November 2012 14:36, Morten Rasmussen <morten.rasmus...@arm.com> wrote:
> On 19/11/12 12:23, Vincent Guittot wrote:
>>
>> On 19 November 2012 13:08, Morten Rasmussen <morten.rasmus...@arm.com>
>> wrote:
>>>
>>> Hi Vincent,
>>>
>>>
>>> On 19/11/12 09:20, Vincent Guittot wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> On 16 November 2012 19:32, Liviu Dudau <liviu.du...@arm.com> wrote:
>>>>>
>>>>>
>>>>> From: Morten Rasmussen <morten.rasmus...@arm.com>
>>>>>
>>>>> Re-enable SD_SHARE_POWERLINE to reflect the power domains of TC2.
>>>>> ---
>>>>> arch/arm/kernel/topology.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
>>>>> index 317dac6..4d34e0e 100644
>>>>> --- a/arch/arm/kernel/topology.c
>>>>> +++ b/arch/arm/kernel/topology.c
>>>>> @@ -228,7 +228,7 @@ struct cputopo_arm cpu_topology[NR_CPUS];
>>>>>
>>>>> int arch_sd_share_power_line(void)
>>>>> {
>>>>> - return 0*SD_SHARE_POWERLINE;
>>>>> + return 1*SD_SHARE_POWERLINE;
>>>>
>>>>
>>>>
>>>> I'm not sure to catch your goal. With this modification, the power
>>>> line (or power domain) is shared at all level which should disable the
>>>> packing mechanism. But in a previous patch you fix the update packing
>>>> loop so I assume that you want to use it. Which kind of configuration
>>>> you would like to have among the proposal below ?
>>>>
>>>> cpu : CPU0 | CPU1 | CPU2 | CPU3 | CPU4
>>>> buddy conf 1 : CPU2 | CPU0 | CPU2 | CPU2 | CPU2
>>>> buddy conf 2 : CPU2 | CPU2 | CPU2 | CPU2 | CPU2
>>>> buddy conf 3 : -1 | -1 | -1 | -1 | -1
>>>>
>>>> When we look at the git://git.linaro.org/arm/big.LITTLE/mp.git
>>>> big-LITTLE-MP-master-v12, we can see that you have defined a custom
>>>> sched_domain which hasn't been updated with SD_SHARE_POWERLINE flag so
>>>> the flag is cleared at CPU level. Based on this, I would say that you
>>>> want buddy conf 2 ? but I would say that buddy conf 1 should give
>>>> better result. Have you tried both ?
>>>>
>>>
>>> My goal with this fix is to set up the SD_SHARE_POWERLINE flags as they
>>> really are on TC2. It could have been done more elegantly. Since the HMP
>>> patches overrides the sched_domain flags at CPU level the
>>> SD_SHARE_POWERLINE
>>> is not being set by arch_sd_share_power_line(). With this fix we will get
>>> SD_SHARE_POWERLINE at MC level and no SD_SHARE_POWERLINE at CPU level,
>>> which
>>> I believe is the correct set up for TC2.
>>>
>>> For the buddy configuration the goal is to get configuration 1 in your
>>> list
>>> above. You should get that when using the other patch to fix the buddy
>>> selection algorithm.
>>> I'm not sure if conf 1 or 2 is best. I think it depends on the
>>> power/performance trade-off of the specific platform. conf 1 may lead to
>>> CPU1->CPU0->CPU2 migrations which may be undesirable. If your cpus are
>>> very
>>> leaky it might make sense to not do packing at all inside a high
>>> performance
>>> cluster and always do packing directly on a another low power cluster
>>> like
>>> conf 2. I think this needs further investigation.
>>>
>>> I have only tested with conf 1 on TC2.
>>
>>
>> Hi Morten,
>>
>> Conf1 is the default configuration for ARM platform because
>> SD_SHARE_POWERLINE is cleared at all levels for this architecture.
>>
>> Conf2 should be used if you can't powergate the core independently but
>> several tests have demonstrated that even if you can't powergate each
>> core independently, it worth packing small task on few CPUs in a core
>> so it's worth using conf1 on TC2 as well.
>>
>> Based on your explanation, we should use the original configuration of
>> SD_SHARE_POWERLINE (cleared at all level for ARM platform)
>
>
> I agree that the result is the same, but I don't like disabling
> SD_SHARE_POWERLINE for all level when the cpus in each cluster actually are
> in the same power domain as it is the case on TC2. The name SHARE_POWERLINE
> implies a clear relation to the actual hardware design, thus setting the
> flags differently than the actual hardware design is misleading in my
> opinion. If the buddy selection algorithm doesn't select appropriate buddies
> when flags are set to reflect the actual hardware design I would suggest
> changing the buddy selection algorithm instead of changing the sched_domain
> flags.
>
> If it is chosen to not have a direct relation between the flags and the
> hardware design, I think that the flag should be renamed so it doesn't give
> the wrong impression.
There is a direct link between the powergating and the SHARE_POWERLINE
and if you want that the buddy selection strictly reflects your HW
configuration, you must use conf2 and not conf1.
Now, beside the packing small task patch and the TC2 configuration, it
has been proven that packing small tasks on an ARM platform (dual
cortex-A9) which can only powergate the cluster, improves the power
consumption of some low cpu load use cases like the MP3 playback (we
had used cpu hotplug at that time). This assumption has been proven
only for ARM platform and that's why the SHARE_POWERLINE is cleared at
all level for ARM platform even if you can only do some WFI inside the
cluster.
But i agree that if you want to strictly reflect the TC2 HW config,
you should use the conf2 and set the SHARE_POWERLINE at MC level for
the TC2 HMP configuration
Regards,
Vincent
>
> Morten
>
>
>>
>> Regards
>> Vincent
>>
>>
>>>
>>> Regards,
>>> Morten
>>>
>>>
>>>> Regards,
>>>> Vincent
>>>>
>>>>> }
>>>>>
>>>>> const struct cpumask *cpu_coregroup_mask(int cpu)
>>>>> --
>>>>> 1.7.9.5
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> linaro-dev mailing list
>>>>> linaro-dev@lists.linaro.org
>>>>> http://lists.linaro.org/mailman/listinfo/linaro-dev
>>>>
>>>>
>>>>
>>>
>>>
>>
>
>
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
