linux-kernel - Re: [PATCH 4/4] base/drivers/topology: Default dmpis-mhz if they are not set in DT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b507cf16-aa78-303e-7459-ad380266317e@linaro.org>
Date:   Thu, 22 Nov 2018 11:29:44 +0100
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     rjw@...ysocki.net, vincent.guittot@...aro.org,
        linux-kernel@...r.kernel.org,
        Chris Redpath <chris.redpath@...aro.org>,
        Quentin Perret <quentin.perret@...aro.org>,
        Amit Kucheria <amit.kucheria@...aro.org>,
        Nicolas Dechesne <nicolas.dechesne@...aro.org>,
        Niklas Cassel <niklas.cassel@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>
Subject: Re: [PATCH 4/4] base/drivers/topology: Default dmpis-mhz if they are
 not set in DT

On 22/11/2018 05:29, Viresh Kumar wrote:
> On 21-11-18, 23:12, Daniel Lezcano wrote:
>> On 30/10/2018 09:58, Viresh Kumar wrote:
>>> s/dmpis/dmips/ in $subject
>>>
>>> On 29-10-18, 17:23, Daniel Lezcano wrote:
>>>> In the case of assymetric SoC with the same micro-architecture, we
>>>
>>>                  asymmetric ?
>>>
>>>> have a group of CPUs with smaller OPPs than the other group. One
>>>> example is the 96boards dragonboard 820c. There is no dmips/MHz
>>>> difference between both groups, so no need to specify the values in
>>>> the DT. Unfortunately, without these defined, there is no scaling
>>>> capacity comutation triggered, so we need to write
>>>
>>>            computation
>>>
>>>> 'capacity-dmips-mhz' for each CPU with the same value in order to
>>>> force the scaled capacity computation.
>>>>
>>>> Fix this by setting a default capacity to SCHED_CAPACITY_SCALE, if no
>>>> 'capacity-dmips-mhz' is defined in the DT.
>>>>
>>>> This was tested on db820c:
>>>>  - specified values in the DT (correct results)
>>>>  - partial values defined in the DT (error + fallback to defaults)
>>>>  - no specified values in the DT (correct results)
>>>>
>>>> correct results are:
>>>>   cat /sys/devices/system/cpu/cpu*/cpu_capacity
>>>>    758
>>>>    758
>>>>   1024
>>>>   1024
>>>>
>>>>   ... respectively for CPU0, CPU1, CPU2 and CPU3.
>>>>
>>>> That reflects the capacity for the max frequencies 1593600 and 2150400.
>>>>
>>>> Cc: Chris Redpath <chris.redpath@...aro.org>
>>>> Cc: Quentin Perret <quentin.perret@...aro.org>
>>>> Cc: Viresh Kumar <viresh.kumar@...aro.org>
>>>> Cc: Amit Kucheria <amit.kucheria@...aro.org>
>>>> Cc: Nicolas Dechesne <nicolas.dechesne@...aro.org>
>>>> Cc: Niklas Cassel <niklas.cassel@...aro.org>
>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
>>>> ---
>>>>  drivers/base/arch_topology.c | 27 ++++++++++++++++++++++++++-
>>>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>>>> index 7311641..7d594a6 100644
>>>> --- a/drivers/base/arch_topology.c
>>>> +++ b/drivers/base/arch_topology.c
>>>> @@ -205,6 +205,21 @@ static struct notifier_block init_cpu_capacity_notifier = {
>>>>  	.notifier_call = init_cpu_capacity_callback,
>>>>  };
>>>>  
>>>> +static int topology_set_default_capacity(void)
>>>> +{
>>>> +	int cpu;
>>>> +
>>>> +	raw_capacity = kzalloc(num_possible_cpus() * sizeof(*raw_capacity),
>>>> +			       GFP_KERNEL);
>>>> +	if (!raw_capacity)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	for_each_possible_cpu(cpu)
>>>> +		raw_capacity[cpu] = SCHED_CAPACITY_SCALE;
>>>
>>> This isn't actually required as the value of raw_capacity isn't used
>>> anymore after this point in code. Rather it is forcefully updated in
>>> init_cpu_capacity_callback():
>>>
>>> raw_capacity[cpu] = topology_get_cpu_scale(NULL, cpu) *
>>>                     policy->cpuinfo.max_freq / 1000UL;
>>>
>>> Maybe it is better to allocate raw_capacity once at boot and use
>>> another global variable as flag (raw_capacity is used as a flag right
>>> now at many places).
>>
>> Can we keep the proposed change as is to simply fix the default value?
>>
>> I want to do a separate change with a raw_capacity rewrite and remove
>> the workqueue freeing it.
> 
> Sure. But you still don't need to update raw_capacity[cpu] above as I pointed
> out earlier. You can drop those lines at least.

Oh ... actually raw_capacity is not needed at all!



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog