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

liulongfang
2022-12-20T03:30:19Z

On 2022/12/1 21:31, Will Deacon Wrote:
> 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
> .
>
Hi Christophe:
"[PATCH] iommu/arm-smmu-v3: Fix an error handling path in arm_smmu_device_probe()"

the patch you sent is the same as mine. The maintainer hopes to optimize the queue
application part of iopf with devres_alloc().

I hope you can modify it, and I will quit this repair work.

Thanks,
Longfang.