Re: [PATCH 04/13] vfio/mdev: remove mdev_{create,remove}_sysfs_files

[Kirti Wankhede <kwankhede@nvidia.com>]

Does this change really required? When mdev was implemented we tried to 
keep all sysfs related stuff in mdev_sysfs.c file and I would still like 
to stick to that approach.

Thanks.
Kirti

On 6/14/2022 10:24 AM, Christoph Hellwig wrote:
> Just fold these two trivial helpers into their only callers.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> ---
>   drivers/vfio/mdev/mdev_core.c    | 12 ++++++++++--
>   drivers/vfio/mdev/mdev_private.h |  3 ---
>   drivers/vfio/mdev/mdev_sysfs.c   | 28 ----------------------------
>   3 files changed, 10 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index 71c7f4e521a74..fe4230b21993d 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -44,7 +44,8 @@ static void mdev_device_remove_common(struct mdev_device *mdev)
>   {
>   	struct mdev_parent *parent = mdev->type->parent;
>   
> -	mdev_remove_sysfs_files(mdev);
> +	sysfs_remove_link(&mdev->dev.kobj, "mdev_type");
> +	sysfs_remove_link(mdev->type->devices_kobj, dev_name(&mdev->dev));
>   	device_del(&mdev->dev);
>   	lockdep_assert_held(&parent->unreg_sem);
>   	/* Balances with device_initialize() */
> @@ -212,16 +213,23 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid)
>   	if (ret)
>   		goto out_del;
>   
> -	ret = mdev_create_sysfs_files(mdev);
> +	ret = sysfs_create_link(type->devices_kobj, &mdev->dev.kobj,
> +				dev_name(&mdev->dev));
>   	if (ret)
>   		goto out_del;
>   
> +	ret = sysfs_create_link(&mdev->dev.kobj, &type->kobj, "mdev_type");
> +	if (ret)
> +		goto out_remove_type_link;
> +
>   	mdev->active = true;
>   	dev_dbg(&mdev->dev, "MDEV: created\n");
>   	up_read(&parent->unreg_sem);
>   
>   	return 0;
>   
> +out_remove_type_link:
> +	sysfs_remove_link(mdev->type->devices_kobj, dev_name(&mdev->dev));
>   out_del:
>   	device_del(&mdev->dev);
>   out_unlock:
> diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h
> index 6980f504018f3..346b9ec320466 100644
> --- a/drivers/vfio/mdev/mdev_private.h
> +++ b/drivers/vfio/mdev/mdev_private.h
> @@ -20,9 +20,6 @@ extern const struct attribute_group *mdev_device_groups[];
>   #define to_mdev_type(_kobj)		\
>   	container_of(_kobj, struct mdev_type, kobj)
>   
> -int  mdev_create_sysfs_files(struct mdev_device *mdev);
> -void mdev_remove_sysfs_files(struct mdev_device *mdev);
> -
>   int mdev_type_add(struct mdev_parent *parent, struct mdev_type *type);
>   void mdev_type_remove(struct mdev_type *type);
>   
> diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c
> index 09745e8e120f9..dd81b91afcf7d 100644
> --- a/drivers/vfio/mdev/mdev_sysfs.c
> +++ b/drivers/vfio/mdev/mdev_sysfs.c
> @@ -170,31 +170,3 @@ const struct attribute_group *mdev_device_groups[] = {
>   	&mdev_device_group,
>   	NULL
>   };
> -
> -int mdev_create_sysfs_files(struct mdev_device *mdev)
> -{
> -	struct mdev_type *type = mdev->type;
> -	struct kobject *kobj = &mdev->dev.kobj;
> -	int ret;
> -
> -	ret = sysfs_create_link(type->devices_kobj, kobj, dev_name(&mdev->dev));
> -	if (ret)
> -		return ret;
> -
> -	ret = sysfs_create_link(kobj, &type->kobj, "mdev_type");
> -	if (ret)
> -		goto type_link_failed;
> -	return ret;
> -
> -type_link_failed:
> -	sysfs_remove_link(mdev->type->devices_kobj, dev_name(&mdev->dev));
> -	return ret;
> -}
> -
> -void mdev_remove_sysfs_files(struct mdev_device *mdev)
> -{
> -	struct kobject *kobj = &mdev->dev.kobj;
> -
> -	sysfs_remove_link(kobj, "mdev_type");
> -	sysfs_remove_link(mdev->type->devices_kobj, dev_name(&mdev->dev));
> -}