[PATCH v1] vduse: Tie vduse mgmtdev and its device

[PATCH v1] vduse: Tie vduse mgmtdev and its device

[Parav Pandit via Virtualization]

vduse devices are not backed by any real devices such as PCI. Hence it
doesn't have any parent device linked to it.

Kernel driver model in [1] suggests to avoid an empty device
release callback.

Hence tie the mgmtdevice object's life cycle to an allocate dummy struct
device instead of static one.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/core-api/kobject.rst?h=v5.18-rc7#n284

Signed-off-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v0->v1:
- removed typo in device name of "-la"
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 60 ++++++++++++++++++------------
 1 file changed, 37 insertions(+), 23 deletions(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 776ad7496f53..3bc27de58f46 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1476,16 +1476,12 @@ static char *vduse_devnode(struct device *dev, umode_t *mode)
 	return kasprintf(GFP_KERNEL, "vduse/%s", dev_name(dev));
 }
 
-static void vduse_mgmtdev_release(struct device *dev)
-{
-}
-
-static struct device vduse_mgmtdev = {
-	.init_name = "vduse",
-	.release = vduse_mgmtdev_release,
+struct vduse_mgmt_dev {
+	struct vdpa_mgmt_dev mgmt_dev;
+	struct device dev;
 };
 
-static struct vdpa_mgmt_dev mgmt_dev;
+static struct vduse_mgmt_dev *vduse_mgmt;
 
 static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
 {
@@ -1510,7 +1506,7 @@ static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
 	}
 	set_dma_ops(&vdev->vdpa.dev, &vduse_dev_dma_ops);
 	vdev->vdpa.dma_dev = &vdev->vdpa.dev;
-	vdev->vdpa.mdev = &mgmt_dev;
+	vdev->vdpa.mdev = &vduse_mgmt->mgmt_dev;
 
 	return 0;
 }
@@ -1556,34 +1552,52 @@ static struct virtio_device_id id_table[] = {
 	{ 0 },
 };
 
-static struct vdpa_mgmt_dev mgmt_dev = {
-	.device = &vduse_mgmtdev,
-	.id_table = id_table,
-	.ops = &vdpa_dev_mgmtdev_ops,
-};
+static void vduse_mgmtdev_release(struct device *dev)
+{
+	struct vduse_mgmt_dev *mgmt_dev;
+
+	mgmt_dev = container_of(dev, struct vduse_mgmt_dev, dev);
+	kfree(mgmt_dev);
+}
 
 static int vduse_mgmtdev_init(void)
 {
 	int ret;
 
-	ret = device_register(&vduse_mgmtdev);
-	if (ret)
+	vduse_mgmt = kzalloc(sizeof(*vduse_mgmt), GFP_KERNEL);
+	if (!vduse_mgmt)
+		return -ENOMEM;
+
+	ret = dev_set_name(&vduse_mgmt->dev, "vduse");
+	if (ret) {
+		kfree(vduse_mgmt);
 		return ret;
+	}
 
-	ret = vdpa_mgmtdev_register(&mgmt_dev);
+	vduse_mgmt->dev.release = vduse_mgmtdev_release;
+
+	ret = device_register(&vduse_mgmt->dev);
 	if (ret)
-		goto err;
+		goto dev_reg_err;
 
-	return 0;
-err:
-	device_unregister(&vduse_mgmtdev);
+	vduse_mgmt->mgmt_dev.id_table = id_table;
+	vduse_mgmt->mgmt_dev.ops = &vdpa_dev_mgmtdev_ops;
+	vduse_mgmt->mgmt_dev.device = &vduse_mgmt->dev;
+	ret = vdpa_mgmtdev_register(&vduse_mgmt->mgmt_dev);
+	if (ret)
+		device_unregister(&vduse_mgmt->dev);
+
+	return ret;
+
+dev_reg_err:
+	put_device(&vduse_mgmt->dev);
 	return ret;
 }
 
 static void vduse_mgmtdev_exit(void)
 {
-	vdpa_mgmtdev_unregister(&mgmt_dev);
-	device_unregister(&vduse_mgmtdev);
+	vdpa_mgmtdev_unregister(&vduse_mgmt->mgmt_dev);
+	device_unregister(&vduse_mgmt->dev);
 }
 
 static int vduse_init(void)
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v1] vduse: Tie vduse mgmtdev and its device

[Jason Wang]

On Tue, Jun 14, 2022 at 3:52 AM Parav Pandit <parav@nvidia.com> wrote:
>
> vduse devices are not backed by any real devices such as PCI. Hence it
> doesn't have any parent device linked to it.
>
> Kernel driver model in [1] suggests to avoid an empty device
> release callback.
>
> Hence tie the mgmtdevice object's life cycle to an allocate dummy struct
> device instead of static one.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/core-api/kobject.rst?h=v5.18-rc7#n284
>
> Signed-off-by: Parav Pandit <parav@nvidia.com>

Acked-by: Jason Wang <jasowang@redhat.com>

> ---
> changelog:
> v0->v1:
> - removed typo in device name of "-la"
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 60 ++++++++++++++++++------------
>  1 file changed, 37 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 776ad7496f53..3bc27de58f46 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1476,16 +1476,12 @@ static char *vduse_devnode(struct device *dev, umode_t *mode)
>         return kasprintf(GFP_KERNEL, "vduse/%s", dev_name(dev));
>  }
>
> -static void vduse_mgmtdev_release(struct device *dev)
> -{
> -}
> -
> -static struct device vduse_mgmtdev = {
> -       .init_name = "vduse",
> -       .release = vduse_mgmtdev_release,
> +struct vduse_mgmt_dev {
> +       struct vdpa_mgmt_dev mgmt_dev;
> +       struct device dev;
>  };
>
> -static struct vdpa_mgmt_dev mgmt_dev;
> +static struct vduse_mgmt_dev *vduse_mgmt;
>
>  static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
>  {
> @@ -1510,7 +1506,7 @@ static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
>         }
>         set_dma_ops(&vdev->vdpa.dev, &vduse_dev_dma_ops);
>         vdev->vdpa.dma_dev = &vdev->vdpa.dev;
> -       vdev->vdpa.mdev = &mgmt_dev;
> +       vdev->vdpa.mdev = &vduse_mgmt->mgmt_dev;
>
>         return 0;
>  }
> @@ -1556,34 +1552,52 @@ static struct virtio_device_id id_table[] = {
>         { 0 },
>  };
>
> -static struct vdpa_mgmt_dev mgmt_dev = {
> -       .device = &vduse_mgmtdev,
> -       .id_table = id_table,
> -       .ops = &vdpa_dev_mgmtdev_ops,
> -};
> +static void vduse_mgmtdev_release(struct device *dev)
> +{
> +       struct vduse_mgmt_dev *mgmt_dev;
> +
> +       mgmt_dev = container_of(dev, struct vduse_mgmt_dev, dev);
> +       kfree(mgmt_dev);
> +}
>
>  static int vduse_mgmtdev_init(void)
>  {
>         int ret;
>
> -       ret = device_register(&vduse_mgmtdev);
> -       if (ret)
> +       vduse_mgmt = kzalloc(sizeof(*vduse_mgmt), GFP_KERNEL);
> +       if (!vduse_mgmt)
> +               return -ENOMEM;
> +
> +       ret = dev_set_name(&vduse_mgmt->dev, "vduse");
> +       if (ret) {
> +               kfree(vduse_mgmt);
>                 return ret;
> +       }
>
> -       ret = vdpa_mgmtdev_register(&mgmt_dev);
> +       vduse_mgmt->dev.release = vduse_mgmtdev_release;
> +
> +       ret = device_register(&vduse_mgmt->dev);
>         if (ret)
> -               goto err;
> +               goto dev_reg_err;
>
> -       return 0;
> -err:
> -       device_unregister(&vduse_mgmtdev);
> +       vduse_mgmt->mgmt_dev.id_table = id_table;
> +       vduse_mgmt->mgmt_dev.ops = &vdpa_dev_mgmtdev_ops;
> +       vduse_mgmt->mgmt_dev.device = &vduse_mgmt->dev;
> +       ret = vdpa_mgmtdev_register(&vduse_mgmt->mgmt_dev);
> +       if (ret)
> +               device_unregister(&vduse_mgmt->dev);
> +
> +       return ret;
> +
> +dev_reg_err:
> +       put_device(&vduse_mgmt->dev);
>         return ret;
>  }
>
>  static void vduse_mgmtdev_exit(void)
>  {
> -       vdpa_mgmtdev_unregister(&mgmt_dev);
> -       device_unregister(&vduse_mgmtdev);
> +       vdpa_mgmtdev_unregister(&vduse_mgmt->mgmt_dev);
> +       device_unregister(&vduse_mgmt->dev);
>  }
>
>  static int vduse_init(void)
> --
> 2.26.2
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization