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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id F04C2C77B7A for ; Thu, 18 May 2023 14:37:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To: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-Owner; bh=GcXnr1KkBhrjB8tmHauV2ClAZNSGDZejuyBmuzNV4Ws=; b=bR614ViEcncgVZ lwlE2zt1jUpOW4OMHnl9givBb2m7wzEWk+wKu8Q8wQMr0JRQSMeWwEMT46IldIXu7fMijgrX8k0Gf pN1eut8bQBZS3qAyY7xLHtdC9ff757NyoxWRDlRnYcqbJm3e6cR6mYlxcCys07xsEs4of5ux4BvcR ZMKB74LSQ2WT7Raua5G5Yvp0TLMbkigJYf4XTHEhw0OnJimodgdPjdx18iqX9P+oNW1VtGQU1cy16 6qDauqnNYf+eJjg+ZEXpxi4gKImikSV+u+pcIRPZMOP/pDQgt6XwXSUJvxp4HO4uduq0Ouz3bArsz UUx7uhzj0aWSqLf8dK3g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pzekm-00DCws-0Y; Thu, 18 May 2023 14:37:16 +0000 Received: from mail-wr1-x42a.google.com ([2a00:1450:4864:20::42a]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pzekj-00DCvU-1L for linux-arm-kernel@lists.infradead.org; Thu, 18 May 2023 14:37:14 +0000 Received: by mail-wr1-x42a.google.com with SMTP id ffacd0b85a97d-30948709b3cso675387f8f.3 for ; Thu, 18 May 2023 07:37:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1684420624; x=1687012624; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=NuGTxg0Bi/ffAC7iy/DtX+nu1bNY6UiYni6S2M+CTBo=; b=uu6zp1lK/hmMYrNMGLBtZREqtb7Tu+8G65p8ZUDUwaLdTMlj+UlOePKjfCN/GuBkzr F4vu8SLwLHOnZgwdkzLpQ7TEZNPGJyaZ8EVhBMICnr9WtiqZBWJKaAQ6iyVVT69/BinU RFsOcXPuYQoKpgMFdIKp6UBt0oBB603zNxYojH7jg8LOpQUNg4LzKnmaJceWG0nq6vkl 01N4Nmy5kfh5IWX7dTfKa0+45woOKASQW7VEPBf3NT2pW25a1PEuzs6Rd8tqpNN5dfn5 6MjtRjasIyyQiBiY4Iz0BWSRxUJf1ZrKSlnGzjEIhvVKzbFIBfQprtTC7oEh0Mc5B2iz Jbng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684420624; x=1687012624; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=NuGTxg0Bi/ffAC7iy/DtX+nu1bNY6UiYni6S2M+CTBo=; b=MgUd0R3hVfz8nf7Qy7SuMaB9dGvFqC/NgC+ElSjnC/o3Fn4bJWLUsszl/mQfmg8AZh Vvff1vVaVwMwE8KvNpCDzbs54iYjggGRcO0x4a7S7VZXXP1WVsgTofCngh7St82YMOqG 8vwqtOvuxfZU9Ki0LqpwPRcwHl3NJbigyGSorxW6YH9Sh3IUze2Cf8xsQK1XknOWAp4R NFh6nidlcpj26dVwswmtuQ2xu015B/WW+D9JKg29gTu8lTz1ckdQkDhqu0ik5NakLapp O0AOITYDuH7zRWJtes2xJMXxqGPgbE+SsgBwdxJKBBL3E5+L6trVYZkHaRqALWRK1fXj SS3g== X-Gm-Message-State: AC+VfDykKVG+8CEerHrOcZU8hwU85mxqtBHBJsfjj84ju2K7j930+IqB ybiMV8KlIw1bnZpLWH9pJ3h3+w== X-Google-Smtp-Source: ACHHUZ7NJGL16Mofflwp6cgsGutN16kV09se5SAVk1PwcuHN/HbYK5qIkbY4G9uUq8a2s04i0T001Q== X-Received: by 2002:adf:f0cc:0:b0:306:2e48:6ded with SMTP id x12-20020adff0cc000000b003062e486dedmr1774419wro.13.1684420623773; Thu, 18 May 2023 07:37:03 -0700 (PDT) Received: from linaro.org ([86.121.163.20]) by smtp.gmail.com with ESMTPSA id k9-20020a5d5189000000b003063176ef09sm2424748wrv.6.2023.05.18.07.37.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 May 2023 07:37:03 -0700 (PDT) Date: Thu, 18 May 2023 17:37:01 +0300 From: Abel Vesa To: Zhanhao Hu Cc: Abel Vesa , Peng Fan , Michael Turquette , Stephen Boyd , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , hust-os-kernel-patches@googlegroups.com, Dongliang Mu , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] clk: imx93: fix memory leak and missing unwind goto in imx93_clocks_probe Message-ID: References: <20230426142552.217435-1-zero12113@hust.edu.cn> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230426142552.217435-1-zero12113@hust.edu.cn> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230518_073713_453646_7A34DBF2 X-CRM114-Status: GOOD ( 26.15 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 23-04-26 14:25:52, Zhanhao Hu wrote: > In function probe(), it returns directly without unregistered hws > when error occurs. > > I fix this by adding 'goto unregister_hws;' on line 295 and > line 310. Hi Zhanhao, I like your patch, but there are some things to improve in the commit message. Have a read here before rephrasing it: https://docs.kernel.org/process/submitting-patches.html Quoting the above doc: "Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders to the codebase to change its behaviour." > > Besides, I use devm_kzalloc() instead of kzalloc() to automatically > free the memory using devm_kfree() when error occurs. So this should be rephrased like: Use devm_kzalloc instead of kzalloc, to automatically free the memory... > > Similarly, I replace of_iomap() with devm_of_iomap() to automatically > handle the unused ioremap region. So I delete 'iounmap(anatop_base);' > in unregister_hws. Same here. Make it imperative rather than mentioning what you're doing in this patch. With the commit rephrased in a new version, I'll be more than happy to apply it. > > Fixes: 24defbe194b6 ("clk: imx: add i.MX93 clk") > Signed-off-by: Zhanhao Hu > Reviewed-by: Dongliang Mu > --- > This issue is found by static analysis and remains untested. > --- > drivers/clk/imx/clk-imx93.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/clk/imx/clk-imx93.c b/drivers/clk/imx/clk-imx93.c > index 07b4a043e449..b6c7c2725906 100644 > --- a/drivers/clk/imx/clk-imx93.c > +++ b/drivers/clk/imx/clk-imx93.c > @@ -264,7 +264,7 @@ static int imx93_clocks_probe(struct platform_device *pdev) > void __iomem *base, *anatop_base; > int i, ret; > > - clk_hw_data = kzalloc(struct_size(clk_hw_data, hws, > + clk_hw_data = devm_kzalloc(dev, struct_size(clk_hw_data, hws, > IMX93_CLK_END), GFP_KERNEL); > if (!clk_hw_data) > return -ENOMEM; > @@ -288,10 +288,12 @@ static int imx93_clocks_probe(struct platform_device *pdev) > "sys_pll_pfd2", 1, 2); > > np = of_find_compatible_node(NULL, NULL, "fsl,imx93-anatop"); > - anatop_base = of_iomap(np, 0); > + anatop_base = devm_of_iomap(dev, np, 0, NULL); > of_node_put(np); > - if (WARN_ON(!anatop_base)) > - return -ENOMEM; > + if (WARN_ON(IS_ERR(anatop_base))) { > + ret = PTR_ERR(base); > + goto unregister_hws; > + } > > clks[IMX93_CLK_ARM_PLL] = imx_clk_fracn_gppll_integer("arm_pll", "osc_24m", > anatop_base + 0x1000, > @@ -304,8 +306,8 @@ static int imx93_clocks_probe(struct platform_device *pdev) > np = dev->of_node; > base = devm_platform_ioremap_resource(pdev, 0); > if (WARN_ON(IS_ERR(base))) { > - iounmap(anatop_base); > - return PTR_ERR(base); > + ret = PTR_ERR(base); > + goto unregister_hws; > } > > for (i = 0; i < ARRAY_SIZE(root_array); i++) { > @@ -345,7 +347,6 @@ static int imx93_clocks_probe(struct platform_device *pdev) > > unregister_hws: > imx_unregister_hw_clocks(clks, IMX93_CLK_END); > - iounmap(anatop_base); > > return ret; > } > -- > 2.34.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel