| Message ID | 20190823143249.8096-4-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm: Use ARM_CPU_TYPE_NAME() and object_initialize_child() | expand |
On 8/23/19 4:32 PM, Philippe Mathieu-Daudé wrote: > As explained in commit aff39be0ed97: > > Both functions, object_initialize() and qdev_set_parent_bus() Commit aff39be0ed97 was not about qdev_set_parent_bus(), so the first sentence sounds somewhat misleading here. Maybe rephrase the commit message without that reference to aff39be0ed97 ? > increase the reference counter of the new object, so one of the > references has to be dropped afterwards to get the reference > counting right. Otherwise the child object will not be properly > cleaned up when the parent gets destroyed. Well, the parent here (the machine) currently never gets destroyed ... so unless you've got a patch in your pipe to fix that, too, you should maybe also rephrase this part of the commit message. Thomas
On 8/23/19 7:32 AM, Philippe Mathieu-Daudé wrote: > diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c > index f69358a5ba..2781d8bd41 100644 > --- a/hw/arm/exynos4_boards.c > +++ b/hw/arm/exynos4_boards.c > @@ -131,8 +131,8 @@ exynos4_boards_init_common(MachineState *machine, > exynos4_boards_init_ram(s, get_system_memory(), > exynos4_board_ram_size[board_type]); > > - object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC); > - qdev_set_parent_bus(DEVICE(&s->soc), sysbus_get_default()); > + sysbus_init_child_obj(OBJECT(machine), "soc", > + &s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC); > object_property_set_bool(OBJECT(&s->soc), true, "realized", > &error_fatal); Thomas' comments re the commit message notwithstanding, the patch itself is correct. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Fri, 23 Aug 2019 at 16:26, Thomas Huth <thuth@redhat.com> wrote: > > On 8/23/19 4:32 PM, Philippe Mathieu-Daudé wrote: > > As explained in commit aff39be0ed97: > > > > Both functions, object_initialize() and qdev_set_parent_bus() > > Commit aff39be0ed97 was not about qdev_set_parent_bus(), so the first > sentence sounds somewhat misleading here. Maybe rephrase the commit > message without that reference to aff39be0ed97 ? > > > increase the reference counter of the new object, so one of the > > references has to be dropped afterwards to get the reference > > counting right. Otherwise the child object will not be properly > > cleaned up when the parent gets destroyed. > > Well, the parent here (the machine) currently never gets destroyed ... > so unless you've got a patch in your pipe to fix that, too, you should > maybe also rephrase this part of the commit message. How about just making the commit message: Both object_initialize() and qdev_set_parent_bus() increase the reference counter of the new object, so one of the references has to be dropped afterwards to get the reference counting right. In machine model code this refcount leak is not particularly problematic because (unlike devices) machines will never be created on demand via QMP, and they are never destroyed. But in any case let's use the new sysbus_init_child_obj() instead to get the reference counting here right. ? thanks -- PMM
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c index f69358a5ba..2781d8bd41 100644 --- a/hw/arm/exynos4_boards.c +++ b/hw/arm/exynos4_boards.c @@ -131,8 +131,8 @@ exynos4_boards_init_common(MachineState *machine, exynos4_boards_init_ram(s, get_system_memory(), exynos4_board_ram_size[board_type]); - object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC); - qdev_set_parent_bus(DEVICE(&s->soc), sysbus_get_default()); + sysbus_init_child_obj(OBJECT(machine), "soc", + &s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC); object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
As explained in commit aff39be0ed97: Both functions, object_initialize() and qdev_set_parent_bus() increase the reference counter of the new object, so one of the references has to be dropped afterwards to get the reference counting right. Otherwise the child object will not be properly cleaned up when the parent gets destroyed. Thus let's use now sysbus_init_child_obj() instead to get the reference counting here right. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- v2: Corrected commit description (pm215) --- hw/arm/exynos4_boards.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)