From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5021679F8 for ; Mon, 14 Nov 2022 18:08:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 12482C433C1; Mon, 14 Nov 2022 18:08:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1668449307; bh=6ggcnc4EObNXAZUMEIKulorxPo+p100CcZmu7UYGb10=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=EScOKEF44ljWX/gUv9/uotNv54Kv7ghtCDDZi1ZPDSO8XhstlO9EuqqwviqhrV0gw MCUCIh68fnbB7WY0yxfxhJePGoCgXtnSfpODVHIUWsln6deXWrWaRAnvzcwY5UfMyf kVYWwPkr0U+XPqTXfVdiSjvQQTBu00gj2J8Xt6G2j8HEjXBqN7gBgIP6DNF9JpFQy6 lEX8H0wv9N0z9byS7aZxEbZ654Q7KNvjis2rOeQJJfOAO7QbfL13Hq7wnRzK+EQVP7 lT7UgoMUdWSd6T5PND93GTDLTcsF+cPLCNa66v8rXrI+HjFm8eX+wpryPwgA2157R0 nvYDmVhkP2S3w== Date: Mon, 14 Nov 2022 18:08:22 +0000 From: Will Deacon To: Longfang Liu Cc: robin.murphy@arm.com, linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev Subject: Re: [PATCH 2/2] iommu: fix smmu initialization memory leak problem Message-ID: <20221114180821.GC31476@willie-the-truck> References: <20221021035147.15292-1-liulongfang@huawei.com> <20221021035147.15292-3-liulongfang@huawei.com> Precedence: bulk X-Mailing-List: iommu@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221021035147.15292-3-liulongfang@huawei.com> User-Agent: Mutt/1.10.1 (2018-07-13) On Fri, Oct 21, 2022 at 11:51:47AM +0800, Longfang Liu wrote: > When iommu_device_register() in arm_smmu_device_probe() fails, > in addition to sysfs needs to be deleted, device should also > be disabled, and the memory of iopf needs to be released to > prevent memory leak of iopf. > > Signed-off-by: Longfang Liu > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > index a1db07bed6a9..c70defb0c866 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -3816,11 +3816,16 @@ static int arm_smmu_device_probe(struct platform_device *pdev) > ret = iommu_device_register(&smmu->iommu, &arm_smmu_ops, dev); > if (ret) { > dev_err(dev, "Failed to register iommu\n"); > - iommu_device_sysfs_remove(&smmu->iommu); > - return ret; > + goto err_sysfs_remove; > } > > return 0; > + > +err_sysfs_remove: > + iommu_device_sysfs_remove(&smmu->iommu); > + arm_smmu_device_disable(smmu); > + iopf_queue_free(smmu->evtq.iopf); > + return ret; Doesn't this miss the cases where iommu_device_sysfs_add() or arm_smmu_device_reset() fail? We'd probably be better off using something like devres_alloc() to track the iopf queue here. Will 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 7A909C4332F for ; Mon, 14 Nov 2022 18:09:36 +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=mdGv9GCArDiRNpgduKKIzTUnrkjQWp8sFo9blJ2QhUA=; b=0e4L7CT3R4CBZ0 P9vcPbC7Kv9ako3WmBdzj7I5Nq4lKS0AaDnVaFAteT9TUES6jUlQhpmYhkxRo708J+vRwC0IfxHVw deiaZNn+BkMbLTSrXlFvRj+IoIui0r8vlt8qkFk5unNdyQ+aD5iXBCj7dm10k6bs8huZZq6i/MgYc rSVg6JI1iH8YQcaUfwfz68SuqWQd++SWy9wzeI62ZfbTUmH6oZjmKNW8i9jr/YGHFPsq70LCeVzYa B8dK/78/ZU2brQJS06vlUnoClzH7tog4j3IEeJueMfSNYo1EAlOXb8S1iUxNS/7avFyJZALP1aRI+ CEXWwK0n5TOOstoEmuvQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oudsn-003bKx-UO; Mon, 14 Nov 2022 18:08:34 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oudsi-003bHT-84 for linux-arm-kernel@lists.infradead.org; Mon, 14 Nov 2022 18:08:30 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 8BC226131C; Mon, 14 Nov 2022 18:08:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 12482C433C1; Mon, 14 Nov 2022 18:08:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1668449307; bh=6ggcnc4EObNXAZUMEIKulorxPo+p100CcZmu7UYGb10=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=EScOKEF44ljWX/gUv9/uotNv54Kv7ghtCDDZi1ZPDSO8XhstlO9EuqqwviqhrV0gw MCUCIh68fnbB7WY0yxfxhJePGoCgXtnSfpODVHIUWsln6deXWrWaRAnvzcwY5UfMyf kVYWwPkr0U+XPqTXfVdiSjvQQTBu00gj2J8Xt6G2j8HEjXBqN7gBgIP6DNF9JpFQy6 lEX8H0wv9N0z9byS7aZxEbZ654Q7KNvjis2rOeQJJfOAO7QbfL13Hq7wnRzK+EQVP7 lT7UgoMUdWSd6T5PND93GTDLTcsF+cPLCNa66v8rXrI+HjFm8eX+wpryPwgA2157R0 nvYDmVhkP2S3w== Date: Mon, 14 Nov 2022 18:08:22 +0000 From: Will Deacon To: Longfang Liu Cc: robin.murphy@arm.com, linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev Subject: Re: [PATCH 2/2] iommu: fix smmu initialization memory leak problem Message-ID: <20221114180821.GC31476@willie-the-truck> References: <20221021035147.15292-1-liulongfang@huawei.com> <20221021035147.15292-3-liulongfang@huawei.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20221021035147.15292-3-liulongfang@huawei.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221114_100828_351624_916B7E0E X-CRM114-Status: GOOD ( 19.55 ) 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 Fri, Oct 21, 2022 at 11:51:47AM +0800, Longfang Liu wrote: > When iommu_device_register() in arm_smmu_device_probe() fails, > in addition to sysfs needs to be deleted, device should also > be disabled, and the memory of iopf needs to be released to > prevent memory leak of iopf. > > Signed-off-by: Longfang Liu > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > index a1db07bed6a9..c70defb0c866 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -3816,11 +3816,16 @@ static int arm_smmu_device_probe(struct platform_device *pdev) > ret = iommu_device_register(&smmu->iommu, &arm_smmu_ops, dev); > if (ret) { > dev_err(dev, "Failed to register iommu\n"); > - iommu_device_sysfs_remove(&smmu->iommu); > - return ret; > + goto err_sysfs_remove; > } > > return 0; > + > +err_sysfs_remove: > + iommu_device_sysfs_remove(&smmu->iommu); > + arm_smmu_device_disable(smmu); > + iopf_queue_free(smmu->evtq.iopf); > + return ret; Doesn't this miss the cases where iommu_device_sysfs_add() or arm_smmu_device_reset() fail? We'd probably be better off using something like devres_alloc() to track the iopf queue here. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel