2022-08-26 03:04:01

by Yuan Can

Subject: [PATCH] iommu: use iommu_group_ref_get to hold group reference

Replace kobject_get with iommu_group_ref_get for better
readability.

Signed-off-by: Yuan Can <[email protected]>
---
drivers/iommu/iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 780fb7071577..8e7f30f7188c 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -903,7 +903,7 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
goto err_free_name;
}

- kobject_get(group->devices_kobj);
+ iommu_group_ref_get(group);

dev->iommu_group = group;

--
2.17.1

2022-08-30 09:40:13

by Robin Murphy

Subject: Re: [PATCH] iommu: use iommu_group_ref_get to hold group reference

On 2022-08-26 03:40, Yuan Can wrote:
> Replace kobject_get with iommu_group_ref_get for better
> readability.

I disagree, sorry. iommu_group_ref_get() is meant as a helper for
drivers, and the core code uses the raw kobject internally everywhere
else, including several other times within this function. Especially the
raw kobject_put() in the cleanup path - in terms of readability I'd say
it's actively harmful to obfuscate what that pairs with.

Thanks,
Robin.

> Signed-off-by: Yuan Can <[email protected]>
> ---
> drivers/iommu/iommu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 780fb7071577..8e7f30f7188c 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -903,7 +903,7 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
> goto err_free_name;
> }
>
> - kobject_get(group->devices_kobj);
> + iommu_group_ref_get(group);
>
> dev->iommu_group = group;
>

2022-08-30 13:37:19

by Yuan Can

Subject: Re: [PATCH] iommu: use iommu_group_ref_get to hold group reference

在 2022/8/30 17:32, Robin Murphy 写道:
> On 2022-08-26 03:40, Yuan Can wrote:
>> Replace kobject_get with iommu_group_ref_get for better
>> readability.
>
> I disagree, sorry. iommu_group_ref_get() is meant as a helper for
> drivers, and the core code uses the raw kobject internally everywhere
> else, including several other times within this function. Especially
> the raw kobject_put() in the cleanup path - in terms of readability
> I'd say it's actively harmful to obfuscate what that pairs with.
>
> Thanks,
> Robin.

Understood, thanks for your reply.

Best regards,

Yuan Can

>
>> Signed-off-by: Yuan Can <[email protected]>
>> ---
>>   drivers/iommu/iommu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 780fb7071577..8e7f30f7188c 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -903,7 +903,7 @@ int iommu_group_add_device(struct iommu_group
>> *group, struct device *dev)
>>           goto err_free_name;
>>       }
>>   -    kobject_get(group->devices_kobj);
>> +    iommu_group_ref_get(group);
>>         dev->iommu_group = group;
>