[PATCH 0/3] delay timer_new to avoid memleaks

[PATCH 0/3] delay timer_new to avoid memleaks

[pannengyuan]

From: Pan Nengyuan <pannengyuan@huawei.com>

This series delay timer_new into realize() to fix some memleaks when we call 'device-list-properties'.

Pan Nengyuan (3):
  armv7m_systick: delay timer_new to avoid memleaks
  stm32f2xx_timer: delay timer_new to avoid memleaks
  stellaris: delay timer_new to avoid memleaks

 hw/arm/stellaris.c         | 7 ++++++-
 hw/timer/armv7m_systick.c  | 6 ++++++
 hw/timer/stm32f2xx_timer.c | 5 +++++
 3 files changed, 17 insertions(+), 1 deletion(-)

-- 
2.21.0.windows.1

[PATCH 1/3] armv7m_systick: delay timer_new to avoid memleaks

[pannengyuan]

From: Pan Nengyuan <pannengyuan@huawei.com>

There is a memory leak when we call 'device_list_properties' with typename = armv7m_systick. It's easy to reproduce as follow:

  virsh qemu-monitor-command vm1 --pretty '{"execute": "device-list-properties", "arguments": {"typename": "armv7m_systick"}}'

This patch delay timer_new to fix this memleaks.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Cc: qemu-arm@nongnu.org
---
 hw/timer/armv7m_systick.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
index 85d122dbcb..74c58bcf24 100644
--- a/hw/timer/armv7m_systick.c
+++ b/hw/timer/armv7m_systick.c
@@ -216,6 +216,11 @@ static void systick_instance_init(Object *obj)
     memory_region_init_io(&s->iomem, obj, &systick_ops, s, "systick", 0xe0);
     sysbus_init_mmio(sbd, &s->iomem);
     sysbus_init_irq(sbd, &s->irq);
+}
+
+static void systick_realize(DeviceState *dev, Error **errp)
+{
+    SysTickState *s = SYSTICK(dev);
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
 }
 
@@ -238,6 +243,7 @@ static void systick_class_init(ObjectClass *klass, void *data)
 
     dc->vmsd = &vmstate_systick;
     dc->reset = systick_reset;
+    dc->realize = systick_realize;
 }
 
 static const TypeInfo armv7m_systick_info = {
-- 
2.21.0.windows.1

[PATCH 2/3] stm32f2xx_timer: delay timer_new to avoid memleaks

[pannengyuan]

From: Pan Nengyuan <pannengyuan@huawei.com>

There is a memory leak when we call 'device_list_properties' with typename = stm32f2xx_timer. It's easy to reproduce as follow:

    virsh qemu-monitor-command vm1 --pretty '{"execute": "device-list-properties", "arguments": {"typename": "stm32f2xx_timer"}}'

This patch delay timer_new to fix this memleaks.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Cc: Alistair Francis <alistair@alistair23.me>
---
 hw/timer/stm32f2xx_timer.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
index fb370ce0f0..06ec8a02c2 100644
--- a/hw/timer/stm32f2xx_timer.c
+++ b/hw/timer/stm32f2xx_timer.c
@@ -314,7 +314,11 @@ static void stm32f2xx_timer_init(Object *obj)
     memory_region_init_io(&s->iomem, obj, &stm32f2xx_timer_ops, s,
                           "stm32f2xx_timer", 0x400);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+}
 
+static void stm32f2xx_timer_realize(DeviceState *dev, Error **errp)
+{
+    STM32F2XXTimerState *s = STM32F2XXTIMER(dev);
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, stm32f2xx_timer_interrupt, s);
 }
 
@@ -325,6 +329,7 @@ static void stm32f2xx_timer_class_init(ObjectClass *klass, void *data)
     dc->reset = stm32f2xx_timer_reset;
     device_class_set_props(dc, stm32f2xx_timer_properties);
     dc->vmsd = &vmstate_stm32f2xx_timer;
+    dc->realize = stm32f2xx_timer_realize;
 }
 
 static const TypeInfo stm32f2xx_timer_info = {
-- 
2.21.0.windows.1

[PATCH 3/3] stellaris: delay timer_new to avoid memleaks

[pannengyuan]

From: Pan Nengyuan <pannengyuan@huawei.com>

There is a memory leak when we call 'device_list_properties' with typename = stellaris-gptm. It's easy to reproduce as follow:

  virsh qemu-monitor-command vm1 --pretty '{"execute": "device-list-properties", "arguments": {"typename": "stellaris-gptm"}}'

This patch delay timer_new in realize to fix it.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Cc: qemu-arm@nongnu.org
---
 hw/arm/stellaris.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index bb025e0bd0..221a78674e 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -347,11 +347,15 @@ static void stellaris_gptm_init(Object *obj)
     sysbus_init_mmio(sbd, &s->iomem);
 
     s->opaque[0] = s->opaque[1] = s;
+}
+
+static void stellaris_gptm_realize(DeviceState *dev, Error **errp)
+{
+    gptm_state *s = STELLARIS_GPTM(dev);
     s->timer[0] = timer_new_ns(QEMU_CLOCK_VIRTUAL, gptm_tick, &s->opaque[0]);
     s->timer[1] = timer_new_ns(QEMU_CLOCK_VIRTUAL, gptm_tick, &s->opaque[1]);
 }
 
-
 /* System controller.  */
 
 typedef struct {
@@ -1536,6 +1540,7 @@ static void stellaris_gptm_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &vmstate_stellaris_gptm;
+    dc->realize = stellaris_gptm_realize;
 }
 
 static const TypeInfo stellaris_gptm_info = {
-- 
2.21.0.windows.1

Re: [PATCH 3/3] stellaris: delay timer_new to avoid memleaks

[Philippe Mathieu-Daudé]

On 2/5/20 8:06 AM, pannengyuan@huawei.com wrote:
> From: Pan Nengyuan <pannengyuan@huawei.com>
> 
> There is a memory leak when we call 'device_list_properties' with typename = stellaris-gptm. It's easy to reproduce as follow:
> 
>    virsh qemu-monitor-command vm1 --pretty '{"execute": "device-list-properties", "arguments": {"typename": "stellaris-gptm"}}'
> 
> This patch delay timer_new in realize to fix it.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> Cc: qemu-arm@nongnu.org
> ---
>   hw/arm/stellaris.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index bb025e0bd0..221a78674e 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -347,11 +347,15 @@ static void stellaris_gptm_init(Object *obj)
>       sysbus_init_mmio(sbd, &s->iomem);
>   
>       s->opaque[0] = s->opaque[1] = s;
> +}
> +
> +static void stellaris_gptm_realize(DeviceState *dev, Error **errp)
> +{
> +    gptm_state *s = STELLARIS_GPTM(dev);
>       s->timer[0] = timer_new_ns(QEMU_CLOCK_VIRTUAL, gptm_tick, &s->opaque[0]);
>       s->timer[1] = timer_new_ns(QEMU_CLOCK_VIRTUAL, gptm_tick, &s->opaque[1]);
>   }
>   
> -
>   /* System controller.  */
>   
>   typedef struct {
> @@ -1536,6 +1540,7 @@ static void stellaris_gptm_class_init(ObjectClass *klass, void *data)
>       DeviceClass *dc = DEVICE_CLASS(klass);
>   
>       dc->vmsd = &vmstate_stellaris_gptm;
> +    dc->realize = stellaris_gptm_realize;
>   }
>   
>   static const TypeInfo stellaris_gptm_info = {
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH 0/3] delay timer_new to avoid memleaks

[Philippe Mathieu-Daudé]

On 2/5/20 8:06 AM, pannengyuan@huawei.com wrote:
> From: Pan Nengyuan <pannengyuan@huawei.com>
> 
> This series delay timer_new into realize() to fix some memleaks when we call 'device-list-properties'.
> 
> Pan Nengyuan (3):
>    armv7m_systick: delay timer_new to avoid memleaks
>    stm32f2xx_timer: delay timer_new to avoid memleaks
>    stellaris: delay timer_new to avoid memleaks
> 
>   hw/arm/stellaris.c         | 7 ++++++-
>   hw/timer/armv7m_systick.c  | 6 ++++++
>   hw/timer/stm32f2xx_timer.c | 5 +++++
>   3 files changed, 17 insertions(+), 1 deletion(-)

You might want to look at Coccinelle [*] and write a spatch script to 
check/fix all the codebase at once. You can find some examples in 
scripts/coccinelle/.

[*] http://coccinelle.lip6.fr/

Re: [PATCH 2/3] stm32f2xx_timer: delay timer_new to avoid memleaks

[Philippe Mathieu-Daudé]

On 2/5/20 8:06 AM, pannengyuan@huawei.com wrote:
> From: Pan Nengyuan <pannengyuan@huawei.com>
> 
> There is a memory leak when we call 'device_list_properties' with typename = stm32f2xx_timer. It's easy to reproduce as follow:
> 
>      virsh qemu-monitor-command vm1 --pretty '{"execute": "device-list-properties", "arguments": {"typename": "stm32f2xx_timer"}}'
> 
> This patch delay timer_new to fix this memleaks.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> Cc: Alistair Francis <alistair@alistair23.me>
> ---
>   hw/timer/stm32f2xx_timer.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
> index fb370ce0f0..06ec8a02c2 100644
> --- a/hw/timer/stm32f2xx_timer.c
> +++ b/hw/timer/stm32f2xx_timer.c
> @@ -314,7 +314,11 @@ static void stm32f2xx_timer_init(Object *obj)
>       memory_region_init_io(&s->iomem, obj, &stm32f2xx_timer_ops, s,
>                             "stm32f2xx_timer", 0x400);
>       sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> +}
>   
> +static void stm32f2xx_timer_realize(DeviceState *dev, Error **errp)
> +{
> +    STM32F2XXTimerState *s = STM32F2XXTIMER(dev);
>       s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, stm32f2xx_timer_interrupt, s);
>   }
>   
> @@ -325,6 +329,7 @@ static void stm32f2xx_timer_class_init(ObjectClass *klass, void *data)
>       dc->reset = stm32f2xx_timer_reset;
>       device_class_set_props(dc, stm32f2xx_timer_properties);
>       dc->vmsd = &vmstate_stm32f2xx_timer;
> +    dc->realize = stm32f2xx_timer_realize;
>   }
>   
>   static const TypeInfo stm32f2xx_timer_info = {
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH 2/3] stm32f2xx_timer: delay timer_new to avoid memleaks

[Alistair Francis]

On Tue, Feb 4, 2020 at 11:09 PM <pannengyuan@huawei.com> wrote:
>
> From: Pan Nengyuan <pannengyuan@huawei.com>
>
> There is a memory leak when we call 'device_list_properties' with typename = stm32f2xx_timer. It's easy to reproduce as follow:
>
>     virsh qemu-monitor-command vm1 --pretty '{"execute": "device-list-properties", "arguments": {"typename": "stm32f2xx_timer"}}'
>
> This patch delay timer_new to fix this memleaks.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> Cc: Alistair Francis <alistair@alistair23.me>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/timer/stm32f2xx_timer.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
> index fb370ce0f0..06ec8a02c2 100644
> --- a/hw/timer/stm32f2xx_timer.c
> +++ b/hw/timer/stm32f2xx_timer.c
> @@ -314,7 +314,11 @@ static void stm32f2xx_timer_init(Object *obj)
>      memory_region_init_io(&s->iomem, obj, &stm32f2xx_timer_ops, s,
>                            "stm32f2xx_timer", 0x400);
>      sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> +}
>
> +static void stm32f2xx_timer_realize(DeviceState *dev, Error **errp)
> +{
> +    STM32F2XXTimerState *s = STM32F2XXTIMER(dev);
>      s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, stm32f2xx_timer_interrupt, s);
>  }
>
> @@ -325,6 +329,7 @@ static void stm32f2xx_timer_class_init(ObjectClass *klass, void *data)
>      dc->reset = stm32f2xx_timer_reset;
>      device_class_set_props(dc, stm32f2xx_timer_properties);
>      dc->vmsd = &vmstate_stm32f2xx_timer;
> +    dc->realize = stm32f2xx_timer_realize;
>  }
>
>  static const TypeInfo stm32f2xx_timer_info = {
> --
> 2.21.0.windows.1
>
>
>

Re: [PATCH 0/3] delay timer_new to avoid memleaks

[Pan Nengyuan]

On 2/5/2020 9:16 PM, Philippe Mathieu-Daudé wrote:
> On 2/5/20 8:06 AM, pannengyuan@huawei.com wrote:
>> From: Pan Nengyuan <pannengyuan@huawei.com>
>>
>> This series delay timer_new into realize() to fix some memleaks when we call 'device-list-properties'.
>>
>> Pan Nengyuan (3):
>>    armv7m_systick: delay timer_new to avoid memleaks
>>    stm32f2xx_timer: delay timer_new to avoid memleaks
>>    stellaris: delay timer_new to avoid memleaks
>>
>>   hw/arm/stellaris.c         | 7 ++++++-
>>   hw/timer/armv7m_systick.c  | 6 ++++++
>>   hw/timer/stm32f2xx_timer.c | 5 +++++
>>   3 files changed, 17 insertions(+), 1 deletion(-)
> 
> You might want to look at Coccinelle [*] and write a spatch script to check/fix all the codebase at once. You can find some examples in scripts/coccinelle/.

Thanks for these tips. I will pay attention.

> 
> [*] http://coccinelle.lip6.fr/
> 

Re: [PATCH 0/3] delay timer_new to avoid memleaks

[Peter Maydell]

On Wed, 5 Feb 2020 at 07:07, <pannengyuan@huawei.com> wrote:
>
> From: Pan Nengyuan <pannengyuan@huawei.com>
>
> This series delay timer_new into realize() to fix some memleaks when we call 'device-list-properties'.
>
> Pan Nengyuan (3):
>   armv7m_systick: delay timer_new to avoid memleaks
>   stm32f2xx_timer: delay timer_new to avoid memleaks
>   stellaris: delay timer_new to avoid memleaks
>
>  hw/arm/stellaris.c         | 7 ++++++-
>  hw/timer/armv7m_systick.c  | 6 ++++++
>  hw/timer/stm32f2xx_timer.c | 5 +++++
>  3 files changed, 17 insertions(+), 1 deletion(-)



Applied to target-arm.next, thanks.

-- PMM