| Message ID | 20200305065422.12707-4-pannengyuan@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | delay timer_new from init to realize to fix memleaks. | expand |
On Thu, Mar 05, 2020 at 02:54:22PM +0800, Pan Nengyuan wrote: > There are some memleaks when we call 'device_list_properties'. This patch move timer_new from init into realize to fix it. > > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> Applied to ppc-for-5.0. Probably the memory region stuff should be in realize() rather than init() as well, but that can be fixed later. > --- > Cc: Laurent Vivier <laurent@vivier.eu> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > Cc: David Gibson <david@gibson.dropbear.id.au> > Cc: qemu-ppc@nongnu.org > --- > v2->v1: > - no changes in this patch. > v3->v2: > - remove null check in reset, and add calls to mos6522_realize() in mac_via_realize to make this move to be valid. > v4->v3: > - split patch into two, this patch fix the memleaks. > --- > hw/misc/mos6522.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c > index 19e154b870..c1cd154a84 100644 > --- a/hw/misc/mos6522.c > +++ b/hw/misc/mos6522.c > @@ -485,6 +485,11 @@ static void mos6522_init(Object *obj) > for (i = 0; i < ARRAY_SIZE(s->timers); i++) { > s->timers[i].index = i; > } > +} > + > +static void mos6522_realize(DeviceState *dev, Error **errp) > +{ > + MOS6522State *s = MOS6522(dev); > > s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer1, s); > s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer2, s); > @@ -502,6 +507,7 @@ static void mos6522_class_init(ObjectClass *oc, void *data) > > dc->reset = mos6522_reset; > dc->vmsd = &vmstate_mos6522; > + dc->realize = mos6522_realize; > device_class_set_props(dc, mos6522_properties); > mdc->parent_reset = dc->reset; > mdc->set_sr_int = mos6522_set_sr_int;
On 3/6/2020 6:56 AM, David Gibson wrote: > On Thu, Mar 05, 2020 at 02:54:22PM +0800, Pan Nengyuan wrote: >> There are some memleaks when we call 'device_list_properties'. This patch move timer_new from init into realize to fix it. >> >> Reported-by: Euler Robot <euler.robot@huawei.com> >> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> > > Applied to ppc-for-5.0. Thanks. And this patch depend to another fix (patch2/3: https://patchwork.kernel.org/patch/11421229/). Otherwise, it'll be invalid for this move. I forgot cc it to you, but I think it should let you known. > > Probably the memory region stuff should be in realize() rather than > init() as well, but that can be fixed later. > >> --- >> Cc: Laurent Vivier <laurent@vivier.eu> >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> Cc: David Gibson <david@gibson.dropbear.id.au> >> Cc: qemu-ppc@nongnu.org >> --- >> v2->v1: >> - no changes in this patch. >> v3->v2: >> - remove null check in reset, and add calls to mos6522_realize() in mac_via_realize to make this move to be valid. >> v4->v3: >> - split patch into two, this patch fix the memleaks. >> --- >> hw/misc/mos6522.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c >> index 19e154b870..c1cd154a84 100644 >> --- a/hw/misc/mos6522.c >> +++ b/hw/misc/mos6522.c >> @@ -485,6 +485,11 @@ static void mos6522_init(Object *obj) >> for (i = 0; i < ARRAY_SIZE(s->timers); i++) { >> s->timers[i].index = i; >> } >> +} >> + >> +static void mos6522_realize(DeviceState *dev, Error **errp) >> +{ >> + MOS6522State *s = MOS6522(dev); >> >> s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer1, s); >> s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer2, s); >> @@ -502,6 +507,7 @@ static void mos6522_class_init(ObjectClass *oc, void *data) >> >> dc->reset = mos6522_reset; >> dc->vmsd = &vmstate_mos6522; >> + dc->realize = mos6522_realize; >> device_class_set_props(dc, mos6522_properties); >> mdc->parent_reset = dc->reset; >> mdc->set_sr_int = mos6522_set_sr_int; >
On Fri, Mar 06, 2020 at 09:56:52AM +1100, David Gibson wrote: > On Thu, Mar 05, 2020 at 02:54:22PM +0800, Pan Nengyuan wrote: > > There are some memleaks when we call 'device_list_properties'. This patch move timer_new from init into realize to fix it. > > > > Reported-by: Euler Robot <euler.robot@huawei.com> > > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> > > Applied to ppc-for-5.0. > > Probably the memory region stuff should be in realize() rather than > init() as well, but that can be fixed later. ....and removed again. This causes SEGVs during make check-qtest-ppc64. > > > --- > > Cc: Laurent Vivier <laurent@vivier.eu> > > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > > Cc: David Gibson <david@gibson.dropbear.id.au> > > Cc: qemu-ppc@nongnu.org > > --- > > v2->v1: > > - no changes in this patch. > > v3->v2: > > - remove null check in reset, and add calls to mos6522_realize() in mac_via_realize to make this move to be valid. > > v4->v3: > > - split patch into two, this patch fix the memleaks. > > --- > > hw/misc/mos6522.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c > > index 19e154b870..c1cd154a84 100644 > > --- a/hw/misc/mos6522.c > > +++ b/hw/misc/mos6522.c > > @@ -485,6 +485,11 @@ static void mos6522_init(Object *obj) > > for (i = 0; i < ARRAY_SIZE(s->timers); i++) { > > s->timers[i].index = i; > > } > > +} > > + > > +static void mos6522_realize(DeviceState *dev, Error **errp) > > +{ > > + MOS6522State *s = MOS6522(dev); > > > > s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer1, s); > > s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer2, s); > > @@ -502,6 +507,7 @@ static void mos6522_class_init(ObjectClass *oc, void *data) > > > > dc->reset = mos6522_reset; > > dc->vmsd = &vmstate_mos6522; > > + dc->realize = mos6522_realize; > > device_class_set_props(dc, mos6522_properties); > > mdc->parent_reset = dc->reset; > > mdc->set_sr_int = mos6522_set_sr_int; >
diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c index 19e154b870..c1cd154a84 100644 --- a/hw/misc/mos6522.c +++ b/hw/misc/mos6522.c @@ -485,6 +485,11 @@ static void mos6522_init(Object *obj) for (i = 0; i < ARRAY_SIZE(s->timers); i++) { s->timers[i].index = i; } +} + +static void mos6522_realize(DeviceState *dev, Error **errp) +{ + MOS6522State *s = MOS6522(dev); s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer1, s); s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer2, s); @@ -502,6 +507,7 @@ static void mos6522_class_init(ObjectClass *oc, void *data) dc->reset = mos6522_reset; dc->vmsd = &vmstate_mos6522; + dc->realize = mos6522_realize; device_class_set_props(dc, mos6522_properties); mdc->parent_reset = dc->reset; mdc->set_sr_int = mos6522_set_sr_int;
There are some memleaks when we call 'device_list_properties'. This patch move timer_new from init into realize to fix it. Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> --- Cc: Laurent Vivier <laurent@vivier.eu> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Cc: David Gibson <david@gibson.dropbear.id.au> Cc: qemu-ppc@nongnu.org --- v2->v1: - no changes in this patch. v3->v2: - remove null check in reset, and add calls to mos6522_realize() in mac_via_realize to make this move to be valid. v4->v3: - split patch into two, this patch fix the memleaks. --- hw/misc/mos6522.c | 6 ++++++ 1 file changed, 6 insertions(+)