Re: [PATCH 1/2] vduse: Remove empty vduse_mgmtdev_release()

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, May 11, 2022 at 09:55:22PM +0800, Xie Yongji wrote:
> It's not recommended to provide an "empty" release function
> for the device object as Documentation/core-api/kobject.rst
> mentioned.

"it is a bug to have an empty release function" is more like it :)

> So let's allocate the device object dynamically
> to get rid of it.

Much better, but not quite there, see below for details.

> 
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 43 +++++++++++++++++-------------
>  1 file changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 160e40d03084..a8a5ebaefa10 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1475,15 +1475,6 @@ 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,
> -};
> -
>  static struct vdpa_mgmt_dev mgmt_dev;

Close.  This should be a pointer and the device structure within it
should control the lifecycle of that structure.  It should not be a
single static structure like this, that's very odd.

thanks,

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