From: Jason Gunthorpe <[email protected]>
This is only used by one sample to print a fixed string that is pointless.
In general, having a device driver attach sysfs attributes to the parent
is horrific. This should never happen, and always leads to some kind of
liftime bug as it become very difficult for the sysfs attribute to go back
to any data owned by the device driver.
Remove the general mechanism to create this abuse.
Signed-off-by: Jason Gunthorpe <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/vfio/mdev/mdev_sysfs.c | 12 ++----------
include/linux/mdev.h | 2 --
samples/vfio-mdev/mtty.c | 30 +-----------------------------
3 files changed, 3 insertions(+), 41 deletions(-)
diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c
index f5cf1931c54e4..66eef08833a4e 100644
--- a/drivers/vfio/mdev/mdev_sysfs.c
+++ b/drivers/vfio/mdev/mdev_sysfs.c
@@ -197,7 +197,6 @@ void parent_remove_sysfs_files(struct mdev_parent *parent)
remove_mdev_supported_type(type);
}
- sysfs_remove_groups(&parent->dev->kobj, parent->ops->dev_attr_groups);
kset_unregister(parent->mdev_types_kset);
}
@@ -213,17 +212,10 @@ int parent_create_sysfs_files(struct mdev_parent *parent)
INIT_LIST_HEAD(&parent->type_list);
- ret = sysfs_create_groups(&parent->dev->kobj,
- parent->ops->dev_attr_groups);
- if (ret)
- goto create_err;
-
ret = add_mdev_supported_type_groups(parent);
if (ret)
- sysfs_remove_groups(&parent->dev->kobj,
- parent->ops->dev_attr_groups);
- else
- return ret;
+ goto create_err;
+ return 0;
create_err:
kset_unregister(parent->mdev_types_kset);
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index 192aed656116f..69df1fa2cb6f9 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -36,7 +36,6 @@ struct device *mtype_get_parent_dev(struct mdev_type *mtype);
*
* @owner: The module owner.
* @device_driver: Which device driver to probe() on newly created devices
- * @dev_attr_groups: Attributes of the parent device.
* @mdev_attr_groups: Attributes of the mediated device.
* @supported_type_groups: Attributes to define supported types. It is mandatory
* to provide supported types.
@@ -47,7 +46,6 @@ struct device *mtype_get_parent_dev(struct mdev_type *mtype);
struct mdev_parent_ops {
struct module *owner;
struct mdev_driver *device_driver;
- const struct attribute_group **dev_attr_groups;
const struct attribute_group **mdev_attr_groups;
struct attribute_group **supported_type_groups;
};
diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c
index a0e1a469bd47a..4f227dc267859 100644
--- a/samples/vfio-mdev/mtty.c
+++ b/samples/vfio-mdev/mtty.c
@@ -1207,38 +1207,11 @@ static long mtty_ioctl(struct vfio_device *vdev, unsigned int cmd,
return -ENOTTY;
}
-static ssize_t
-sample_mtty_dev_show(struct device *dev, struct device_attribute *attr,
- char *buf)
-{
- return sprintf(buf, "This is phy device\n");
-}
-
-static DEVICE_ATTR_RO(sample_mtty_dev);
-
-static struct attribute *mtty_dev_attrs[] = {
- &dev_attr_sample_mtty_dev.attr,
- NULL,
-};
-
-static const struct attribute_group mtty_dev_group = {
- .name = "mtty_dev",
- .attrs = mtty_dev_attrs,
-};
-
-static const struct attribute_group *mtty_dev_groups[] = {
- &mtty_dev_group,
- NULL,
-};
-
static ssize_t
sample_mdev_dev_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- if (mdev_from_dev(dev))
- return sprintf(buf, "This is MDEV %s\n", dev_name(dev));
-
- return sprintf(buf, "\n");
+ return sprintf(buf, "This is MDEV %s\n", dev_name(dev));
}
static DEVICE_ATTR_RO(sample_mdev_dev);
@@ -1333,7 +1306,6 @@ static struct mdev_driver mtty_driver = {
static const struct mdev_parent_ops mdev_fops = {
.owner = THIS_MODULE,
.device_driver = &mtty_driver,
- .dev_attr_groups = mtty_dev_groups,
.supported_type_groups = mdev_type_groups,
};
--
2.30.2
On 4/11/2022 7:44 PM, Christoph Hellwig wrote:
> From: Jason Gunthorpe <[email protected]>
>
> This is only used by one sample to print a fixed string that is pointless.
>
> In general, having a device driver attach sysfs attributes to the parent
> is horrific. This should never happen, and always leads to some kind of
> liftime bug as it become very difficult for the sysfs attribute to go back
> to any data owned by the device driver.
>
> Remove the general mechanism to create this abuse.
>
> Signed-off-by: Jason Gunthorpe <[email protected]>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> drivers/vfio/mdev/mdev_sysfs.c | 12 ++----------
> include/linux/mdev.h | 2 --
> samples/vfio-mdev/mtty.c | 30 +-----------------------------
> 3 files changed, 3 insertions(+), 41 deletions(-)
>
Update Documentation/driver-api/vfio-mediated-device.rst where mtty
sample code explained. Tree command output should be updated, i.e remove
below 2 lines.
|-- mtty_dev
| `-- sample_mtty_dev
Rest looks good to me.
Thanks,
Kirti