Re: [PATCH 02/13] vfio/mdev: embedd struct mdev_parent in the parent data structure

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

 





On 6/14/2022 10:24 AM, Christoph Hellwig wrote:
<snip>


  /*
- * mdev_register_device : Register a device
+ * mdev_register_parent: Register a device as parent for mdevs
+ * @parent: parent structure registered
   * @dev: device structure representing parent device.
   * @mdev_driver: Device driver to bind to the newly created mdev
   *
- * Add device to list of registered parent devices.
   * Returns a negative value on error, otherwise 0.
   */
-int mdev_register_device(struct device *dev, struct mdev_driver *mdev_driver)
+int mdev_register_parent(struct mdev_parent *parent, struct device *dev,
+		struct mdev_driver *mdev_driver)
  {
-	int ret;
-	struct mdev_parent *parent;
  	char *env_string = "MDEV_STATE=registered";
  	char *envp[] = { env_string, NULL };
+	int ret;
/* check for mandatory ops */
  	if (!mdev_driver->supported_type_groups)
  		return -EINVAL;
- dev = get_device(dev);
-	if (!dev)
-		return -EINVAL;
-

Hold the reference for device here once rather than helding multiple times while adding sysfs for each mdev_types below. See more below

-	mutex_lock(&parent_list_lock);
-
-	/* Check for duplicate */
-	parent = __find_parent_device(dev);
-	if (parent) {
-		parent = NULL;
-		ret = -EEXIST;
-		goto add_dev_err;
-	}
-
-	parent = kzalloc(sizeof(*parent), GFP_KERNEL);
-	if (!parent) {
-		ret = -ENOMEM;
-		goto add_dev_err;
-	}
-
-	kref_init(&parent->ref);
+	memset(parent, 0, sizeof(*parent));
  	init_rwsem(&parent->unreg_sem);
-
  	parent->dev = dev;
  	parent->mdev_driver = mdev_driver;
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;
-		}
+		if (!mdev_bus_compat_class)
+			return -ENOMEM;
  	}
ret = parent_create_sysfs_files(parent);
  	if (ret)
-		goto add_dev_err;
+		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");
- list_add(&parent->next, &parent_list);
-	mutex_unlock(&parent_list_lock);
-
  	dev_info(dev, "MDEV: Registered\n");
  	kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
-
  	return 0;
-
-add_dev_err:
-	mutex_unlock(&parent_list_lock);
-	if (parent)
-		mdev_put_parent(parent);
-	else
-		put_device(dev);
-	return ret;
  }
-EXPORT_SYMBOL(mdev_register_device);
+EXPORT_SYMBOL(mdev_register_parent);
/*
- * mdev_unregister_device : Unregister a parent device
- * @dev: device structure representing parent device.
- *
- * Remove device from list of registered parent devices. Give a chance to free
- * existing mediated devices for given device.
+ * mdev_unregister_parent : Unregister a parent device
+ * @parent: parent structure to unregister
   */
-
-void mdev_unregister_device(struct device *dev)
+void mdev_unregister_parent(struct mdev_parent *parent)
  {
-	struct mdev_parent *parent;
  	char *env_string = "MDEV_STATE=unregistered";
  	char *envp[] = { env_string, NULL };
- mutex_lock(&parent_list_lock);
-	parent = __find_parent_device(dev);
-
-	if (!parent) {
-		mutex_unlock(&parent_list_lock);
-		return;
-	}
-	dev_info(dev, "MDEV: Unregistering\n");
-
-	list_del(&parent->next);
-	mutex_unlock(&parent_list_lock);
+	dev_info(parent->dev, "MDEV: Unregistering\n");
down_write(&parent->unreg_sem);
-
-	class_compat_remove_link(mdev_bus_compat_class, dev, NULL);
-
-	device_for_each_child(dev, NULL, mdev_device_remove_cb);
-
+	class_compat_remove_link(mdev_bus_compat_class, parent->dev, NULL);
+	device_for_each_child(parent->dev, NULL, mdev_device_remove_cb);
  	parent_remove_sysfs_files(parent);
  	up_write(&parent->unreg_sem);
- mdev_put_parent(parent);
-
-	/* We still have the caller's reference to use for the uevent */
-	kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
+	kobject_uevent_env(&parent->dev->kobj, KOBJ_CHANGE, envp);
  }
-EXPORT_SYMBOL(mdev_unregister_device);
+EXPORT_SYMBOL(mdev_unregister_parent);
static void mdev_device_release(struct device *dev)
  {
diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h
index 7c9fc79f3d838..297f911fdc890 100644
--- a/drivers/vfio/mdev/mdev_private.h
+++ b/drivers/vfio/mdev/mdev_private.h
@@ -13,17 +13,6 @@
  int  mdev_bus_register(void);
  void mdev_bus_unregister(void);
-struct mdev_parent {
-	struct device *dev;
-	struct mdev_driver *mdev_driver;
-	struct kref ref;
-	struct list_head next;
-	struct kset *mdev_types_kset;
-	struct list_head type_list;
-	/* Synchronize device creation/removal with parent unregistration */
-	struct rw_semaphore unreg_sem;
-};
-
  struct mdev_type {
  	struct kobject kobj;
  	struct kobject *devices_kobj;
@@ -48,16 +37,4 @@ void mdev_remove_sysfs_files(struct mdev_device *mdev);
  int mdev_device_create(struct mdev_type *kobj, const guid_t *uuid);
  int  mdev_device_remove(struct mdev_device *dev);
-void mdev_release_parent(struct kref *kref);
-
-static inline void mdev_get_parent(struct mdev_parent *parent)
-{
-	kref_get(&parent->ref);
-}
-
-static inline void mdev_put_parent(struct mdev_parent *parent)
-{
-	kref_put(&parent->ref, mdev_release_parent);
-}
-
  #endif /* MDEV_PRIVATE_H */
diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c
index 4bfbf49aaa66a..b71ffc5594870 100644
--- a/drivers/vfio/mdev/mdev_sysfs.c
+++ b/drivers/vfio/mdev/mdev_sysfs.c
@@ -81,7 +81,7 @@ static void mdev_type_release(struct kobject *kobj)
pr_debug("Releasing group %s\n", kobj->name);
  	/* Pairs with the get in add_mdev_supported_type() */
-	mdev_put_parent(type->parent);
+	put_device(type->parent->dev);
  	kfree(type);
  }
@@ -110,7 +110,7 @@ static struct mdev_type *add_mdev_supported_type(struct mdev_parent *parent,
  	type->kobj.kset = parent->mdev_types_kset;
  	type->parent = parent;
  	/* Pairs with the put in mdev_type_release() */
-	mdev_get_parent(parent);
+	get_device(parent->dev);
  	type->type_group_id = type_group_id;


Here device reference is held and release multiple times for each mdev_type. It should be held once from mdev_register_parent() and released from mdev_unregister_parent().

Thanks,
Kirti



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux