* [PATCH V2] vhost-user-test: fix a memory leak @ 2019-12-20 1:26 pannengyuan 2020-01-10 14:07 ` Thomas Huth 0 siblings, 1 reply; 8+ messages in thread From: pannengyuan @ 2019-12-20 1:26 UTC (permalink / raw) To: thuth, lvivier, pbonzini; +Cc: Pan Nengyuan, zhang.zhanghailiang, qemu-devel From: Pan Nengyuan <pannengyuan@huawei.com> Spotted by ASAN. Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> --- Changes V2 to V1: - use a "goto cleanup", instead of duplicating the "free" functions. - free "dest_cmdline" at the end. --- tests/vhost-user-test.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c index 91ea373..dcb8617 100644 --- a/tests/vhost-user-test.c +++ b/tests/vhost-user-test.c @@ -717,7 +717,7 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc) guint64 size; if (!wait_for_fds(s)) { - return; + goto cleanup; } size = get_log_size(s); @@ -776,8 +776,11 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc) g_source_unref(source); qtest_quit(to); + + cleanup: test_server_free(dest); g_free(uri); + g_string_free(dest_cmdline, true); } static void wait_for_rings_started(TestServer *s, size_t count) -- 2.7.2.windows.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2] vhost-user-test: fix a memory leak 2019-12-20 1:26 [PATCH V2] vhost-user-test: fix a memory leak pannengyuan @ 2020-01-10 14:07 ` Thomas Huth 2020-01-12 10:39 ` Thomas Huth 0 siblings, 1 reply; 8+ messages in thread From: Thomas Huth @ 2020-01-10 14:07 UTC (permalink / raw) To: pannengyuan, lvivier, pbonzini; +Cc: zhang.zhanghailiang, qemu-devel On 20/12/2019 02.26, pannengyuan@huawei.com wrote: > From: Pan Nengyuan <pannengyuan@huawei.com> > > Spotted by ASAN. > > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> > --- > Changes V2 to V1: > - use a "goto cleanup", instead of duplicating the "free" functions. > - free "dest_cmdline" at the end. > --- > tests/vhost-user-test.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c > index 91ea373..dcb8617 100644 > --- a/tests/vhost-user-test.c > +++ b/tests/vhost-user-test.c > @@ -717,7 +717,7 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc) > guint64 size; > > if (!wait_for_fds(s)) { > - return; > + goto cleanup; > } > > size = get_log_size(s); > @@ -776,8 +776,11 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc) > g_source_unref(source); > > qtest_quit(to); > + > + cleanup: > test_server_free(dest); > g_free(uri); > + g_string_free(dest_cmdline, true); > } > > static void wait_for_rings_started(TestServer *s, size_t count) > Reviewed-by: Thomas Huth <thuth@redhat.com> ... and picked up to my qtest-next tree. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] vhost-user-test: fix a memory leak 2020-01-10 14:07 ` Thomas Huth @ 2020-01-12 10:39 ` Thomas Huth 2020-01-13 2:32 ` Pan Nengyuan 0 siblings, 1 reply; 8+ messages in thread From: Thomas Huth @ 2020-01-12 10:39 UTC (permalink / raw) To: pannengyuan, lvivier, pbonzini; +Cc: zhang.zhanghailiang, qemu-devel On 10/01/2020 15.07, Thomas Huth wrote: > On 20/12/2019 02.26, pannengyuan@huawei.com wrote: >> From: Pan Nengyuan <pannengyuan@huawei.com> >> >> Spotted by ASAN. >> >> Reported-by: Euler Robot <euler.robot@huawei.com> >> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> >> --- >> Changes V2 to V1: >> - use a "goto cleanup", instead of duplicating the "free" functions. >> - free "dest_cmdline" at the end. >> --- >> tests/vhost-user-test.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c >> index 91ea373..dcb8617 100644 >> --- a/tests/vhost-user-test.c >> +++ b/tests/vhost-user-test.c >> @@ -717,7 +717,7 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc) >> guint64 size; >> >> if (!wait_for_fds(s)) { >> - return; >> + goto cleanup; >> } >> >> size = get_log_size(s); >> @@ -776,8 +776,11 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc) >> g_source_unref(source); >> >> qtest_quit(to); >> + >> + cleanup: >> test_server_free(dest); >> g_free(uri); >> + g_string_free(dest_cmdline, true); >> } >> >> static void wait_for_rings_started(TestServer *s, size_t count) >> > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > ... and picked up to my qtest-next tree. ... and now I had to unqueue the patch again. It is reproducibly causing one of the gitlab CI pipelines to fail with a timeout, e.g.: https://gitlab.com/huth/qemu/-/jobs/400101552 Not sure what is going on here, though, there is no obvious error message in the output... this needs some more investigation... do you have a gitlab account and could have a look? Thomas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] vhost-user-test: fix a memory leak 2020-01-12 10:39 ` Thomas Huth @ 2020-01-13 2:32 ` Pan Nengyuan 2020-01-15 3:10 ` Pan Nengyuan 0 siblings, 1 reply; 8+ messages in thread From: Pan Nengyuan @ 2020-01-13 2:32 UTC (permalink / raw) To: Thomas Huth, lvivier, pbonzini; +Cc: zhang.zhanghailiang, qemu-devel On 1/12/2020 6:39 PM, Thomas Huth wrote: > On 10/01/2020 15.07, Thomas Huth wrote: >> On 20/12/2019 02.26, pannengyuan@huawei.com wrote: >>> From: Pan Nengyuan <pannengyuan@huawei.com> >>> >>> Spotted by ASAN. >>> >>> Reported-by: Euler Robot <euler.robot@huawei.com> >>> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> >>> --- >>> Changes V2 to V1: >>> - use a "goto cleanup", instead of duplicating the "free" functions. >>> - free "dest_cmdline" at the end. >>> --- >> >> Reviewed-by: Thomas Huth <thuth@redhat.com> >> >> ... and picked up to my qtest-next tree. > > ... and now I had to unqueue the patch again. It is reproducibly causing > one of the gitlab CI pipelines to fail with a timeout, e.g.: > > https://gitlab.com/huth/qemu/-/jobs/400101552 > > Not sure what is going on here, though, there is no obvious error > message in the output... this needs some more investigation... do you > have a gitlab account and could have a look? > OK, I will register a account and have a look. > Thomas > > . > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] vhost-user-test: fix a memory leak 2020-01-13 2:32 ` Pan Nengyuan @ 2020-01-15 3:10 ` Pan Nengyuan 2020-01-15 9:13 ` Thomas Huth 0 siblings, 1 reply; 8+ messages in thread From: Pan Nengyuan @ 2020-01-15 3:10 UTC (permalink / raw) To: Thomas Huth; +Cc: lvivier, pbonzini, zhang.zhanghailiang, qemu-devel On 1/13/2020 10:32 AM, Pan Nengyuan wrote: > > > On 1/12/2020 6:39 PM, Thomas Huth wrote: >> On 10/01/2020 15.07, Thomas Huth wrote: >>> On 20/12/2019 02.26, pannengyuan@huawei.com wrote: >>>> From: Pan Nengyuan <pannengyuan@huawei.com> >>>> >>>> Spotted by ASAN. >>>> >>>> Reported-by: Euler Robot <euler.robot@huawei.com> >>>> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> >>>> --- >>>> Changes V2 to V1: >>>> - use a "goto cleanup", instead of duplicating the "free" functions. >>>> - free "dest_cmdline" at the end. >>>> --- >>> >>> Reviewed-by: Thomas Huth <thuth@redhat.com> >>> >>> ... and picked up to my qtest-next tree. >> >> ... and now I had to unqueue the patch again. It is reproducibly causing >> one of the gitlab CI pipelines to fail with a timeout, e.g.: >> >> https://gitlab.com/huth/qemu/-/jobs/400101552 >> >> Not sure what is going on here, though, there is no obvious error >> message in the output... this needs some more investigation... do you >> have a gitlab account and could have a look? >> > > OK, I will register a account and have a look. > I'm sorry, I build and test with the same params, but I can't reproduce it. Could you add "V=1 or V=2" params to get more information ? >> Thomas >> >> . >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] vhost-user-test: fix a memory leak 2020-01-15 3:10 ` Pan Nengyuan @ 2020-01-15 9:13 ` Thomas Huth 2020-01-16 13:29 ` Thomas Huth 0 siblings, 1 reply; 8+ messages in thread From: Thomas Huth @ 2020-01-15 9:13 UTC (permalink / raw) To: Pan Nengyuan; +Cc: lvivier, pbonzini, zhang.zhanghailiang, qemu-devel On 15/01/2020 04.10, Pan Nengyuan wrote: > > On 1/13/2020 10:32 AM, Pan Nengyuan wrote: >> >> On 1/12/2020 6:39 PM, Thomas Huth wrote: [...] >>> ... and now I had to unqueue the patch again. It is reproducibly causing >>> one of the gitlab CI pipelines to fail with a timeout, e.g.: >>> >>> https://gitlab.com/huth/qemu/-/jobs/400101552 >>> >>> Not sure what is going on here, though, there is no obvious error >>> message in the output... this needs some more investigation... do you >>> have a gitlab account and could have a look? >>> >> >> OK, I will register a account and have a look. >> > > I'm sorry, I build and test with the same params, but I can't reproduce it. > Could you add "V=1 or V=2" params to get more information ? It seems to hang forever in qos-test /arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/announce-self : https://gitlab.com/huth/qemu/-/jobs/403472594 It's completely weird, I also added some fprintf statements: https://gitlab.com/huth/qemu/commit/8ae76c0cf37cf46d26620dd ... but none of them show up in the output of the test run... so I'm currently completely puzzled what might be going wrong here... Any other ideas what we could try here? Thomas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] vhost-user-test: fix a memory leak 2020-01-15 9:13 ` Thomas Huth @ 2020-01-16 13:29 ` Thomas Huth 2020-01-17 0:44 ` Pan Nengyuan 0 siblings, 1 reply; 8+ messages in thread From: Thomas Huth @ 2020-01-16 13:29 UTC (permalink / raw) To: Pan Nengyuan; +Cc: lvivier, pbonzini, zhang.zhanghailiang, qemu-devel On 15/01/2020 10.13, Thomas Huth wrote: > On 15/01/2020 04.10, Pan Nengyuan wrote: >> >> On 1/13/2020 10:32 AM, Pan Nengyuan wrote: >>> >>> On 1/12/2020 6:39 PM, Thomas Huth wrote: > [...] >>>> ... and now I had to unqueue the patch again. It is reproducibly causing >>>> one of the gitlab CI pipelines to fail with a timeout, e.g.: >>>> >>>> https://gitlab.com/huth/qemu/-/jobs/400101552 >>>> >>>> Not sure what is going on here, though, there is no obvious error >>>> message in the output... this needs some more investigation... do you >>>> have a gitlab account and could have a look? >>>> >>> >>> OK, I will register a account and have a look. >>> >> >> I'm sorry, I build and test with the same params, but I can't reproduce it. >> Could you add "V=1 or V=2" params to get more information ? > > It seems to hang forever in qos-test > /arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/announce-self > : > > https://gitlab.com/huth/qemu/-/jobs/403472594 > > It's completely weird, I also added some fprintf statements: > > https://gitlab.com/huth/qemu/commit/8ae76c0cf37cf46d26620dd > > ... but none of them show up in the output of the test run... so I'm > currently completely puzzled what might be going wrong here... Any other > ideas what we could try here? I tried to add some more fprintfs here and there to see where it hangs, but I did not succeed to get any further. However, the CI build succeeds with this fix instead: diff a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c --- a/tests/qtest/vhost-user-test.c +++ b/tests/qtest/vhost-user-test.c @@ -707,9 +707,9 @@ static void test_read_guest_mem(void *obj, void *arg, QGuestAllocator *alloc) static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc) { TestServer *s = arg; - TestServer *dest = test_server_new("dest"); - GString *dest_cmdline = g_string_new(qos_get_current_command_line()); - char *uri = g_strdup_printf("%s%s", "unix:", dest->mig_path); + TestServer *dest; + GString *dest_cmdline; + char *uri; QTestState *to; GSource *source; QDict *rsp; @@ -720,6 +720,10 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc) return; } + dest = test_server_new("dest"); + dest_cmdline = g_string_new(qos_get_current_command_line()); + uri = g_strdup_printf("%s%s", "unix:", dest->mig_path); + size = get_log_size(s); g_assert_cmpint(size, ==, (256 * 1024 * 1024) / (VHOST_LOG_PAGE * 8)); @@ -778,6 +782,7 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc) qtest_quit(to); test_server_free(dest); g_free(uri); + g_string_free(dest_cmdline, true); } Here's a build with that patch that succeeded: https://gitlab.com/huth/qemu/-/jobs/405357307 So if you agree with that patch, I think we should simply use that version instead, ok? Thomas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] vhost-user-test: fix a memory leak 2020-01-16 13:29 ` Thomas Huth @ 2020-01-17 0:44 ` Pan Nengyuan 0 siblings, 0 replies; 8+ messages in thread From: Pan Nengyuan @ 2020-01-17 0:44 UTC (permalink / raw) To: Thomas Huth; +Cc: lvivier, pbonzini, zhang.zhanghailiang, qemu-devel On 1/16/2020 9:29 PM, Thomas Huth wrote: > On 15/01/2020 10.13, Thomas Huth wrote: >> On 15/01/2020 04.10, Pan Nengyuan wrote: >>> >>> On 1/13/2020 10:32 AM, Pan Nengyuan wrote: >>>> >>>> On 1/12/2020 6:39 PM, Thomas Huth wrote: >> [...] >>>>> ... and now I had to unqueue the patch again. It is reproducibly causing >>>>> one of the gitlab CI pipelines to fail with a timeout, e.g.: >>>>> >>>>> https://gitlab.com/huth/qemu/-/jobs/400101552 >>>>> >>>>> Not sure what is going on here, though, there is no obvious error >>>>> message in the output... this needs some more investigation... do you >>>>> have a gitlab account and could have a look? >>>>> >>>> >>>> OK, I will register a account and have a look. >>>> >>> >>> I'm sorry, I build and test with the same params, but I can't reproduce it. >>> Could you add "V=1 or V=2" params to get more information ? >> >> It seems to hang forever in qos-test >> /arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/announce-self >> : >> >> https://gitlab.com/huth/qemu/-/jobs/403472594 >> >> It's completely weird, I also added some fprintf statements: >> >> https://gitlab.com/huth/qemu/commit/8ae76c0cf37cf46d26620dd >> >> ... but none of them show up in the output of the test run... so I'm >> currently completely puzzled what might be going wrong here... Any other >> ideas what we could try here? > > I tried to add some more fprintfs here and there to see where it hangs, > but I did not succeed to get any further. > > However, the CI build succeeds with this fix instead: > > diff a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c > --- a/tests/qtest/vhost-user-test.c > +++ b/tests/qtest/vhost-user-test.c > @@ -707,9 +707,9 @@ static void test_read_guest_mem(void *obj, void > *arg, QGuestAllocator *alloc) > static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc) > { > TestServer *s = arg; > - TestServer *dest = test_server_new("dest"); > - GString *dest_cmdline = g_string_new(qos_get_current_command_line()); > - char *uri = g_strdup_printf("%s%s", "unix:", dest->mig_path); > + TestServer *dest; > + GString *dest_cmdline; > + char *uri; > QTestState *to; > GSource *source; > QDict *rsp; > @@ -720,6 +720,10 @@ static void test_migrate(void *obj, void *arg, > QGuestAllocator *alloc) > return; > } > > + dest = test_server_new("dest"); > + dest_cmdline = g_string_new(qos_get_current_command_line()); > + uri = g_strdup_printf("%s%s", "unix:", dest->mig_path); > + > size = get_log_size(s); > g_assert_cmpint(size, ==, (256 * 1024 * 1024) / (VHOST_LOG_PAGE * 8)); > > @@ -778,6 +782,7 @@ static void test_migrate(void *obj, void *arg, > QGuestAllocator *alloc) > qtest_quit(to); > test_server_free(dest); > g_free(uri); > + g_string_free(dest_cmdline, true); > } > > > Here's a build with that patch that succeeded: > > https://gitlab.com/huth/qemu/-/jobs/405357307 > > So if you agree with that patch, I think we should simply use that > version instead, ok? Ok, I agree with it. Thanks. > > Thomas > > . > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-01-17 0:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-20 1:26 [PATCH V2] vhost-user-test: fix a memory leak pannengyuan 2020-01-10 14:07 ` Thomas Huth 2020-01-12 10:39 ` Thomas Huth 2020-01-13 2:32 ` Pan Nengyuan 2020-01-15 3:10 ` Pan Nengyuan 2020-01-15 9:13 ` Thomas Huth 2020-01-16 13:29 ` Thomas Huth 2020-01-17 0:44 ` Pan Nengyuan
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).