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-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html