Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752236AbbLRKY0 (ORCPT ); Fri, 18 Dec 2015 05:24:26 -0500 Received: from hqemgate15.nvidia.com ([216.228.121.64]:14935 "EHLO hqemgate15.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751213AbbLRKYX (ORCPT ); Fri, 18 Dec 2015 05:24:23 -0500 X-PGP-Universal: processed; by hqnvupgp07.nvidia.com on Fri, 18 Dec 2015 02:09:40 -0800 Subject: Re: [RFC PATCH V2 5/8] irqchip/gic: Return an error if GIC initialisation fails To: Linus Walleij References: <1450349309-8107-1-git-send-email-jonathanh@nvidia.com> <1450349309-8107-6-git-send-email-jonathanh@nvidia.com> CC: Thomas Gleixner , Jason Cooper , Marc Zyngier , Jiang Liu , Stephen Warren , Thierry Reding , Kevin Hilman , "Geert Uytterhoeven" , Grygorii Strashko , Lars-Peter Clausen , "Soren Brinkmann" , "linux-kernel@vger.kernel.org" , "linux-tegra@vger.kernel.org" From: Jon Hunter Message-ID: <5673DED0.9060505@nvidia.com> Date: Fri, 18 Dec 2015 10:24:16 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [10.21.132.159] X-ClientProxiedBy: UKMAIL101.nvidia.com (10.26.138.13) To UKMAIL101.nvidia.com (10.26.138.13) Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1632 Lines: 49 On 17/12/15 13:26, Linus Walleij wrote: > On Thu, Dec 17, 2015 at 11:48 AM, Jon Hunter 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 > (...) > > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/