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

liulongfang
2022-11-22T12:01:55Z

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.

Thanks,
Longfang.
> Will
> .