* [PATCH 1/2] s390x: fix memleaks in cpu_finalize
@ 2020-02-15 8:32 pannengyuan
2020-02-15 8:27 ` Pan Nengyuan
2020-02-15 10:28 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 5+ messages in thread
From: pannengyuan @ 2020-02-15 8:32 UTC (permalink / raw)
To: cohuck, rth, david
Cc: qemu-s390x, euler.robot, Pan Nengyuan, qemu-devel, zhang.zhanghailiang
From: Pan Nengyuan <pannengyuan@huawei.com>
This patch fix memleaks when we call tests/qtest/cpu-plug-test on s390x. The leak stack is as follow:
Direct leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7fb43c7cd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7fb43be2149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x558ba96da716 in timer_new_full /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:530
#3 0x558ba96da716 in timer_new /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:551
#4 0x558ba96da716 in timer_new_ns /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:569
#5 0x558ba96da716 in s390_cpu_initfn /mnt/sdb/qemu-new/qemu/target/s390x/cpu.c:285
#6 0x558ba9c969ab in object_init_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:372
#7 0x558ba9c9eb5f in object_initialize_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:516
#8 0x558ba9c9f053 in object_new_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:684
#9 0x558ba967ede6 in s390x_new_cpu /mnt/sdb/qemu-new/qemu/hw/s390x/s390-virtio-ccw.c:64
#10 0x558ba99764b3 in hmp_cpu_add /mnt/sdb/qemu-new/qemu/hw/core/machine-hmp-cmds.c:57
#11 0x558ba9b1c27f in handle_hmp_command /mnt/sdb/qemu-new/qemu/monitor/hmp.c:1082
#12 0x558ba96c1b02 in qmp_human_monitor_command /mnt/sdb/qemu-new/qemu/monitor/misc.c:142
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
target/s390x/cpu.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index cf84d307c6..fff793a4eb 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -294,6 +294,10 @@ static void s390_cpu_finalize(Object *obj)
qemu_unregister_reset(s390_cpu_machine_reset_cb, cpu);
g_free(cpu->irqstate);
+ timer_del(cpu->env.tod_timer);
+ timer_del(cpu->env.cpu_timer);
+ timer_free(cpu->env.tod_timer);
+ timer_free(cpu->env.cpu_timer);
#endif
}
--
2.18.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] s390x: fix memleaks in cpu_finalize
2020-02-15 8:32 [PATCH 1/2] s390x: fix memleaks in cpu_finalize pannengyuan
@ 2020-02-15 8:27 ` Pan Nengyuan
2020-02-15 10:28 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 5+ messages in thread
From: Pan Nengyuan @ 2020-02-15 8:27 UTC (permalink / raw)
To: cohuck, rth, david
Cc: qemu-s390x, euler.robot, qemu-devel, zhang.zhanghailiang
On 2/15/2020 4:32 PM, pannengyuan@huawei.com wrote:
I'm sorry for the mail's subject, it's a single patch.
[PATCH 1/2] ---> [PATCH]
> From: Pan Nengyuan <pannengyuan@huawei.com>
>
> This patch fix memleaks when we call tests/qtest/cpu-plug-test on s390x. The leak stack is as follow:
>
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
> #0 0x7fb43c7cd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
> #1 0x7fb43be2149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
> #2 0x558ba96da716 in timer_new_full /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:530
> #3 0x558ba96da716 in timer_new /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:551
> #4 0x558ba96da716 in timer_new_ns /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:569
> #5 0x558ba96da716 in s390_cpu_initfn /mnt/sdb/qemu-new/qemu/target/s390x/cpu.c:285
> #6 0x558ba9c969ab in object_init_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:372
> #7 0x558ba9c9eb5f in object_initialize_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:516
> #8 0x558ba9c9f053 in object_new_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:684
> #9 0x558ba967ede6 in s390x_new_cpu /mnt/sdb/qemu-new/qemu/hw/s390x/s390-virtio-ccw.c:64
> #10 0x558ba99764b3 in hmp_cpu_add /mnt/sdb/qemu-new/qemu/hw/core/machine-hmp-cmds.c:57
> #11 0x558ba9b1c27f in handle_hmp_command /mnt/sdb/qemu-new/qemu/monitor/hmp.c:1082
> #12 0x558ba96c1b02 in qmp_human_monitor_command /mnt/sdb/qemu-new/qemu/monitor/misc.c:142
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> target/s390x/cpu.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index cf84d307c6..fff793a4eb 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -294,6 +294,10 @@ static void s390_cpu_finalize(Object *obj)
>
> qemu_unregister_reset(s390_cpu_machine_reset_cb, cpu);
> g_free(cpu->irqstate);
> + timer_del(cpu->env.tod_timer);
> + timer_del(cpu->env.cpu_timer);
> + timer_free(cpu->env.tod_timer);
> + timer_free(cpu->env.cpu_timer);
> #endif
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] s390x: fix memleaks in cpu_finalize
2020-02-15 8:32 [PATCH 1/2] s390x: fix memleaks in cpu_finalize pannengyuan
2020-02-15 8:27 ` Pan Nengyuan
@ 2020-02-15 10:28 ` Philippe Mathieu-Daudé
2020-02-15 10:42 ` Pan Nengyuan
2020-02-17 8:10 ` David Hildenbrand
1 sibling, 2 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-15 10:28 UTC (permalink / raw)
To: pannengyuan, cohuck, rth, david
Cc: zhang.zhanghailiang, qemu-s390x, qemu-devel, euler.robot
Hi Pan,
On 2/15/20 9:32 AM, pannengyuan@huawei.com wrote:
> From: Pan Nengyuan <pannengyuan@huawei.com>
>
> This patch fix memleaks when we call tests/qtest/cpu-plug-test on s390x. The leak stack is as follow:
>
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
> #0 0x7fb43c7cd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
> #1 0x7fb43be2149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
> #2 0x558ba96da716 in timer_new_full /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:530
> #3 0x558ba96da716 in timer_new /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:551
> #4 0x558ba96da716 in timer_new_ns /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:569
> #5 0x558ba96da716 in s390_cpu_initfn /mnt/sdb/qemu-new/qemu/target/s390x/cpu.c:285
> #6 0x558ba9c969ab in object_init_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:372
> #7 0x558ba9c9eb5f in object_initialize_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:516
> #8 0x558ba9c9f053 in object_new_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:684
> #9 0x558ba967ede6 in s390x_new_cpu /mnt/sdb/qemu-new/qemu/hw/s390x/s390-virtio-ccw.c:64
> #10 0x558ba99764b3 in hmp_cpu_add /mnt/sdb/qemu-new/qemu/hw/core/machine-hmp-cmds.c:57
> #11 0x558ba9b1c27f in handle_hmp_command /mnt/sdb/qemu-new/qemu/monitor/hmp.c:1082
> #12 0x558ba96c1b02 in qmp_human_monitor_command /mnt/sdb/qemu-new/qemu/monitor/misc.c:142
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> target/s390x/cpu.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index cf84d307c6..fff793a4eb 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -294,6 +294,10 @@ static void s390_cpu_finalize(Object *obj)
>
> qemu_unregister_reset(s390_cpu_machine_reset_cb, cpu);
> g_free(cpu->irqstate);
> + timer_del(cpu->env.tod_timer);
> + timer_del(cpu->env.cpu_timer);
Similarly to your other cleanups, shouldn't we move the timer_new_ns()
into a realize() function, then do the timer_del() in unrealize()?
> + timer_free(cpu->env.tod_timer);
> + timer_free(cpu->env.cpu_timer);
The timer_free() is correct.
> #endif
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] s390x: fix memleaks in cpu_finalize
2020-02-15 10:28 ` Philippe Mathieu-Daudé
@ 2020-02-15 10:42 ` Pan Nengyuan
2020-02-17 8:10 ` David Hildenbrand
1 sibling, 0 replies; 5+ messages in thread
From: Pan Nengyuan @ 2020-02-15 10:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, cohuck, rth, david
Cc: zhang.zhanghailiang, qemu-s390x, qemu-devel, euler.robot
On 2/15/2020 6:28 PM, Philippe Mathieu-Daudé wrote:
> Hi Pan,
>
> On 2/15/20 9:32 AM, pannengyuan@huawei.com wrote:
>> From: Pan Nengyuan <pannengyuan@huawei.com>
>>
>> This patch fix memleaks when we call tests/qtest/cpu-plug-test on s390x. The leak stack is as follow:
>>
>> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>> #0 0x7fb43c7cd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
>> #1 0x7fb43be2149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
>> #2 0x558ba96da716 in timer_new_full /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:530
>> #3 0x558ba96da716 in timer_new /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:551
>> #4 0x558ba96da716 in timer_new_ns /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:569
>> #5 0x558ba96da716 in s390_cpu_initfn /mnt/sdb/qemu-new/qemu/target/s390x/cpu.c:285
>> #6 0x558ba9c969ab in object_init_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:372
>> #7 0x558ba9c9eb5f in object_initialize_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:516
>> #8 0x558ba9c9f053 in object_new_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:684
>> #9 0x558ba967ede6 in s390x_new_cpu /mnt/sdb/qemu-new/qemu/hw/s390x/s390-virtio-ccw.c:64
>> #10 0x558ba99764b3 in hmp_cpu_add /mnt/sdb/qemu-new/qemu/hw/core/machine-hmp-cmds.c:57
>> #11 0x558ba9b1c27f in handle_hmp_command /mnt/sdb/qemu-new/qemu/monitor/hmp.c:1082
>> #12 0x558ba96c1b02 in qmp_human_monitor_command /mnt/sdb/qemu-new/qemu/monitor/misc.c:142
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
>> ---
>> target/s390x/cpu.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index cf84d307c6..fff793a4eb 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -294,6 +294,10 @@ static void s390_cpu_finalize(Object *obj)
>> qemu_unregister_reset(s390_cpu_machine_reset_cb, cpu);
>> g_free(cpu->irqstate);
>> + timer_del(cpu->env.tod_timer);
>> + timer_del(cpu->env.cpu_timer);
>
> Similarly to your other cleanups, shouldn't we move the timer_new_ns() into a realize() function, then do the timer_del() in unrealize()?
Sure, I have no problem with that. I will change and repost it.
Thanks.
>
>> + timer_free(cpu->env.tod_timer);
>> + timer_free(cpu->env.cpu_timer);
>
> The timer_free() is correct.
>
>> #endif
>> }
>>
>
> .
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] s390x: fix memleaks in cpu_finalize
2020-02-15 10:28 ` Philippe Mathieu-Daudé
2020-02-15 10:42 ` Pan Nengyuan
@ 2020-02-17 8:10 ` David Hildenbrand
1 sibling, 0 replies; 5+ messages in thread
From: David Hildenbrand @ 2020-02-17 8:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, pannengyuan, cohuck, rth
Cc: zhang.zhanghailiang, qemu-s390x, qemu-devel, euler.robot
On 15.02.20 11:28, Philippe Mathieu-Daudé wrote:
> Hi Pan,
>
> On 2/15/20 9:32 AM, pannengyuan@huawei.com wrote:
>> From: Pan Nengyuan <pannengyuan@huawei.com>
>>
>> This patch fix memleaks when we call tests/qtest/cpu-plug-test on s390x. The leak stack is as follow:
>>
>> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>> #0 0x7fb43c7cd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
>> #1 0x7fb43be2149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
>> #2 0x558ba96da716 in timer_new_full /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:530
>> #3 0x558ba96da716 in timer_new /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:551
>> #4 0x558ba96da716 in timer_new_ns /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:569
>> #5 0x558ba96da716 in s390_cpu_initfn /mnt/sdb/qemu-new/qemu/target/s390x/cpu.c:285
>> #6 0x558ba9c969ab in object_init_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:372
>> #7 0x558ba9c9eb5f in object_initialize_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:516
>> #8 0x558ba9c9f053 in object_new_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:684
>> #9 0x558ba967ede6 in s390x_new_cpu /mnt/sdb/qemu-new/qemu/hw/s390x/s390-virtio-ccw.c:64
>> #10 0x558ba99764b3 in hmp_cpu_add /mnt/sdb/qemu-new/qemu/hw/core/machine-hmp-cmds.c:57
>> #11 0x558ba9b1c27f in handle_hmp_command /mnt/sdb/qemu-new/qemu/monitor/hmp.c:1082
>> #12 0x558ba96c1b02 in qmp_human_monitor_command /mnt/sdb/qemu-new/qemu/monitor/misc.c:142
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
>> ---
>> target/s390x/cpu.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index cf84d307c6..fff793a4eb 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -294,6 +294,10 @@ static void s390_cpu_finalize(Object *obj)
>>
>> qemu_unregister_reset(s390_cpu_machine_reset_cb, cpu);
>> g_free(cpu->irqstate);
>> + timer_del(cpu->env.tod_timer);
>> + timer_del(cpu->env.cpu_timer);
>
> Similarly to your other cleanups, shouldn't we move the timer_new_ns()
> into a realize() function, then do the timer_del() in unrealize()?
include/hw/qdev-core.h
"Trivial field initializations should go into #TypeInfo.instance_init.
Operations depending on @props static properties should go into @realize."
IMHO, that here is the right thing to do. But not strong feelings.
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-17 8:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-15 8:32 [PATCH 1/2] s390x: fix memleaks in cpu_finalize pannengyuan
2020-02-15 8:27 ` Pan Nengyuan
2020-02-15 10:28 ` Philippe Mathieu-Daudé
2020-02-15 10:42 ` Pan Nengyuan
2020-02-17 8:10 ` David Hildenbrand
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.