> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Wednesday, March 24, 2021 1:55 AM
>
> The kobj pointer in mdev_device is actually pointing at a struct
> mdev_type. Use the proper type so things are understandable.
>
> There are a number of places that are confused and passing both the mdev
> and the mtype as function arguments, fix these to derive the mtype
> directly from the mdev to remove the redundancy.
>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> ---
> drivers/vfio/mdev/mdev_core.c | 16 ++++++----------
> drivers/vfio/mdev/mdev_private.h | 7 +++----
> drivers/vfio/mdev/mdev_sysfs.c | 15 ++++++++-------
> include/linux/mdev.h | 4 +++-
> 4 files changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/vfio/mdev/mdev_core.c
> b/drivers/vfio/mdev/mdev_core.c
> index 057922a1707e04..5ca0efa5266bad 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -73,11 +73,9 @@ static void mdev_put_parent(struct mdev_parent
> *parent)
> static void mdev_device_remove_common(struct mdev_device *mdev)
> {
> struct mdev_parent *parent;
> - struct mdev_type *type;
> int ret;
>
> - type = to_mdev_type(mdev->type_kobj);
> - mdev_remove_sysfs_files(mdev, type);
> + mdev_remove_sysfs_files(mdev);
> device_del(&mdev->dev);
> parent = mdev->parent;
> lockdep_assert_held(&parent->unreg_sem);
> @@ -241,13 +239,11 @@ static void mdev_device_release(struct device *dev)
> mdev_device_free(mdev);
> }
>
> -int mdev_device_create(struct kobject *kobj,
> - struct device *dev, const guid_t *uuid)
> +int mdev_device_create(struct mdev_type *type, const guid_t *uuid)
> {
> int ret;
> struct mdev_device *mdev, *tmp;
> struct mdev_parent *parent;
> - struct mdev_type *type = to_mdev_type(kobj);
>
> parent = mdev_get_parent(type->parent);
> if (!parent)
> @@ -285,14 +281,14 @@ int mdev_device_create(struct kobject *kobj,
> }
>
> device_initialize(&mdev->dev);
> - mdev->dev.parent = dev;
> + mdev->dev.parent = parent->dev;
> mdev->dev.bus = &mdev_bus_type;
> mdev->dev.release = mdev_device_release;
> dev_set_name(&mdev->dev, "%pUl", uuid);
> mdev->dev.groups = parent->ops->mdev_attr_groups;
> - mdev->type_kobj = kobj;
> + mdev->type = type;
>
> - ret = parent->ops->create(kobj, mdev);
> + ret = parent->ops->create(&type->kobj, mdev);
> if (ret)
> goto ops_create_fail;
>
> @@ -300,7 +296,7 @@ int mdev_device_create(struct kobject *kobj,
> if (ret)
> goto add_fail;
>
> - ret = mdev_create_sysfs_files(mdev, type);
> + ret = mdev_create_sysfs_files(mdev);
> if (ret)
> goto sysfs_fail;
>
> diff --git a/drivers/vfio/mdev/mdev_private.h
> b/drivers/vfio/mdev/mdev_private.h
> index bb60ec4a8d9d21..debf27f95b4f10 100644
> --- a/drivers/vfio/mdev/mdev_private.h
> +++ b/drivers/vfio/mdev/mdev_private.h
> @@ -40,11 +40,10 @@ struct mdev_type {
> int parent_create_sysfs_files(struct mdev_parent *parent);
> void parent_remove_sysfs_files(struct mdev_parent *parent);
>
> -int mdev_create_sysfs_files(struct mdev_device *mdev, struct mdev_type
> *type);
> -void mdev_remove_sysfs_files(struct mdev_device *mdev, struct
> mdev_type *type);
> +int mdev_create_sysfs_files(struct mdev_device *mdev);
> +void mdev_remove_sysfs_files(struct mdev_device *mdev);
>
> -int mdev_device_create(struct kobject *kobj,
> - struct device *dev, const guid_t *uuid);
> +int mdev_device_create(struct mdev_type *kobj, const guid_t *uuid);
> int mdev_device_remove(struct mdev_device *dev);
>
> #endif /* MDEV_PRIVATE_H */
> diff --git a/drivers/vfio/mdev/mdev_sysfs.c
> b/drivers/vfio/mdev/mdev_sysfs.c
> index 6a5450587b79e9..321b4d13ead7b8 100644
> --- a/drivers/vfio/mdev/mdev_sysfs.c
> +++ b/drivers/vfio/mdev/mdev_sysfs.c
> @@ -67,7 +67,7 @@ static ssize_t create_store(struct kobject *kobj, struct
> device *dev,
> if (ret)
> return ret;
>
> - ret = mdev_device_create(kobj, dev, &uuid);
> + ret = mdev_device_create(to_mdev_type(kobj), &uuid);
> if (ret)
> return ret;
>
> @@ -249,16 +249,17 @@ static const struct attribute *mdev_device_attrs[] =
> {
> NULL,
> };
>
> -int mdev_create_sysfs_files(struct mdev_device *mdev, struct mdev_type
> *type)
> +int mdev_create_sysfs_files(struct mdev_device *mdev)
> {
> struct kobject *kobj = &mdev->dev.kobj;
What about adding a local "struct mdev_type *type" here? otherwise,
Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> int ret;
>
> - ret = sysfs_create_link(type->devices_kobj, kobj, dev_name(&mdev-
> >dev));
> + ret = sysfs_create_link(mdev->type->devices_kobj, kobj,
> + dev_name(&mdev->dev));
> if (ret)
> return ret;
>
> - ret = sysfs_create_link(kobj, &type->kobj, "mdev_type");
> + ret = sysfs_create_link(kobj, &mdev->type->kobj, "mdev_type");
> if (ret)
> goto type_link_failed;
>
> @@ -271,15 +272,15 @@ int mdev_create_sysfs_files(struct mdev_device
> *mdev, struct mdev_type *type)
> create_files_failed:
> sysfs_remove_link(kobj, "mdev_type");
> type_link_failed:
> - sysfs_remove_link(type->devices_kobj, dev_name(&mdev->dev));
> + sysfs_remove_link(mdev->type->devices_kobj, dev_name(&mdev-
> >dev));
> return ret;
> }
>
> -void mdev_remove_sysfs_files(struct mdev_device *mdev, struct
> mdev_type *type)
> +void mdev_remove_sysfs_files(struct mdev_device *mdev)
> {
> struct kobject *kobj = &mdev->dev.kobj;
>
> sysfs_remove_files(kobj, mdev_device_attrs);
> sysfs_remove_link(kobj, "mdev_type");
> - sysfs_remove_link(type->devices_kobj, dev_name(&mdev->dev));
> + sysfs_remove_link(mdev->type->devices_kobj, dev_name(&mdev-
> >dev));
> }
> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index cb771c712da0f4..349e8ac1fe3382 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -10,13 +10,15 @@
> #ifndef MDEV_H
> #define MDEV_H
>
> +struct mdev_type;
> +
> struct mdev_device {
> struct device dev;
> struct mdev_parent *parent;
> guid_t uuid;
> void *driver_data;
> struct list_head next;
> - struct kobject *type_kobj;
> + struct mdev_type *type;
> struct device *iommu_device;
> bool active;
> };
> --
> 2.31.0
