wang.yi59 at zte.com.cn
2021-Dec-24 07:02 UTC
Re:[PATCH] vdpa: regist vhost-vdpa dev class
Hi Stefano, Thanks for your quick reply and review, :)> On Thu, Dec 23, 2021 at 03:31:45PM +0800, Yi Wang wrote: > >From: Zhang Min <zhang.min9 at zte.com.cn> > > > >Some applications like kata-containers need to acquire MAJOR/MINOR/DEVNAME > >for devInfo [1], so regist vhost-vdpa dev class to expose uevent. > > > >1. https://github.com/kata-containers/kata-containers/blob/main/src/runtime/virtcontainers/device/config/config.go > > > >Signed-off-by: Zhang Min <zhang.min9 at zte.com.cn> > >Signed-off-by: Yi Wang <wang.yi59 at zte.com.cn> > >--- > > drivers/vhost/vdpa.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > >diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > >index fb41db3da611..90fbad93e7a2 100644 > >--- a/drivers/vhost/vdpa.c > >+++ b/drivers/vhost/vdpa.c..> >+ vhost_vdpa_class = class_create(THIS_MODULE, "vhost-vdpa"); > >+ if (IS_ERR(vhost_vdpa_class)) { > >+ r = PTR_ERR(vhost_vdpa_class); > >+ pr_warn("vhost vdpa class create error %d, maybe mod reinserted\n", r); > ^ > double space. > > I'm not a native speaker, but I would rephrase the second part to "maybe > the module is already loaded" > > >+ vhost_vdpa_class = NULL; > >+ return r; > >+ } > >+ > > r = alloc_chrdev_region(&vhost_vdpa_major, 0, VHOST_VDPA_DEV_MAX, > > "vhost-vdpa"); > > if (r) > >@@ -1111,6 +1121,7 @@ static int __init vhost_vdpa_init(void) > > err_vdpa_register_driver: > > unregister_chrdev_region(vhost_vdpa_major, VHOST_VDPA_DEV_MAX); > > err_alloc_chrdev: > >+ class_destroy(vhost_vdpa_class); > > Should we set `vhost_vdpa_class` to NULL here? > > If yes, maybe better to add a new label, and a goto in the > `class_create` error handling.Yes, reset to NULL is unnecessary, and pr_warn will not be reached when module is already loaded, so it's no need too. There will be a v2 patch which simply return PTR_ERR(vhost_vdpa_class); Other suggestions are also accepted.> > return r; > > }