diff mbox

[RFC,v6-based,v1,1/5] mdev: create separate device for parent_device

Message ID 1471335257-6226-2-git-send-email-jike.song@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jike Song Aug. 16, 2016, 8:14 a.m. UTC
From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

By introducing a separate device for parent_device, we can have
the parent list and lock removed, letting driver core and sysfs
to deal with the mutual exclusion.

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 drivers/vfio/mdev/mdev_core.c    | 242 +++++----------------------------------
 drivers/vfio/mdev/mdev_private.h |   9 +-
 drivers/vfio/mdev/mdev_sysfs.c   |  69 +++--------
 drivers/vfio/mdev/vfio_mpci.c    |   2 +-
 include/linux/mdev.h             |   6 +-
 5 files changed, 57 insertions(+), 271 deletions(-)
diff mbox

Patch

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 90ff073..9138588 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -27,11 +27,6 @@ 
 #define DRIVER_AUTHOR		"NVIDIA Corporation"
 #define DRIVER_DESC		"Mediated device Core Driver"
 
-#define MDEV_CLASS_NAME		"mdev"
-
-static LIST_HEAD(parent_list);
-static DEFINE_MUTEX(parent_list_lock);
-
 static int mdev_add_attribute_group(struct device *dev,
 				    const struct attribute_group **groups)
 {
@@ -58,55 +53,6 @@  static struct mdev_device *find_mdev_device(struct parent_device *parent,
 	return NULL;
 }
 
-/* Should be called holding parent_list_lock */
-static struct parent_device *find_parent_device(struct device *dev)
-{
-	struct parent_device *parent;
-
-	list_for_each_entry(parent, &parent_list, next) {
-		if (parent->dev == dev)
-			return parent;
-	}
-	return NULL;
-}
-
-static void mdev_release_parent(struct kref *kref)
-{
-	struct parent_device *parent = container_of(kref, struct parent_device,
-						    ref);
-	kfree(parent);
-}
-
-static
-inline struct parent_device *mdev_get_parent(struct parent_device *parent)
-{
-	if (parent)
-		kref_get(&parent->ref);
-
-	return parent;
-}
-
-static inline void mdev_put_parent(struct parent_device *parent)
-{
-	if (parent)
-		kref_put(&parent->ref, mdev_release_parent);
-}
-
-static struct parent_device *mdev_get_parent_by_dev(struct device *dev)
-{
-	struct parent_device *parent = NULL, *p;
-
-	mutex_lock(&parent_list_lock);
-	list_for_each_entry(p, &parent_list, next) {
-		if (p->dev == dev) {
-			parent = mdev_get_parent(p);
-			break;
-		}
-	}
-	mutex_unlock(&parent_list_lock);
-	return parent;
-}
-
 static int mdev_device_create_ops(struct mdev_device *mdev, char *mdev_params)
 {
 	struct parent_device *parent = mdev->parent;
@@ -153,7 +99,6 @@  static void mdev_release_device(struct kref *kref)
 
 	device_unregister(&mdev->dev);
 	wake_up(&parent->release_done);
-	mdev_put_parent(parent);
 }
 
 struct mdev_device *mdev_get_device(struct mdev_device *mdev)
@@ -173,66 +118,6 @@  void mdev_put_device(struct mdev_device *mdev)
 EXPORT_SYMBOL(mdev_put_device);
 
 /*
- * Find first mediated device from given uuid and increment refcount of
- * mediated device. Caller should call mdev_put_device() when the use of
- * mdev_device is done.
- */
-static struct mdev_device *mdev_get_first_device_by_uuid(uuid_le uuid)
-{
-	struct mdev_device *mdev = NULL, *p;
-	struct parent_device *parent;
-
-	mutex_lock(&parent_list_lock);
-	list_for_each_entry(parent, &parent_list, next) {
-		mutex_lock(&parent->mdev_list_lock);
-		list_for_each_entry(p, &parent->mdev_list, next) {
-			if (uuid_le_cmp(p->uuid, uuid) == 0) {
-				mdev = mdev_get_device(p);
-				break;
-			}
-		}
-		mutex_unlock(&parent->mdev_list_lock);
-
-		if (mdev)
-			break;
-	}
-	mutex_unlock(&parent_list_lock);
-	return mdev;
-}
-
-/*
- * Find mediated device from given iommu_group and increment refcount of
- * mediated device. Caller should call mdev_put_device() when the use of
- * mdev_device is done.
- */
-struct mdev_device *mdev_get_device_by_group(struct iommu_group *group)
-{
-	struct mdev_device *mdev = NULL, *p;
-	struct parent_device *parent;
-
-	mutex_lock(&parent_list_lock);
-	list_for_each_entry(parent, &parent_list, next) {
-		mutex_lock(&parent->mdev_list_lock);
-		list_for_each_entry(p, &parent->mdev_list, next) {
-			if (!p->group)
-				continue;
-
-			if (iommu_group_id(p->group) == iommu_group_id(group)) {
-				mdev = mdev_get_device(p);
-				break;
-			}
-		}
-		mutex_unlock(&parent->mdev_list_lock);
-
-		if (mdev)
-			break;
-	}
-	mutex_unlock(&parent_list_lock);
-	return mdev;
-}
-EXPORT_SYMBOL(mdev_get_device_by_group);
-
-/*
  * mdev_register_device : Register a device
  * @dev: device structure representing parent device.
  * @ops: Parent device operation structure to be registered.
@@ -252,30 +137,21 @@  int mdev_register_device(struct device *dev, const struct parent_ops *ops)
 	if (!ops->create || !ops->destroy)
 		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);
-	list_add(&parent->next, &parent_list);
-
-	parent->dev = dev;
+	parent->dev.parent = dev;
 	parent->ops = ops;
 	mutex_init(&parent->mdev_list_lock);
 	INIT_LIST_HEAD(&parent->mdev_list);
 	init_waitqueue_head(&parent->release_done);
-	mutex_unlock(&parent_list_lock);
+
+	ret = device_register(&parent->dev);
+	if (ret)
+		goto register_error;
 
 	ret = mdev_create_sysfs_files(dev);
 	if (ret)
@@ -291,50 +167,37 @@  int mdev_register_device(struct device *dev, const struct parent_ops *ops)
 add_group_error:
 	mdev_remove_sysfs_files(dev);
 add_sysfs_error:
-	mutex_lock(&parent_list_lock);
-	list_del(&parent->next);
-	mutex_unlock(&parent_list_lock);
-	mdev_put_parent(parent);
+	device_unregister(&parent->dev);
+register_error:
 	return ret;
 
 add_dev_err:
-	mutex_unlock(&parent_list_lock);
 	return ret;
 }
 EXPORT_SYMBOL(mdev_register_device);
 
 /*
  * mdev_unregister_device : Unregister a parent device
- * @dev: device structure representing parent device.
+ * @dev: device structure representing parent device which is returned by
+ *       mdev_register_device.
  *
  * Remove device from list of registered parent devices. Give a chance to free
  * existing mediated devices for given device.
  */
-
-void mdev_unregister_device(struct device *dev)
+void mdev_unregister_device(struct parent_device *parent)
 {
-	struct parent_device *parent;
 	struct mdev_device *mdev, *n;
 	int ret;
 
-	mutex_lock(&parent_list_lock);
-	parent = find_parent_device(dev);
-
-	if (!parent) {
-		mutex_unlock(&parent_list_lock);
-		return;
-	}
-	dev_info(dev, "MDEV: Unregistering\n");
+	dev_info(&parent->dev, "MDEV: Unregistering\n");
 
 	/*
 	 * Remove parent from the list and remove create and destroy sysfs
 	 * files so that no new mediated device could be created for this parent
 	 */
-	list_del(&parent->next);
-	mdev_remove_sysfs_files(dev);
-	mutex_unlock(&parent_list_lock);
+	mdev_remove_sysfs_files(&parent->dev);
 
-	mdev_remove_attribute_group(dev,
+	mdev_remove_attribute_group(&parent->dev,
 				    parent->ops->dev_attr_groups);
 
 	mutex_lock(&parent->mdev_list_lock);
@@ -350,14 +213,12 @@  void mdev_unregister_device(struct device *dev)
 		ret = wait_event_interruptible_timeout(parent->release_done,
 				list_empty(&parent->mdev_list), HZ * 10);
 		if (ret == -ERESTARTSYS) {
-			dev_warn(dev, "Mediated devices are in use, task"
+			dev_warn(&parent->dev, "Mediated devices are in use, task"
 				      " \"%s\" (%d) "
 				      "blocked until all are released",
 				      current->comm, task_pid_nr(current));
 		}
 	} while (ret <= 0);
-
-	mdev_put_parent(parent);
 }
 EXPORT_SYMBOL(mdev_unregister_device);
 
@@ -377,11 +238,7 @@  int mdev_device_create(struct device *dev, uuid_le uuid, uint32_t instance,
 {
 	int ret;
 	struct mdev_device *mdev;
-	struct parent_device *parent;
-
-	parent = mdev_get_parent_by_dev(dev);
-	if (!parent)
-		return -EINVAL;
+	struct parent_device *parent = dev_to_parent_dev(dev);
 
 	mutex_lock(&parent->mdev_list_lock);
 	/* Check for duplicate */
@@ -403,6 +260,7 @@  int mdev_device_create(struct device *dev, uuid_le uuid, uint32_t instance,
 	kref_init(&mdev->ref);
 
 	mdev->dev.parent  = dev;
+	mdev->dev.class = &mdev_class;
 	mdev->dev.bus     = &mdev_bus_type;
 	mdev->dev.release = mdev_device_release;
 	dev_set_name(&mdev->dev, "%pUl-%d", uuid.b, instance);
@@ -429,19 +287,14 @@  create_failed:
 
 create_err:
 	mutex_unlock(&parent->mdev_list_lock);
-	mdev_put_parent(parent);
 	return ret;
 }
 
 int mdev_device_destroy(struct device *dev, uuid_le uuid, uint32_t instance)
 {
 	struct mdev_device *mdev;
-	struct parent_device *parent;
 	int ret;
-
-	parent = mdev_get_parent_by_dev(dev);
-	if (!parent)
-		return -EINVAL;
+	struct parent_device *parent = dev_to_parent_dev(dev);
 
 	mutex_lock(&parent->mdev_list_lock);
 	mdev = find_mdev_device(parent, uuid, instance);
@@ -457,12 +310,10 @@  int mdev_device_destroy(struct device *dev, uuid_le uuid, uint32_t instance)
 	mutex_unlock(&parent->mdev_list_lock);
 	mdev_put_device(mdev);
 
-	mdev_put_parent(parent);
 	return ret;
 
 destroy_err:
 	mutex_unlock(&parent->mdev_list_lock);
-	mdev_put_parent(parent);
 	return ret;
 }
 
@@ -575,72 +426,37 @@  EXPORT_SYMBOL(mdev_del_phys_mapping);
 
 void mdev_device_supported_config(struct device *dev, char *str)
 {
-	struct parent_device *parent;
-
-	parent = mdev_get_parent_by_dev(dev);
+	struct parent_device *parent = dev_to_parent_dev(dev);
 
 	if (parent) {
 		if (parent->ops->supported_config)
-			parent->ops->supported_config(parent->dev, str);
-		mdev_put_parent(parent);
+			parent->ops->supported_config(&parent->dev, str);
 	}
 }
 
-int mdev_device_start(uuid_le uuid)
+int mdev_device_start(struct device *dev, bool start)
 {
 	int ret = 0;
-	struct mdev_device *mdev;
-	struct parent_device *parent;
+	struct mdev_device *mdev = dev_to_mdev(dev);
+	struct parent_device *parent = dev_to_parent_dev(dev->parent);
 
-	mdev = mdev_get_first_device_by_uuid(uuid);
-	if (!mdev)
-		return -EINVAL;
-
-	parent = mdev->parent;
-
-	if (parent->ops->start)
+	mdev_get_device(mdev);
+	if (start && parent->ops->start)
 		ret = parent->ops->start(mdev->uuid);
-
-	if (ret)
-		pr_err("mdev_start failed  %d\n", ret);
-	else
-		kobject_uevent(&mdev->dev.kobj, KOBJ_ONLINE);
-
-	mdev_put_device(mdev);
-
-	return ret;
-}
-
-int mdev_device_stop(uuid_le uuid)
-{
-	int ret = 0;
-	struct mdev_device *mdev;
-	struct parent_device *parent;
-
-	mdev = mdev_get_first_device_by_uuid(uuid);
-	if (!mdev)
-		return -EINVAL;
-
-	parent = mdev->parent;
-
-	if (parent->ops->stop)
+	else if (!start && parent->ops->stop)
 		ret = parent->ops->stop(mdev->uuid);
 
 	if (ret)
-		pr_err("mdev stop failed %d\n", ret);
+		pr_err("mdev %s failed  %d\n", start ? "start" : "stop", ret);
 	else
-		kobject_uevent(&mdev->dev.kobj, KOBJ_OFFLINE);
+		kobject_uevent(&mdev->dev.kobj,
+			       start ? KOBJ_ONLINE : KOBJ_OFFLINE);
 
 	mdev_put_device(mdev);
+
 	return ret;
 }
 
-static struct class mdev_class = {
-	.name		= MDEV_CLASS_NAME,
-	.owner		= THIS_MODULE,
-	.class_attrs	= mdev_class_attrs,
-};
-
 static int __init mdev_init(void)
 {
 	int ret;
diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h
index ee2db61..3fce40f 100644
--- a/drivers/vfio/mdev/mdev_private.h
+++ b/drivers/vfio/mdev/mdev_private.h
@@ -13,12 +13,16 @@ 
 #ifndef MDEV_PRIVATE_H
 #define MDEV_PRIVATE_H
 
+#define dev_to_parent_dev(_dev) container_of((_dev),	\
+					     struct parent_device, dev)
+#define dev_to_mdev(_dev) container_of((_dev), struct mdev_device, dev)
+
 int  mdev_bus_register(void);
 void mdev_bus_unregister(void);
 
 /* Function prototypes for mdev_sysfs */
 
-extern struct class_attribute mdev_class_attrs[];
+extern struct class mdev_class;
 
 int  mdev_create_sysfs_files(struct device *dev);
 void mdev_remove_sysfs_files(struct device *dev);
@@ -27,7 +31,6 @@  int  mdev_device_create(struct device *dev, uuid_le uuid, uint32_t instance,
 			char *mdev_params);
 int  mdev_device_destroy(struct device *dev, uuid_le uuid, uint32_t instance);
 void mdev_device_supported_config(struct device *dev, char *str);
-int  mdev_device_start(uuid_le uuid);
-int  mdev_device_stop(uuid_le uuid);
+int  mdev_device_start(struct device *dev, bool start);
 
 #endif /* MDEV_PRIVATE_H */
diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c
index e0457e6..3080edc 100644
--- a/drivers/vfio/mdev/mdev_sysfs.c
+++ b/drivers/vfio/mdev/mdev_sysfs.c
@@ -171,65 +171,34 @@  destroy_error:
 	return ret;
 }
 
-ssize_t mdev_start_store(struct class *class, struct class_attribute *attr,
-			 const char *buf, size_t count)
+static ssize_t start_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t len)
 {
-	char *uuid_str, *ptr;
-	uuid_le uuid;
-	int ret;
-
-	ptr = uuid_str = kstrndup(buf, count, GFP_KERNEL);
-
-	if (!uuid_str)
-		return -ENOMEM;
+	int start, ret;
 
-	ret = uuid_le_to_bin(uuid_str, &uuid);
-	if (ret) {
-		pr_err("mdev_start: UUID parse error  %s\n", buf);
-		goto start_error;
-	}
+	ret = kstrtoint(buf, 10, &start);
+	if (ret)
+		goto exit;
 
-	ret = mdev_device_start(uuid);
+	ret = mdev_device_start(dev, !!start);
 	if (ret == 0)
-		ret = count;
-
-start_error:
-	kfree(ptr);
+		ret = len;
+exit:
 	return ret;
 }
+static DEVICE_ATTR_WO(start);
 
-ssize_t mdev_stop_store(struct class *class, struct class_attribute *attr,
-			    const char *buf, size_t count)
-{
-	char *uuid_str, *ptr;
-	uuid_le uuid;
-	int ret;
-
-	ptr = uuid_str = kstrndup(buf, count, GFP_KERNEL);
-
-	if (!uuid_str)
-		return -ENOMEM;
-
-	ret = uuid_le_to_bin(uuid_str, &uuid);
-	if (ret) {
-		pr_err("mdev_stop: UUID parse error %s\n", buf);
-		goto stop_error;
-	}
-
-	ret = mdev_device_stop(uuid);
-	if (ret == 0)
-		ret = count;
-
-stop_error:
-	kfree(ptr);
-	return ret;
+static struct attribute *mdev_device_attrs[] = {
+	&dev_attr_start.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(mdev_device);
 
-}
+#define MDEV_CLASS_NAME		"mdev"
 
-struct class_attribute mdev_class_attrs[] = {
-	__ATTR_WO(mdev_start),
-	__ATTR_WO(mdev_stop),
-	__ATTR_NULL
+struct class mdev_class = {
+	.name		= MDEV_CLASS_NAME,
+	.dev_groups	= mdev_device_groups,
 };
 
 int mdev_create_sysfs_files(struct device *dev)
diff --git a/drivers/vfio/mdev/vfio_mpci.c b/drivers/vfio/mdev/vfio_mpci.c
index 9da94b7..88c0ba6 100644
--- a/drivers/vfio/mdev/vfio_mpci.c
+++ b/drivers/vfio/mdev/vfio_mpci.c
@@ -420,7 +420,7 @@  static int mdev_dev_mmio_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 		virtaddr = vma->vm_start;
 		req_size = vma->vm_end - vma->vm_start;
 
-		pdev = to_pci_dev(parent->dev);
+		pdev = to_pci_dev(parent->dev.parent);
 		index = VFIO_PCI_OFFSET_TO_INDEX(vma->vm_pgoff << PAGE_SHIFT);
 		pgoff = pci_resource_start(pdev, index) >> PAGE_SHIFT;
 	}
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index 0b41f30..42da41b 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -159,12 +159,10 @@  struct parent_ops {
  */
 
 struct parent_device {
-	struct device		*dev;
+	struct device dev;
 	const struct parent_ops	*ops;
 
 	/* internal */
-	struct kref		ref;
-	struct list_head	next;
 	struct list_head	mdev_list;
 	struct mutex		mdev_list_lock;
 	wait_queue_head_t	release_done;
@@ -214,7 +212,7 @@  extern struct bus_type mdev_bus_type;
 
 extern int  mdev_register_device(struct device *dev,
 				 const struct parent_ops *ops);
-extern void mdev_unregister_device(struct device *dev);
+extern void mdev_unregister_device(struct parent_device *parent);
 
 extern int  mdev_register_driver(struct mdev_driver *drv, struct module *owner);
 extern void mdev_unregister_driver(struct mdev_driver *drv);