diff mbox series

[v3,2/5] softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()

Message ID 20220321141409.3112932-3-mark.kanda@oracle.com
State New
Headers show
Series vCPU hotunplug related memory leaks | expand

Commit Message

Mark Kanda March 21, 2022, 2:14 p.m. UTC 
Free cpu->thread in a new AccelOpsClass::destroy_vcpu_thread() handler
generic_destroy_vcpu_thread().

vCPU hotunplug related leak reported by Valgrind:

==102631== 8 bytes in 1 blocks are definitely lost in loss record 1,037 of 8,555
==102631==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
==102631==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
==102631==    by 0x92443A: kvm_start_vcpu_thread (kvm-accel-ops.c:68)
==102631==    by 0x4505C2: qemu_init_vcpu (cpus.c:643)
==102631==    by 0x76B4D1: x86_cpu_realizefn (cpu.c:6520)
==102631==    by 0x9344A7: device_set_realized (qdev.c:531)
==102631==    by 0x93E329: property_set_bool (object.c:2273)
==102631==    by 0x93C2F8: object_property_set (object.c:1408)
==102631==    by 0x940796: object_property_set_qobject (qom-qobject.c:28)
==102631==    by 0x93C663: object_property_set_bool (object.c:1477)
==102631==    by 0x933D3B: qdev_realize (qdev.c:333)
==102631==    by 0x455EC4: qdev_device_add_from_qdict (qdev-monitor.c:713)

Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
---
 accel/accel-common.c              | 6 ++++++
 accel/hvf/hvf-accel-ops.c         | 1 +
 accel/kvm/kvm-accel-ops.c         | 1 +
 accel/qtest/qtest.c               | 1 +
 accel/tcg/tcg-accel-ops.c         | 1 +
 accel/xen/xen-all.c               | 1 +
 include/sysemu/accel-ops.h        | 2 ++
 target/i386/hax/hax-accel-ops.c   | 1 +
 target/i386/nvmm/nvmm-accel-ops.c | 1 +
 target/i386/whpx/whpx-accel-ops.c | 1 +
 10 files changed, 16 insertions(+)

Comments

Philippe Mathieu-Daudé March 21, 2022, 10:08 p.m. UTC | #1 
On 21/3/22 15:14, Mark Kanda wrote:
> Free cpu->thread in a new AccelOpsClass::destroy_vcpu_thread() handler
> generic_destroy_vcpu_thread().
> 
> vCPU hotunplug related leak reported by Valgrind:
> 
> ==102631== 8 bytes in 1 blocks are definitely lost in loss record 1,037 of 8,555
> ==102631==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
> ==102631==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
> ==102631==    by 0x92443A: kvm_start_vcpu_thread (kvm-accel-ops.c:68)
> ==102631==    by 0x4505C2: qemu_init_vcpu (cpus.c:643)
> ==102631==    by 0x76B4D1: x86_cpu_realizefn (cpu.c:6520)
> ==102631==    by 0x9344A7: device_set_realized (qdev.c:531)
> ==102631==    by 0x93E329: property_set_bool (object.c:2273)
> ==102631==    by 0x93C2F8: object_property_set (object.c:1408)
> ==102631==    by 0x940796: object_property_set_qobject (qom-qobject.c:28)
> ==102631==    by 0x93C663: object_property_set_bool (object.c:1477)
> ==102631==    by 0x933D3B: qdev_realize (qdev.c:333)
> ==102631==    by 0x455EC4: qdev_device_add_from_qdict (qdev-monitor.c:713)
> 
> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
> ---
>   accel/accel-common.c              | 6 ++++++
>   accel/hvf/hvf-accel-ops.c         | 1 +
>   accel/kvm/kvm-accel-ops.c         | 1 +
>   accel/qtest/qtest.c               | 1 +
>   accel/tcg/tcg-accel-ops.c         | 1 +
>   accel/xen/xen-all.c               | 1 +
>   include/sysemu/accel-ops.h        | 2 ++
>   target/i386/hax/hax-accel-ops.c   | 1 +
>   target/i386/nvmm/nvmm-accel-ops.c | 1 +
>   target/i386/whpx/whpx-accel-ops.c | 1 +
>   10 files changed, 16 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Paolo Bonzini March 23, 2022, 2:43 p.m. UTC | #2 
On 3/21/22 15:14, Mark Kanda wrote:
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index ea7dcad674..527592c4d7 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -94,6 +94,7 @@ void tcg_handle_interrupt(CPUState *cpu, int mask)
>   
>   static void tcg_accel_ops_init(AccelOpsClass *ops)
>   {
> +    ops->destroy_vcpu_thread = generic_destroy_vcpu_thread;
>       if (qemu_tcg_mttcg_enabled()) {
>           ops->create_vcpu_thread = mttcg_start_vcpu_thread;
>           ops->kick_vcpu_thread = mttcg_kick_vcpu_thread;

This should not be done for the icount case, where there is only one thread.

Paolo
diff mbox series

Patch

diff --git a/accel/accel-common.c b/accel/accel-common.c
index 7b8ec7e0f7..623df43cc3 100644
--- a/accel/accel-common.c
+++ b/accel/accel-common.c
@@ -28,6 +28,7 @@ 
 
 #include "cpu.h"
 #include "hw/core/accel-cpu.h"
+#include "sysemu/accel-ops.h"
 
 #ifndef CONFIG_USER_ONLY
 #include "accel-softmmu.h"
@@ -135,3 +136,8 @@  static void register_accel_types(void)
 }
 
 type_init(register_accel_types);
+
+void generic_destroy_vcpu_thread(CPUState *cpu)
+{
+    g_free(cpu->thread);
+}
diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
index 54457c76c2..b23a67881c 100644
--- a/accel/hvf/hvf-accel-ops.c
+++ b/accel/hvf/hvf-accel-ops.c
@@ -467,6 +467,7 @@  static void hvf_accel_ops_class_init(ObjectClass *oc, void *data)
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
     ops->create_vcpu_thread = hvf_start_vcpu_thread;
+    ops->destroy_vcpu_thread = generic_destroy_vcpu_thread;
     ops->kick_vcpu_thread = hvf_kick_vcpu_thread;
 
     ops->synchronize_post_reset = hvf_cpu_synchronize_post_reset;
diff --git a/accel/kvm/kvm-accel-ops.c b/accel/kvm/kvm-accel-ops.c
index c4244a23c6..5a7a9ae79c 100644
--- a/accel/kvm/kvm-accel-ops.c
+++ b/accel/kvm/kvm-accel-ops.c
@@ -89,6 +89,7 @@  static void kvm_accel_ops_class_init(ObjectClass *oc, void *data)
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
     ops->create_vcpu_thread = kvm_start_vcpu_thread;
+    ops->destroy_vcpu_thread = generic_destroy_vcpu_thread;
     ops->cpu_thread_is_idle = kvm_vcpu_thread_is_idle;
     ops->cpus_are_resettable = kvm_cpus_are_resettable;
     ops->synchronize_post_reset = kvm_cpu_synchronize_post_reset;
diff --git a/accel/qtest/qtest.c b/accel/qtest/qtest.c
index f6056ac836..ba8573fc2c 100644
--- a/accel/qtest/qtest.c
+++ b/accel/qtest/qtest.c
@@ -51,6 +51,7 @@  static void qtest_accel_ops_class_init(ObjectClass *oc, void *data)
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
     ops->create_vcpu_thread = dummy_start_vcpu_thread;
+    ops->destroy_vcpu_thread = generic_destroy_vcpu_thread;
     ops->get_virtual_clock = qtest_get_virtual_clock;
 };
 
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index ea7dcad674..527592c4d7 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -94,6 +94,7 @@  void tcg_handle_interrupt(CPUState *cpu, int mask)
 
 static void tcg_accel_ops_init(AccelOpsClass *ops)
 {
+    ops->destroy_vcpu_thread = generic_destroy_vcpu_thread;
     if (qemu_tcg_mttcg_enabled()) {
         ops->create_vcpu_thread = mttcg_start_vcpu_thread;
         ops->kick_vcpu_thread = mttcg_kick_vcpu_thread;
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 69aa7d018b..0efda554cc 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -220,6 +220,7 @@  static void xen_accel_ops_class_init(ObjectClass *oc, void *data)
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
     ops->create_vcpu_thread = dummy_start_vcpu_thread;
+    ops->destroy_vcpu_thread = generic_destroy_vcpu_thread;
 }
 
 static const TypeInfo xen_accel_ops_type = {
diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index e296b27b82..fac7d6b34e 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -46,4 +46,6 @@  struct AccelOpsClass {
     int64_t (*get_elapsed_ticks)(void);
 };
 
+/* free vcpu thread structures */
+void generic_destroy_vcpu_thread(CPUState *cpu);
 #endif /* ACCEL_OPS_H */
diff --git a/target/i386/hax/hax-accel-ops.c b/target/i386/hax/hax-accel-ops.c
index 136630e9b2..b6ef246cbb 100644
--- a/target/i386/hax/hax-accel-ops.c
+++ b/target/i386/hax/hax-accel-ops.c
@@ -79,6 +79,7 @@  static void hax_accel_ops_class_init(ObjectClass *oc, void *data)
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
     ops->create_vcpu_thread = hax_start_vcpu_thread;
+    ops->destroy_vcpu_thread = generic_destroy_vcpu_thread;
     ops->kick_vcpu_thread = hax_kick_vcpu_thread;
 
     ops->synchronize_post_reset = hax_cpu_synchronize_post_reset;
diff --git a/target/i386/nvmm/nvmm-accel-ops.c b/target/i386/nvmm/nvmm-accel-ops.c
index f788f75289..ff9033127a 100644
--- a/target/i386/nvmm/nvmm-accel-ops.c
+++ b/target/i386/nvmm/nvmm-accel-ops.c
@@ -88,6 +88,7 @@  static void nvmm_accel_ops_class_init(ObjectClass *oc, void *data)
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
     ops->create_vcpu_thread = nvmm_start_vcpu_thread;
+    ops->destroy_vcpu_thread = generic_destroy_vcpu_thread;
     ops->kick_vcpu_thread = nvmm_kick_vcpu_thread;
 
     ops->synchronize_post_reset = nvmm_cpu_synchronize_post_reset;
diff --git a/target/i386/whpx/whpx-accel-ops.c b/target/i386/whpx/whpx-accel-ops.c
index 1d30e4e2ed..f4ec6b3518 100644
--- a/target/i386/whpx/whpx-accel-ops.c
+++ b/target/i386/whpx/whpx-accel-ops.c
@@ -93,6 +93,7 @@  static void whpx_accel_ops_class_init(ObjectClass *oc, void *data)
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
     ops->create_vcpu_thread = whpx_start_vcpu_thread;
+    ops->destroy_vcpu_thread = generic_destroy_vcpu_thread;
     ops->kick_vcpu_thread = whpx_kick_vcpu_thread;
     ops->cpu_thread_is_idle = whpx_vcpu_thread_is_idle;