Re: [PATCH v2 2/3] vdpa: Add a device object for vdpa management device

[Jason Wang <jasowang@xxxxxxxxxx>]

在 2022/5/18 07:03, Parav Pandit 写道:
> > > And regarding vduse_dev_release() and existing empty release function,
> > they can be dynamically allocated.
> > > This is because they are really the struct device.
> > I do not understand this statement, sorry.
> It was bad sentence, my bad.
>
> What I wanted to say is, probably better explained with real patch below.
> In context of [1], struct vduse_mgmtdev empty release function can be avoided by below inline patch [2].
>
> [1]https://www.spinics.net/lists/linux-virtualization/msg56371.html
>
> [2] patch:


Ok, if we go this way (looks more like mdev_parent). I think we need at 
least rename the vdpa_mgmt_device, maybe vdpa_parent (like mdev_parent)?

Thanks


>  From f87d557fe939a1632473dd11526a87301adbab8d Mon Sep 17 00:00:00 2001
> From: Parav Pandit<parav@xxxxxxxxxx>
> Date: Wed, 18 May 2022 01:22:21 +0300
> Subject: [PATCH] vduse: Tie vduse mgmtdev and its device
>
> vduse management device is not backed by any real device such as PCI. Hence it
> doesn't have any parent device linked to it.
>
> Kernel driver model in [1] suggests avoiding an empty device release callback.
>
> Hence tie the mgmtdevice object's life cycle to an allocate dummy struct
> device instead of having it static.
>
> [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@xxxxxxxxxx>
> ---
>   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 f85d1a08ed87..ebe272575fb8 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1475,16 +1475,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)
>   {
> @@ -1509,7 +1505,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;
>   }
> @@ -1555,34 +1551,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-la");
> +	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@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization