Messages in this thread | | | Subject | Re: [PATCH 3/4] base/drivers/topology: Move instructions in the error path | From | Daniel Lezcano <> | Date | Tue, 30 Oct 2018 09:32:44 +0100 |
| |
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
| |