| Message ID | 11-v1-d88406ed308e+418-vfio3_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Remove vfio_mdev.c, mdev_parent_ops and more | expand |
> diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c > index 5a3873d1a275ae..0ccfeb3dda2455 100644 > --- a/drivers/vfio/mdev/mdev_sysfs.c > +++ b/drivers/vfio/mdev/mdev_sysfs.c > @@ -244,11 +244,20 @@ static ssize_t remove_store(struct device *dev, struct device_attribute *attr, > > static DEVICE_ATTR_WO(remove); > > -static const struct attribute *mdev_device_attrs[] = { > +static struct attribute *mdev_device_attrs[] = { Why does this lose the const? Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Mon, Apr 26, 2021 at 04:20:11PM +0200, Christoph Hellwig wrote: > > diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c > > index 5a3873d1a275ae..0ccfeb3dda2455 100644 > > +++ b/drivers/vfio/mdev/mdev_sysfs.c > > @@ -244,11 +244,20 @@ static ssize_t remove_store(struct device *dev, struct device_attribute *attr, > > > > static DEVICE_ATTR_WO(remove); > > > > -static const struct attribute *mdev_device_attrs[] = { > > +static struct attribute *mdev_device_attrs[] = { > > Why does this lose the const? Due to the way the driver core sets up it structs: drivers/vfio/mdev/mdev_sysfs.c:273:11: error: initialization of ‘struct attribute **’ from incompatible pointer type ‘const struct attribute **’ [-Werror=incompatible-pointer-types] 273 | .attrs = mdev_device_attrs, struct attribute_group { [..] struct attribute **attrs; Thanks, Jason
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index 7e918241de10cc..93d0955ba993f9 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -302,6 +302,7 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) mdev->dev.parent = parent->dev; mdev->dev.bus = &mdev_bus_type; mdev->dev.release = mdev_device_release; + mdev->dev.groups = mdev_device_groups; mdev->type = type; /* Pairs with the put in mdev_device_release() */ kobject_get(&type->kobj); diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index 839567d059a07d..c6944d3eaf78fa 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -32,6 +32,8 @@ struct mdev_type { unsigned int type_group_id; }; +extern const struct attribute_group *mdev_device_groups[]; + #define to_mdev_type_attr(_attr) \ container_of(_attr, struct mdev_type_attribute, attr) #define to_mdev_type(_kobj) \ diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c index 5a3873d1a275ae..0ccfeb3dda2455 100644 --- a/drivers/vfio/mdev/mdev_sysfs.c +++ b/drivers/vfio/mdev/mdev_sysfs.c @@ -244,11 +244,20 @@ static ssize_t remove_store(struct device *dev, struct device_attribute *attr, static DEVICE_ATTR_WO(remove); -static const struct attribute *mdev_device_attrs[] = { +static struct attribute *mdev_device_attrs[] = { &dev_attr_remove.attr, NULL, }; +static const struct attribute_group mdev_device_group = { + .attrs = mdev_device_attrs, +}; + +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; @@ -262,15 +271,8 @@ int mdev_create_sysfs_files(struct mdev_device *mdev) ret = sysfs_create_link(kobj, &type->kobj, "mdev_type"); if (ret) goto type_link_failed; - - ret = sysfs_create_files(kobj, mdev_device_attrs); - if (ret) - goto create_files_failed; - return ret; -create_files_failed: - sysfs_remove_link(kobj, "mdev_type"); type_link_failed: sysfs_remove_link(mdev->type->devices_kobj, dev_name(&mdev->dev)); return ret; @@ -280,7 +282,6 @@ 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(mdev->type->devices_kobj, dev_name(&mdev->dev)); }
The device creator is supposed to use the dev.groups value to add sysfs files before device_add is called, not call sysfs_create_files() after device_add() returns. This creates a race with uevent delivery where the extra attribute will not be visible. This was being done because the groups had been co-opted by the mdev driver, now that prior patches have moved the driver's groups to the struct device_driver the dev.group is properly free for use here. Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/vfio/mdev/mdev_core.c | 1 + drivers/vfio/mdev/mdev_private.h | 2 ++ drivers/vfio/mdev/mdev_sysfs.c | 19 ++++++++++--------- 3 files changed, 13 insertions(+), 9 deletions(-)