Re: [PATCH 3/13] vfio/mdev: simplify mdev_type handling

[Kirti Wankhede <kwankhede@nvidia.com>]

On 6/24/2022 2:57 AM, Jason Gunthorpe wrote:
> On Tue, Jun 14, 2022 at 06:54:18AM +0200, Christoph Hellwig wrote:
> 
>> diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c
>> index b71ffc5594870..09745e8e120f9 100644
>> --- a/drivers/vfio/mdev/mdev_sysfs.c
>> +++ b/drivers/vfio/mdev/mdev_sysfs.c
>> @@ -80,8 +80,6 @@ static void mdev_type_release(struct kobject *kobj)
>>   	struct mdev_type *type = to_mdev_type(kobj);
>>   
>>   	pr_debug("Releasing group %s\n", kobj->name);
>> -	/* Pairs with the get in add_mdev_supported_type() */
>> -	put_device(type->parent->dev);
> 
> I couldn't figure out why is it now OK to delete this?
> 
> It still looks required because mdev_core.c continues to use
> mdev-type->parent in various places and that pointer was being
> protected by the
> 
>     kobject_get(&type->kobj);
> 
> in mdev_device_create() through the above kref..
> 
> eg after the whole series is applied this looks troubled:
> 
> 	/* Pairs with the get in mdev_device_create() */
> 	kobject_put(&mdev->type->kobj);
> 
> 	mutex_lock(&mdev_list_lock);
> 	list_del(&mdev->next);
> 	mdev->type->parent->available_instances++;
>          ^^^^^^^^^^^^^^^^^^^^^
> 
> As there is now nothing keeping parent or type alive?
> 
> I think what would be good here is to directly
> get_device(type->parent->dev) in mdev_device_create() and put in
> mdev_device_release() then it is really clear how parent and
> parent->dev are kept alive.
> 
> It looks like we also have to keep the type around and ref'd too for
> the sysfs manipulation.
> 
> But overall I think this is fine
> 

See my reply on [PATCH 2/13].

Thanks,
Kirti



> Jason