Re: [PATCH 2/3] virtio: use put_device instead of kfree

[weiping zhang]

On Tue, Dec 12, 2017 at 11:17:42AM +0100, Cornelia Huck wrote:
> On Mon, 11 Dec 2017 23:55:26 +0800
> weiping zhang  wrote:
> 
> > don't free vp_vdev until vp_vdev.dev.release be called.
> 
> Same comment as for the virtio_pci patch.
OK, I'll add it at V2.
> 
> > 
> > Signed-off-by: weiping zhang 
> > ---
> >  drivers/misc/mic/vop/vop_main.c | 16 +---
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/misc/mic/vop/vop_main.c 
> > b/drivers/misc/mic/vop/vop_main.c
> > index a341938..8c716a0 100644
> > --- a/drivers/misc/mic/vop/vop_main.c
> > +++ b/drivers/misc/mic/vop/vop_main.c
> > @@ -452,10 +452,12 @@ static irqreturn_t vop_virtio_intr_handler(int irq, 
> > void *data)
> >  
> >  static void vop_virtio_release_dev(struct device *_d)
> >  {
> > -   /*
> > -* No need for a release method similar to virtio PCI.
> > -* Provide an empty one to avoid getting a warning from core.
> > -*/
> > +   struct virtio_device *vdev =
> > +   container_of(_d, struct virtio_device, dev);
> > +   struct _vop_vdev *vop_vdev =
> > +   container_of(vdev, struct _vop_vdev, vdev);
> > +
> > +   kfree(vop_vdev);
> >  }
> >  
> >  /*
> > @@ -501,7 +503,9 @@ static int _vop_add_device(struct mic_device_desc 
> > __iomem *d,
> > dev_err(_vop_dev(vdev),
> > "Failed to register vop device %u type %u\n",
> > offset, type);
> > -   goto free_irq;
> > +   vpdev->hw_ops->free_irq(vpdev, vdev->virtio_cookie, vdev);
> > +   put_device(>vdev.dev);
> > +   return ret;
> > }
> > writeq((u64)vdev, >dc->vdev);
> > dev_dbg(_vop_dev(vdev), "%s: registered vop device %u type %u vdev 
> > %p\n",
> > @@ -509,8 +513,6 @@ static int _vop_add_device(struct mic_device_desc 
> > __iomem *d,
> >  
> > return 0;
> >  
> > -free_irq:
> > -   vpdev->hw_ops->free_irq(vpdev, vdev->virtio_cookie, vdev);
> >  kfree:
> > kfree(vdev);
> > return ret;
> 
> BTW, the kfree(vdev) in _vop_remove_device() is also wrong (needs to be
> a put_device() as well).
thanks for your comments, I'll fix it at v2.

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

Re: [PATCH 2/3] virtio: use put_device instead of kfree

[Cornelia Huck]

On Mon, 11 Dec 2017 23:55:26 +0800
weiping zhang  wrote:

> don't free vp_vdev until vp_vdev.dev.release be called.

Same comment as for the virtio_pci patch.

> 
> Signed-off-by: weiping zhang 
> ---
>  drivers/misc/mic/vop/vop_main.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/misc/mic/vop/vop_main.c b/drivers/misc/mic/vop/vop_main.c
> index a341938..8c716a0 100644
> --- a/drivers/misc/mic/vop/vop_main.c
> +++ b/drivers/misc/mic/vop/vop_main.c
> @@ -452,10 +452,12 @@ static irqreturn_t vop_virtio_intr_handler(int irq, 
> void *data)
>  
>  static void vop_virtio_release_dev(struct device *_d)
>  {
> - /*
> -  * No need for a release method similar to virtio PCI.
> -  * Provide an empty one to avoid getting a warning from core.
> -  */
> + struct virtio_device *vdev =
> + container_of(_d, struct virtio_device, dev);
> + struct _vop_vdev *vop_vdev =
> + container_of(vdev, struct _vop_vdev, vdev);
> +
> + kfree(vop_vdev);
>  }
>  
>  /*
> @@ -501,7 +503,9 @@ static int _vop_add_device(struct mic_device_desc __iomem 
> *d,
>   dev_err(_vop_dev(vdev),
>   "Failed to register vop device %u type %u\n",
>   offset, type);
> - goto free_irq;
> + vpdev->hw_ops->free_irq(vpdev, vdev->virtio_cookie, vdev);
> + put_device(>vdev.dev);
> + return ret;
>   }
>   writeq((u64)vdev, >dc->vdev);
>   dev_dbg(_vop_dev(vdev), "%s: registered vop device %u type %u vdev 
> %p\n",
> @@ -509,8 +513,6 @@ static int _vop_add_device(struct mic_device_desc __iomem 
> *d,
>  
>   return 0;
>  
> -free_irq:
> - vpdev->hw_ops->free_irq(vpdev, vdev->virtio_cookie, vdev);
>  kfree:
>   kfree(vdev);
>   return ret;

BTW, the kfree(vdev) in _vop_remove_device() is also wrong (needs to be
a put_device() as well).
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 2/3] virtio: use put_device instead of kfree

[weiping zhang]

don't free vp_vdev until vp_vdev.dev.release be called.

Signed-off-by: weiping zhang 
---
 drivers/misc/mic/vop/vop_main.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/mic/vop/vop_main.c b/drivers/misc/mic/vop/vop_main.c
index a341938..8c716a0 100644
--- a/drivers/misc/mic/vop/vop_main.c
+++ b/drivers/misc/mic/vop/vop_main.c
@@ -452,10 +452,12 @@ static irqreturn_t vop_virtio_intr_handler(int irq, void 
*data)
 
 static void vop_virtio_release_dev(struct device *_d)
 {
-   /*
-* No need for a release method similar to virtio PCI.
-* Provide an empty one to avoid getting a warning from core.
-*/
+   struct virtio_device *vdev =
+   container_of(_d, struct virtio_device, dev);
+   struct _vop_vdev *vop_vdev =
+   container_of(vdev, struct _vop_vdev, vdev);
+
+   kfree(vop_vdev);
 }
 
 /*
@@ -501,7 +503,9 @@ static int _vop_add_device(struct mic_device_desc __iomem 
*d,
dev_err(_vop_dev(vdev),
"Failed to register vop device %u type %u\n",
offset, type);
-   goto free_irq;
+   vpdev->hw_ops->free_irq(vpdev, vdev->virtio_cookie, vdev);
+   put_device(>vdev.dev);
+   return ret;
}
writeq((u64)vdev, >dc->vdev);
dev_dbg(_vop_dev(vdev), "%s: registered vop device %u type %u vdev 
%p\n",
@@ -509,8 +513,6 @@ static int _vop_add_device(struct mic_device_desc __iomem 
*d,
 
return 0;
 
-free_irq:
-   vpdev->hw_ops->free_irq(vpdev, vdev->virtio_cookie, vdev);
 kfree:
kfree(vdev);
return ret;
-- 
2.9.4

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