From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,USER_AGENT_NEOMUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0115FC43381 for ; Tue, 5 Mar 2019 11:41:11 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C10E820842 for ; Tue, 5 Mar 2019 11:41:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Cw9kY4QB"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="dL+AEf9M" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C10E820842 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=armlinux.org.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=bc/97l9nmgXJrLXJOz+QM+j/D7+hpwD+dEM9HdVBOj8=; b=Cw9kY4QBjdvkbd 5iZYC2T3CaQoXlRBvk7Y1ogNjmVDHZ3WovJqnf+UDnaNZsBraC/m1l+NaF/4bmczAuRbD4jfGKMyY m3/cDfVWtDoUxL5kKjpMM3wsUGOYREGM3nlO0DfEXCu5OveOU4oXp6qNacxDOeLed7aTC05tPVSIM hD8yWXVOimI1Tc6NWnDorksAdGIRl0BunxjYWHfCfT9FifsPFHq2DPEmts+nAU3fKZeEs1mXrzxqV h5Ks+G+C2JpdIYE8f1rWpy3K/B82v3S3ev+Oo3ImDnxSsIBxWuIaHog8grFSfF44ZfLe+CEMtkb3t h1IXmigGlUZaha8GL9Kg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h18RY-00022D-0a; Tue, 05 Mar 2019 11:41:08 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h18RU-00021m-8E for linux-arm-kernel@lists.infradead.org; Tue, 05 Mar 2019 11:41:06 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=cjsHhJalwzEJjk0J0bDWA/Eixzr743EMPJ6TG/6uaXs=; b=dL+AEf9MLmtrfMMiukk7SACDO Xd07FMEw79yWf6CYjG69/qBVG7zIuC9JYfHaUlIYuw/gWjYREw/g2Lx9+ZaV5WNMjRZ2eip8Hrp4/ aEHOXbsN648ABsa27LNJJW6mZSVGZocnqqtLrKDecC3GShXt1iDI2P9SDUvxwiSsWSHbIBSV9z4Cy 4RCMRnDaCwpXwQiwT6fYFkdT7zEDqN4wZTqjf4mMegLec+ex3FQ64IPHbbqhHjqj0HvgHNeGsuuGw yc5PO9iTgi5GJmIpTiX400d52UMiN2zRBT21HdSkP4EP6Tqgg04FbL5rTsZGkOH0B7mNqoGikeqex Nv3hp4Gtw==; Received: from shell.armlinux.org.uk ([2001:4d48:ad52:3201:5054:ff:fe00:4ec]:54946) by pandora.armlinux.org.uk with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1h18RH-0000yE-2J; Tue, 05 Mar 2019 11:40:51 +0000 Received: from linux by shell.armlinux.org.uk with local (Exim 4.89) (envelope-from ) id 1h18RF-0003AN-13; Tue, 05 Mar 2019 11:40:49 +0000 Date: Tue, 5 Mar 2019 11:40:48 +0000 From: Russell King - ARM Linux admin To: Wen Yang Subject: Re: [PATCH v2 01/15] ARM: actions: fix a leaked reference by adding missing of_node_put Message-ID: <20190305114048.egpbeddy4pqbwl5b@shell.armlinux.org.uk> References: <1551785646-46173-1-git-send-email-wen.yang99@zte.com.cn> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1551785646-46173-1-git-send-email-wen.yang99@zte.com.cn> User-Agent: NeoMutt/20170113 (1.7.2) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190305_034104_306317_55079D44 X-CRM114-Status: GOOD ( 20.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: wang.yi59@zte.com.cn, Manivannan Sadhasivam , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Andreas =?iso-8859-1?Q?F=E4rber?= Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Mar 05, 2019 at 07:33:52PM +0800, Wen Yang wrote: > The call to of_get_next_child returns a node pointer with refcount > incremented thus it must be explicitly decremented after the last > usage. > = > Detected by coccinelle with the following warnings: > ./arch/arm/mach-actions/platsmp.c:112:2-8: ERROR: missing of_node_put; ac= quired a node pointer with refcount incremented on line 103, but without a = corresponding object release within this function. > ./arch/arm/mach-actions/platsmp.c:124:2-8: ERROR: missing of_node_put; ac= quired a node pointer with refcount incremented on line 115, but without a = corresponding object release within this function. > ./arch/arm/mach-actions/platsmp.c:137:3-9: ERROR: missing of_node_put; ac= quired a node pointer with refcount incremented on line 128, but without a = corresponding object release within this function. I question this. Your reasoning is that the node is no longer used so the reference count needs to be put. However, in all these cases, data is read from the nodes properties and the device remains in-use for the life of the kernel. There is a big difference here. With normal drivers, each device is bound to their associated device node associated with the device. When the device node goes away, then the corresponding device goes away too, which causes the driver to be unbound from the device. However, there is another class of "driver" which are the ones below, where they are "permanent" devices. These can never go away, even if the device node refcount hits zero and the device node is freed - the device is still present and in-use in the system. So, having the device node refcount hit zero is actually a bug: what that's saying is the system device (eg, SCU) has gone away. If you somehow were to remove the SCU from the system, you'd end up severing the connection between the CPU cores and the rest of the system - obviously resulting in a dead system! So, what is the point of dropping these refcounts for devices that can never go away - and thus their associated device_node should also never go away? > = > Signed-off-by: Wen Yang > Reviewed-by: Florian Fainelli > Cc: "Andreas F=E4rber" > Cc: Manivannan Sadhasivam > Cc: Russell King > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > --- > v2->v1: add a missing space between "adding" and "missing" > = > arch/arm/mach-actions/platsmp.c | 3 +++ > 1 file changed, 3 insertions(+) > = > diff --git a/arch/arm/mach-actions/platsmp.c b/arch/arm/mach-actions/plat= smp.c > index 4fd479c..1a8e078 100644 > --- a/arch/arm/mach-actions/platsmp.c > +++ b/arch/arm/mach-actions/platsmp.c > @@ -107,6 +107,7 @@ static void __init s500_smp_prepare_cpus(unsigned int= max_cpus) > } > = > timer_base_addr =3D of_iomap(node, 0); > + of_node_put(node); > if (!timer_base_addr) { > pr_err("%s: could not map timer registers\n", __func__); > return; > @@ -119,6 +120,7 @@ static void __init s500_smp_prepare_cpus(unsigned int= max_cpus) > } > = > sps_base_addr =3D of_iomap(node, 0); > + of_node_put(node); > if (!sps_base_addr) { > pr_err("%s: could not map sps registers\n", __func__); > return; > @@ -132,6 +134,7 @@ static void __init s500_smp_prepare_cpus(unsigned int= max_cpus) > } > = > scu_base_addr =3D of_iomap(node, 0); > + of_node_put(node); > if (!scu_base_addr) { > pr_err("%s: could not map scu registers\n", __func__); > return; > -- = > 2.9.5 > = > = -- = RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps = up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel