* [PATCH 0/2] delete virtio queues in vhost-user-blk-unrealize @ 2020-02-13 1:28 pannengyuan 2020-02-13 1:28 ` [PATCH 1/2] vhost-user-blk: delete virtioqueues in unrealize to fix memleaks pannengyuan 2020-02-13 1:28 ` [PATCH 2/2] vhost-use-blk: convert to new virtio_delete_queue pannengyuan 0 siblings, 2 replies; 7+ messages in thread From: pannengyuan @ 2020-02-13 1:28 UTC (permalink / raw) To: mst, kwolf, mreitz Cc: Pan Nengyuan, qemu-devel, qemu-block, zhang.zhanghailiang From: Pan Nengyuan <pannengyuan@huawei.com> This series patch fix memleaks when detaching vhost-user-blk device. 1. use old virtio_del_queue to fix memleaks, it's easier for stable branches to merge. As the discussion in https://lists.nongnu.org/archive/html/qemu-devel/2020-01/msg02903.html 2. convert virtio_del_queue to the new one(virtio_delete_queue). Pan Nengyuan (2): vhost-user-blk: delete virtioqueues in unrealize to fix memleaks vhost-use-blk: convert to new virtio_delete_queue hw/block/vhost-user-blk.c | 15 +++++++++++++-- include/hw/virtio/vhost-user-blk.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-) -- 2.21.0.windows.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] vhost-user-blk: delete virtioqueues in unrealize to fix memleaks 2020-02-13 1:28 [PATCH 0/2] delete virtio queues in vhost-user-blk-unrealize pannengyuan @ 2020-02-13 1:28 ` pannengyuan 2020-02-21 11:32 ` Stefan Hajnoczi 2020-02-13 1:28 ` [PATCH 2/2] vhost-use-blk: convert to new virtio_delete_queue pannengyuan 1 sibling, 1 reply; 7+ messages in thread From: pannengyuan @ 2020-02-13 1:28 UTC (permalink / raw) To: mst, kwolf, mreitz Cc: Euler Robot, Pan Nengyuan, qemu-devel, qemu-block, zhang.zhanghailiang From: Pan Nengyuan <pannengyuan@huawei.com> virtio queues forgot to delete in unrealize, and aslo error path in realize, this patch fix these memleaks, the leak stack is as follow: Direct leak of 114688 byte(s) in 16 object(s) allocated from: #0 0x7f24024fdbf0 in calloc (/lib64/libasan.so.3+0xcabf0) #1 0x7f2401642015 in g_malloc0 (/lib64/libglib-2.0.so.0+0x50015) #2 0x55ad175a6447 in virtio_add_queue /mnt/sdb/qemu/hw/virtio/virtio.c:2327 #3 0x55ad17570cf9 in vhost_user_blk_device_realize /mnt/sdb/qemu/hw/block/vhost-user-blk.c:419 #4 0x55ad175a3707 in virtio_device_realize /mnt/sdb/qemu/hw/virtio/virtio.c:3509 #5 0x55ad176ad0d1 in device_set_realized /mnt/sdb/qemu/hw/core/qdev.c:876 #6 0x55ad1781ff9d in property_set_bool /mnt/sdb/qemu/qom/object.c:2080 #7 0x55ad178245ae in object_property_set_qobject /mnt/sdb/qemu/qom/qom-qobject.c:26 #8 0x55ad17821eb4 in object_property_set_bool /mnt/sdb/qemu/qom/object.c:1338 #9 0x55ad177aeed7 in virtio_pci_realize /mnt/sdb/qemu/hw/virtio/virtio-pci.c:1801 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> --- hw/block/vhost-user-blk.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index d8c459c575..2eba8b9db0 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -460,6 +460,9 @@ reconnect: virtio_err: g_free(s->vqs); g_free(s->inflight); + for (i = 0; i < s->num_queues; i++) { + virtio_del_queue(vdev, i); + } virtio_cleanup(vdev); vhost_user_cleanup(&s->vhost_user); } @@ -468,6 +471,7 @@ static void vhost_user_blk_device_unrealize(DeviceState *dev, Error **errp) { VirtIODevice *vdev = VIRTIO_DEVICE(dev); VHostUserBlk *s = VHOST_USER_BLK(dev); + int i; virtio_set_status(vdev, 0); qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, NULL, @@ -476,6 +480,10 @@ static void vhost_user_blk_device_unrealize(DeviceState *dev, Error **errp) vhost_dev_free_inflight(s->inflight); g_free(s->vqs); g_free(s->inflight); + + for (i = 0; i < s->num_queues; i++) { + virtio_del_queue(vdev, i); + } virtio_cleanup(vdev); vhost_user_cleanup(&s->vhost_user); } -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] vhost-user-blk: delete virtioqueues in unrealize to fix memleaks 2020-02-13 1:28 ` [PATCH 1/2] vhost-user-blk: delete virtioqueues in unrealize to fix memleaks pannengyuan @ 2020-02-21 11:32 ` Stefan Hajnoczi 0 siblings, 0 replies; 7+ messages in thread From: Stefan Hajnoczi @ 2020-02-21 11:32 UTC (permalink / raw) To: pannengyuan Cc: kwolf, zhang.zhanghailiang, qemu-block, mst, qemu-devel, mreitz, Euler Robot [-- Attachment #1: Type: text/plain, Size: 1423 bytes --] On Thu, Feb 13, 2020 at 09:28:06AM +0800, pannengyuan@huawei.com wrote: > From: Pan Nengyuan <pannengyuan@huawei.com> > > virtio queues forgot to delete in unrealize, and aslo error path in > realize, this patch fix these memleaks, the leak stack is as follow: > > Direct leak of 114688 byte(s) in 16 object(s) allocated from: > #0 0x7f24024fdbf0 in calloc (/lib64/libasan.so.3+0xcabf0) > #1 0x7f2401642015 in g_malloc0 (/lib64/libglib-2.0.so.0+0x50015) > #2 0x55ad175a6447 in virtio_add_queue /mnt/sdb/qemu/hw/virtio/virtio.c:2327 > #3 0x55ad17570cf9 in vhost_user_blk_device_realize /mnt/sdb/qemu/hw/block/vhost-user-blk.c:419 > #4 0x55ad175a3707 in virtio_device_realize /mnt/sdb/qemu/hw/virtio/virtio.c:3509 > #5 0x55ad176ad0d1 in device_set_realized /mnt/sdb/qemu/hw/core/qdev.c:876 > #6 0x55ad1781ff9d in property_set_bool /mnt/sdb/qemu/qom/object.c:2080 > #7 0x55ad178245ae in object_property_set_qobject /mnt/sdb/qemu/qom/qom-qobject.c:26 > #8 0x55ad17821eb4 in object_property_set_bool /mnt/sdb/qemu/qom/object.c:1338 > #9 0x55ad177aeed7 in virtio_pci_realize /mnt/sdb/qemu/hw/virtio/virtio-pci.c:1801 > > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> > --- > hw/block/vhost-user-blk.c | 8 ++++++++ > 1 file changed, 8 insertions(+) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] vhost-use-blk: convert to new virtio_delete_queue 2020-02-13 1:28 [PATCH 0/2] delete virtio queues in vhost-user-blk-unrealize pannengyuan 2020-02-13 1:28 ` [PATCH 1/2] vhost-user-blk: delete virtioqueues in unrealize to fix memleaks pannengyuan @ 2020-02-13 1:28 ` pannengyuan 2020-02-21 11:31 ` Stefan Hajnoczi 2020-02-25 13:31 ` Michael S. Tsirkin 1 sibling, 2 replies; 7+ messages in thread From: pannengyuan @ 2020-02-13 1:28 UTC (permalink / raw) To: mst, kwolf, mreitz Cc: Pan Nengyuan, qemu-devel, qemu-block, zhang.zhanghailiang From: Pan Nengyuan <pannengyuan@huawei.com> use the new virtio_delete_queue function to cleanup. Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> --- hw/block/vhost-user-blk.c | 11 +++++++---- include/hw/virtio/vhost-user-blk.h | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index 2eba8b9db0..ed6a5cc03b 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -420,9 +420,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, sizeof(struct virtio_blk_config)); + s->virtqs = g_new0(VirtQueue *, s->num_queues); for (i = 0; i < s->num_queues; i++) { - virtio_add_queue(vdev, s->queue_size, - vhost_user_blk_handle_output); + s->virtqs[i] = virtio_add_queue(vdev, s->queue_size, + vhost_user_blk_handle_output); } s->inflight = g_new0(struct vhost_inflight, 1); @@ -461,8 +462,9 @@ virtio_err: g_free(s->vqs); g_free(s->inflight); for (i = 0; i < s->num_queues; i++) { - virtio_del_queue(vdev, i); + virtio_delete_queue(s->virtqs[i]); } + g_free(s->virtqs); virtio_cleanup(vdev); vhost_user_cleanup(&s->vhost_user); } @@ -482,8 +484,9 @@ static void vhost_user_blk_device_unrealize(DeviceState *dev, Error **errp) g_free(s->inflight); for (i = 0; i < s->num_queues; i++) { - virtio_del_queue(vdev, i); + virtio_delete_queue(s->virtqs[i]); } + g_free(s->virtqs); virtio_cleanup(vdev); vhost_user_cleanup(&s->vhost_user); } diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h index 108bfadeeb..f68911f6f0 100644 --- a/include/hw/virtio/vhost-user-blk.h +++ b/include/hw/virtio/vhost-user-blk.h @@ -37,6 +37,7 @@ typedef struct VHostUserBlk { struct vhost_inflight *inflight; VhostUserState vhost_user; struct vhost_virtqueue *vqs; + VirtQueue **virtqs; guint watch; bool connected; } VHostUserBlk; -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] vhost-use-blk: convert to new virtio_delete_queue 2020-02-13 1:28 ` [PATCH 2/2] vhost-use-blk: convert to new virtio_delete_queue pannengyuan @ 2020-02-21 11:31 ` Stefan Hajnoczi 2020-02-22 7:56 ` Pan Nengyuan 2020-02-25 13:31 ` Michael S. Tsirkin 1 sibling, 1 reply; 7+ messages in thread From: Stefan Hajnoczi @ 2020-02-21 11:31 UTC (permalink / raw) To: pannengyuan Cc: kwolf, zhang.zhanghailiang, qemu-block, mst, qemu-devel, mreitz [-- Attachment #1: Type: text/plain, Size: 1203 bytes --] On Thu, Feb 13, 2020 at 09:28:07AM +0800, pannengyuan@huawei.com wrote: > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c > index 2eba8b9db0..ed6a5cc03b 100644 > --- a/hw/block/vhost-user-blk.c > +++ b/hw/block/vhost-user-blk.c > @@ -420,9 +420,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) > virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, > sizeof(struct virtio_blk_config)); > > + s->virtqs = g_new0(VirtQueue *, s->num_queues); Minor point, up to you if you want to change it: the array is fully initialized by the for loop in the next line. There is no need to clear the memory first: s/g_new0/g_new/ > diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h > index 108bfadeeb..f68911f6f0 100644 > --- a/include/hw/virtio/vhost-user-blk.h > +++ b/include/hw/virtio/vhost-user-blk.h > @@ -37,6 +37,7 @@ typedef struct VHostUserBlk { > struct vhost_inflight *inflight; > VhostUserState vhost_user; > struct vhost_virtqueue *vqs; > + VirtQueue **virtqs; Both vqs and virtqs exist and are easily confused. Please rename vqs to vhost_vqs. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] vhost-use-blk: convert to new virtio_delete_queue 2020-02-21 11:31 ` Stefan Hajnoczi @ 2020-02-22 7:56 ` Pan Nengyuan 0 siblings, 0 replies; 7+ messages in thread From: Pan Nengyuan @ 2020-02-22 7:56 UTC (permalink / raw) To: Stefan Hajnoczi Cc: kwolf, zhang.zhanghailiang, qemu-block, mst, qemu-devel, mreitz On 2/21/2020 7:31 PM, Stefan Hajnoczi wrote: > On Thu, Feb 13, 2020 at 09:28:07AM +0800, pannengyuan@huawei.com wrote: >> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c >> index 2eba8b9db0..ed6a5cc03b 100644 >> --- a/hw/block/vhost-user-blk.c >> +++ b/hw/block/vhost-user-blk.c >> @@ -420,9 +420,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) >> virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, >> sizeof(struct virtio_blk_config)); >> >> + s->virtqs = g_new0(VirtQueue *, s->num_queues); > > Minor point, up to you if you want to change it: the array is fully > initialized by the for loop in the next line. There is no need to clear > the memory first: > > s/g_new0/g_new/ OK, it's fine, I will change it. Thanks. > >> diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h >> index 108bfadeeb..f68911f6f0 100644 >> --- a/include/hw/virtio/vhost-user-blk.h >> +++ b/include/hw/virtio/vhost-user-blk.h >> @@ -37,6 +37,7 @@ typedef struct VHostUserBlk { >> struct vhost_inflight *inflight; >> VhostUserState vhost_user; >> struct vhost_virtqueue *vqs; >> + VirtQueue **virtqs; > > Both vqs and virtqs exist and are easily confused. Please rename vqs to > vhost_vqs. OK, I will do it. Thanks. > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] vhost-use-blk: convert to new virtio_delete_queue 2020-02-13 1:28 ` [PATCH 2/2] vhost-use-blk: convert to new virtio_delete_queue pannengyuan 2020-02-21 11:31 ` Stefan Hajnoczi @ 2020-02-25 13:31 ` Michael S. Tsirkin 1 sibling, 0 replies; 7+ messages in thread From: Michael S. Tsirkin @ 2020-02-25 13:31 UTC (permalink / raw) To: pannengyuan; +Cc: kwolf, zhang.zhanghailiang, qemu-devel, qemu-block, mreitz On Thu, Feb 13, 2020 at 09:28:07AM +0800, pannengyuan@huawei.com wrote: > From: Pan Nengyuan <pannengyuan@huawei.com> > > use the new virtio_delete_queue function to cleanup. > > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> typo in subject use-blk->user-blk > --- > hw/block/vhost-user-blk.c | 11 +++++++---- > include/hw/virtio/vhost-user-blk.h | 1 + > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c > index 2eba8b9db0..ed6a5cc03b 100644 > --- a/hw/block/vhost-user-blk.c > +++ b/hw/block/vhost-user-blk.c > @@ -420,9 +420,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) > virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, > sizeof(struct virtio_blk_config)); > > + s->virtqs = g_new0(VirtQueue *, s->num_queues); > for (i = 0; i < s->num_queues; i++) { > - virtio_add_queue(vdev, s->queue_size, > - vhost_user_blk_handle_output); > + s->virtqs[i] = virtio_add_queue(vdev, s->queue_size, > + vhost_user_blk_handle_output); > } > > s->inflight = g_new0(struct vhost_inflight, 1); > @@ -461,8 +462,9 @@ virtio_err: > g_free(s->vqs); > g_free(s->inflight); > for (i = 0; i < s->num_queues; i++) { > - virtio_del_queue(vdev, i); > + virtio_delete_queue(s->virtqs[i]); > } > + g_free(s->virtqs); > virtio_cleanup(vdev); > vhost_user_cleanup(&s->vhost_user); > } > @@ -482,8 +484,9 @@ static void vhost_user_blk_device_unrealize(DeviceState *dev, Error **errp) > g_free(s->inflight); > > for (i = 0; i < s->num_queues; i++) { > - virtio_del_queue(vdev, i); > + virtio_delete_queue(s->virtqs[i]); > } > + g_free(s->virtqs); > virtio_cleanup(vdev); > vhost_user_cleanup(&s->vhost_user); > } > diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h > index 108bfadeeb..f68911f6f0 100644 > --- a/include/hw/virtio/vhost-user-blk.h > +++ b/include/hw/virtio/vhost-user-blk.h > @@ -37,6 +37,7 @@ typedef struct VHostUserBlk { > struct vhost_inflight *inflight; > VhostUserState vhost_user; > struct vhost_virtqueue *vqs; > + VirtQueue **virtqs; > guint watch; > bool connected; > } VHostUserBlk; > -- > 2.21.0.windows.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-25 13:51 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-13 1:28 [PATCH 0/2] delete virtio queues in vhost-user-blk-unrealize pannengyuan 2020-02-13 1:28 ` [PATCH 1/2] vhost-user-blk: delete virtioqueues in unrealize to fix memleaks pannengyuan 2020-02-21 11:32 ` Stefan Hajnoczi 2020-02-13 1:28 ` [PATCH 2/2] vhost-use-blk: convert to new virtio_delete_queue pannengyuan 2020-02-21 11:31 ` Stefan Hajnoczi 2020-02-22 7:56 ` Pan Nengyuan 2020-02-25 13:31 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).