Re: [PATCH v10 01/19] vfio: Mediated device Core driver

[Kirti Wankhede <kwankhede@xxxxxxxxxx>]

On 10/29/2016 10:00 AM, Jike Song wrote:
> On 10/27/2016 05:29 AM, Kirti Wankhede wrote:
>> +int mdev_register_device(struct device *dev, const struct parent_ops *ops)
>> +{
>> +	int ret;
>> +	struct parent_device *parent;
>> +
>> +	/* check for mandatory ops */
>> +	if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
>> +		return -EINVAL;
>> +
>> +	dev = get_device(dev);
>> +	if (!dev)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&parent_list_lock);
>> +
>> +	/* Check for duplicate */
>> +	parent = __find_parent_device(dev);
>> +	if (parent) {
>> +		ret = -EEXIST;
>> +		goto add_dev_err;
>> +	}
>> +
>> +	parent = kzalloc(sizeof(*parent), GFP_KERNEL);
>> +	if (!parent) {
>> +		ret = -ENOMEM;
>> +		goto add_dev_err;
>> +	}
>> +
>> +	kref_init(&parent->ref);
>> +	mutex_init(&parent->lock);
>> +
>> +	parent->dev = dev;
>> +	parent->ops = ops;
>> +
>> +	ret = parent_create_sysfs_files(parent);
>> +	if (ret) {
>> +		mutex_unlock(&parent_list_lock);
>> +		mdev_put_parent(parent);
>> +		return ret;
>> +	}
>> +
>> +	ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL);
>> +	if (ret)
>> +		dev_warn(dev, "Failed to create compatibility class link\n");
>> +
> 
> Hi Kirti,
> 
> Like I replied to previous version:
> 
> 	http://www.spinics.net/lists/kvm/msg139331.html
> 

Hi Jike,

I saw your reply but by that time v10 version of patch series was out
for review.

> You can always check if mdev_bus_compat_class already registered
> here, and register it if not yet. Same logic should be adopted to
> mdev_init.
> 
> Current implementation will simply panic if configured as builtin,
> which is rare but far from impossible.
> 

Can you verify attached patch with v10 patch-set whether this works for you?
I'll incorporate this change in my next version.

Thanks,
Kirti

From: Kirti Wankhede <kwankhede@xxxxxxxxxx>
Date: Sat, 29 Oct 2016 15:12:01 +0530
Subject: [PATCH 1/1] Register mdev_bus class on first mdev_device_register

Signed-off-by: Kirti Wankhede <kwankhede@xxxxxxxxxx>
Signed-off-by: Neo Jia <cjia@xxxxxxxxxx>
---
 drivers/vfio/mdev/mdev_core.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 9d8fa5c91c2e..54c59f325336 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -187,13 +187,18 @@ int mdev_register_device(struct device *dev, const struct parent_ops *ops)
 	parent->dev = dev;
 	parent->ops = ops;
 
-	ret = parent_create_sysfs_files(parent);
-	if (ret) {
-		mutex_unlock(&parent_list_lock);
-		mdev_put_parent(parent);
-		return ret;
+	if (!mdev_bus_compat_class) {
+		mdev_bus_compat_class = class_compat_register("mdev_bus");
+		if (!mdev_bus_compat_class) {
+			ret = -ENOMEM;
+			goto add_dev_err;
+		}
 	}
 
+	ret = parent_create_sysfs_files(parent);
+	if (ret)
+		goto add_dev_err;
+
 	ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL);
 	if (ret)
 		dev_warn(dev, "Failed to create compatibility class link\n");
@@ -206,7 +211,10 @@ int mdev_register_device(struct device *dev, const struct parent_ops *ops)
 
 add_dev_err:
 	mutex_unlock(&parent_list_lock);
-	put_device(dev);
+	if (parent)
+		mdev_put_parent(parent);
+	else
+		put_device(dev);
 	return ret;
 }
 EXPORT_SYMBOL(mdev_register_device);
@@ -354,12 +362,6 @@ static int __init mdev_init(void)
 		return ret;
 	}
 
-	mdev_bus_compat_class = class_compat_register("mdev_bus");
-	if (!mdev_bus_compat_class) {
-		mdev_bus_unregister();
-		return -ENOMEM;
-	}
-
 	/*
 	 * Attempt to load known vfio_mdev.  This gives us a working environment
 	 * without the user needing to explicitly load vfio_mdev driver.
@@ -371,7 +373,9 @@ static int __init mdev_init(void)
 
 static void __exit mdev_exit(void)
 {
-	class_compat_unregister(mdev_bus_compat_class);
+	if (mdev_bus_compat_class)
+		class_compat_unregister(mdev_bus_compat_class);
+
 	mdev_bus_unregister();
 }
 
-- 
2.7.0