Re: [RFC PATCH V2 5/8] irqchip/gic: Return an error if GIC initialisation fails

[Jon Hunter]

On 17/12/15 13:26, Linus Walleij wrote:
> On Thu, Dec 17, 2015 at 11:48 AM, Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
> 
>> If the GIC initialisation fails, then currently we do not return an error
>> or clean-up afterwards. Although for root controllers, this failure may be
>> fatal anyway, for secondary controllers, it may not be fatal and so return
>> an error on failure and clean-up.
>>
>> Also for non-banked GIC controllers, make sure that we free any memory
>> allocated if we fail to initialise the IRQ domain.
>>
>> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> (...)
> 
> Almost perfect but...
> 
>> +err:
>> +       if (IS_ENABLED(CONFIG_GIC_NON_BANKED) && percpu_offset) {
>> +               free_percpu(gic->dist_base.percpu_base);
>> +               free_percpu(gic->cpu_base.percpu_base);
> 
> What if the first map worked but not the second?
> 
> Should it be:
> 
> if (gic->dist_base.percpu_base)
>    free_percpu(gic->dist_base.percpu_base);
> if (gic->cpu_base.percpu_base)
>    free_percpu(gic->cpu_base.percpu_base);
> 
> ?

Yes this is a bit lazy, but the first thing free_percpu() checks if the
pointer is NULL and simply returns.

If you look at the current code in __gic_init_bases(), if one of the two
fail, we still try to free both. I did not like this, but when I looked
at it, I could see that is does work. Happy to change it though.

Cheers
Jon


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/