Re: [PATCH v2 05/10] iommu: Add iommu_init/deinit_device() paired functions

[Baolu Lu <baolu.lu@linux.intel.com>]

On 5/21/23 7:31 PM, Baolu Lu wrote:
>>       for_each_group_device(group, device) {
>>           if (device->dev != dev)
>> @@ -510,44 +564,30 @@ static void __iommu_group_remove_device(struct 
>> device *dev)
>>           list_del(&device->list);
>>           __iommu_group_free_device(group, device);
>> -        /* Caller must put iommu_group */
>> -        return;
>> +        if (dev->iommu && dev->iommu->iommu_dev)
>> +            iommu_deinit_device(dev);
>> +        else
>> +            dev->iommu_group = NULL;
>> +        goto out;
>>       }
>>       WARN(true, "Corrupted iommu_group device_list");
>> +out:
>> +    mutex_unlock(&group->mutex);
>> +
>> +    /* Pairs with the get in iommu_group_add_device() */
>> +    iommu_group_put(group);
> 
> The group->devices_kobj was increased on the probe device path twice:
> 
> - iommu_init_device() - allocate the group
> - iommu_group_add_device() - add device to the group
> 
> But, on the release path, it seems that group->devices_kobj is only
> decreased once.
> 
> Did I overlook anything? Otherwise, the group will never be released,
> right?

I can answer this question by myself now. The
iommu_group_add/remove_device() helpers are only for external users.
They are not on the internal probe/release paths.

The code is fine. I can see below debug message during my test:

         pr_debug("Releasing group %d\n", group->id);

Sorry for the noise.

Best regards,
baolu