lkml.org 
[lkml]   [2018]   [Oct]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 3/4] base/drivers/topology: Move instructions in the error path
From
Date
On 30/10/2018 07:12, Viresh Kumar wrote:
> On Mon, Oct 29, 2018 at 9:56 PM Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>>
>> When the function topology_parse_cpu_capacity() fails, we set the boolean
>> cap_parsing_failed to true and we free the raw_capacity. This is correct as
>> the function begins with a check against cap_parsing_failed thus protecting
>> the function to be re-entered.
>>
>> However, even it is impossible that can happen with the current code, let's
>
> Why impossible ?
>> move in the instructions:
>>
>> - cap_parsing_failed = true;
>> - free_raw_capacity();
>>
>> ... in the 'else' block when the error is detected, that is more semantically
>> correct.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>> drivers/base/arch_topology.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>> index b19d6d4..7311641 100644
>> --- a/drivers/base/arch_topology.c
>> +++ b/drivers/base/arch_topology.c
>> @@ -155,9 +155,9 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
>> pr_err("cpu_capacity: missing %pOF raw capacity\n",
>> cpu_node);
>> pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
>> + cap_parsing_failed = true;
>> + free_raw_capacity();
>> }
>> - cap_parsing_failed = true;
>> - free_raw_capacity();
>
> While it is fine to move free_raw_capacity(), it is not to move the
> other line. With your
> patch what will happen if the first CPU in DT doesn't have the
> "capacity-dmips-mhz"
> property set ? We will never set cap_parsing_failed and keep on
> re-entering this routine
> which wasn't required.

Ok.


--
<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

\
 
 \ /
  Last update: 2018-10-30 09:33    [W:0.059 / U:0.780 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site