Re: [PATCH v2] iommu: fix smmu initialization memory leak problem

Will Deacon
2022-12-01T13:32:39Z

On Thu, Dec 01, 2022 at 08:42:02PM +0800, liulongfang wrote:
> On 2022/11/29 23:24, Will Deacon wrote:
> > On Tue, Nov 22, 2022 at 08:00:39PM +0800, liulongfang wrote:
> >> On 2022/11/22 2:05, Will Deacon wrote:
> >>> On Mon, Nov 21, 2022 at 11:04:21AM +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.
> >>>>
> >>>> Changes v1 -> v2:
> >>>> -Improve arm_smmu_device_probe() abnormal exit function.
> >>>>
> >>>> Signed-off-by: Longfang Liu <liulongfang@huawei.com>
> >>>> ---
> >>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 +++++++++++-----
> >>>> 1 file changed, 11 insertions(+), 5 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 ab160198edd6..b892f5233f88 100644
> >>>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> >>>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> >>>> @@ -3815,7 +3815,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> >>>> /* Initialise in-memory data structures */
> >>>> ret = arm_smmu_init_structures(smmu);
> >>>> if (ret)
> >>>> - return ret;
> >>>> + goto err_iopf;
> >>>>
> >>>> /* Record our private device structure */
> >>>> platform_set_drvdata(pdev, smmu);
> >>>> @@ -3826,22 +3826,28 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> >>>> /* Reset the device */
> >>>> ret = arm_smmu_device_reset(smmu, bypass);
> >>>> if (ret)
> >>>> - return ret;
> >>>> + goto err_iopf;
> >>>>
> >>>> /* And we're up. Go go go! */
> >>>> ret = iommu_device_sysfs_add(&smmu->iommu, dev, NULL,
> >>>> "smmu3.%pa", &ioaddr);
> >>>> if (ret)
> >>>> - return ret;
> >>>> + goto err_reset;
> >>>>
> >>>> 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_add;
> >>>> }
> >>>>
> >>>> return 0;
> >>>> +err_sysfs_add:
> >>>> + iommu_device_sysfs_remove(&smmu->iommu);
> >>>> +err_reset:
> >>>> + arm_smmu_device_disable(smmu);
> >>>> +err_iopf:
> >>>> + iopf_queue_free(smmu->evtq.iopf);
> >>>> + return ret;
> >>>
> >>> I previously suggested using devres_alloc() for this instead. Did that
> >>> not work?
> >>>
> >>
> >> This patch is only for fixing iopf's memory leak.
> >> The use of devres_alloc() is an optimization solution for iopf queue management,
> >> which is another set of patch matters.
> >
> > Great, I look forward to that set of patches!
> >
>
> Will this patch be merged into the next branch?

I don't plan to merge this one, no. I'll wait for the other patches which do
this using devres_alloc() instead.

Thanks,

Will