Re: [RFC PATCH 02/24] vhost-vdpa: fix vqs leak in vhost_vdpa_open()

[Jason Wang]

On 2020/9/24 下午3:48, Eli Cohen wrote:
> On Thu, Sep 24, 2020 at 11:21:03AM +0800, Jason Wang wrote:
> > We need to free vqs during the err path after it has been allocated
> > since vhost won't do that for us.
> >
> > Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
> > ---
> >   drivers/vhost/vdpa.c | 11 ++++++++---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> > index 796fe979f997..9c641274b9f3 100644
> > --- a/drivers/vhost/vdpa.c
> > +++ b/drivers/vhost/vdpa.c
> > @@ -764,6 +764,12 @@ static void vhost_vdpa_free_domain(struct vhost_vdpa *v)
> >   	v->domain = NULL;
> >   }
> >   
> > +static void vhost_vdpa_cleanup(struct vhost_vdpa *v)
> > +{
> > +	vhost_dev_cleanup(&v->vdev);
> > +	kfree(v->vdev.vqs);
> > +}
> > +
> Wouldn't it be cleaner to call kfree(vqs) explicilty inside
> vhost_vdpa_open() in case of failure and keep the symetry of
> vhost_dev_init()/vhost_dev_cleanup()?


That's also fine.

See 
https://www.mail-archive.com/virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx/msg42558.html

I will use that for the next version.

Thanks.


> >   static int vhost_vdpa_open(struct inode *inode, struct file *filep)
> >   {
> >   	struct vhost_vdpa *v;
> > @@ -809,7 +815,7 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
> >   	return 0;
> >   
> >   err_init_iotlb:
> > -	vhost_dev_cleanup(&v->vdev);
> > +	vhost_vdpa_cleanup(v);
> >   err:
> >   	atomic_dec(&v->opened);
> >   	return r;
> > @@ -840,8 +846,7 @@ static int vhost_vdpa_release(struct inode *inode, struct file *filep)
> >   	vhost_vdpa_free_domain(v);
> >   	vhost_vdpa_config_put(v);
> >   	vhost_vdpa_clean_irq(v);
> > -	vhost_dev_cleanup(&v->vdev);
> > -	kfree(v->vdev.vqs);
> > +	vhost_vdpa_cleanup(v);
> >   	mutex_unlock(&d->mutex);
> >   
> >   	atomic_dec(&v->opened);
> > --
> > 2.20.1