qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] [RFC v6-based v1 1/5] mdev: create separate device for pare


From: Jike Song
Subject: [Qemu-devel] [RFC v6-based v1 1/5] mdev: create separate device for parent_device
Date: Tue, 16 Aug 2016 16:14:13 +0800

From: Xiao Guangrong <address@hidden>

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 <address@hidden>
---
 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 --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);
-- 
1.9.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]