| Message ID | 20220614045428.278494-5-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/13] vfio/mdev: make mdev.h standalone includable | expand |
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)); > -}
On Thu, Jun 16, 2022 at 01:33:30AM +0530, Kirti Wankhede wrote: > > 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. It isn't strictly required, but it removes a lot of pointless boilerplate code.
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)); -}