Re: [Qemu-devel] [RFC PATCH for 4.1] linux-user: unparent CPU object before unref

[Alex Bennée]

Peter Maydell <address@hidden> writes:

> Ccing the QOM maintainers to make sure we have the
> QOM lifecycle operations right here...
>
> On Tue, 16 Jul 2019 at 15:02, Alex Bennée <address@hidden> wrote:
>>
>> When a CPU object is created it is parented during it's realize stage.
>> If we don't unparent before the "final" unref we will never finzalize
>> the object leading to a memory leak. For most setups you probably
>> won't notice but with anything that creates and destroys a lot of
>> threads this will add up. This goes especially for architectures which
>> allocate a lot of memory in their CPU structures.
>>
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1836558
>> Cc: address@hidden
>> Signed-off-by: Alex Bennée <address@hidden>
>> ---
>>  linux-user/syscall.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 39a37496fed..4c9313fd9d0 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7183,6 +7183,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
>> abi_long arg1,
>>                            NULL, NULL, 0);
>>              }
>>              thread_cpu = NULL;
>> +            object_unparent(OBJECT(cpu));
>>              object_unref(OBJECT(cpu));
>>              g_free(ts);
>>              rcu_unregister_thread();
>
> I think (as I mentioned on IRC) that we also need to unrealize
> the CPU object, because target/ppc at least does some freeing
> of memory in its unrealize method. I think we do that by
> setting the QOM "realize" property back to "false" -- but that
> might barf if we try it on a CPU that isn't hotpluggable...

I have tried:

             thread_cpu = NULL;
+            object_unparent(OBJECT(cpu));
+            object_property_set_bool(OBJECT(cpu), false, "realized", NULL);
             object_unref(OBJECT(cpu));

but it didn't manifestly change anything (i.e. both with and without
setting realized the thread allocated stuff is freed). Valgrind still
complains about:

==22483== 6,656 bytes in 26 blocks are possibly lost in loss record 1,639 of 
1,654
==22483==    at 0x483577F: malloc (vg_replace_malloc.c:299)
==22483==    by 0x4D7F8D0: g_malloc (in 
/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.5800.3)
==22483==    by 0x27D692: create_new_table (translate_init.inc.c:9252)
==22483==    by 0x27D7CD: register_ind_in_table (translate_init.inc.c:9291)
==22483==    by 0x27D975: register_dblind_insn (translate_init.inc.c:9337)
==22483==    by 0x27DBBB: register_insn (translate_init.inc.c:9384)
==22483==    by 0x27DE4E: create_ppc_opcodes (translate_init.inc.c:9449)
==22483==    by 0x27E79C: ppc_cpu_realize (translate_init.inc.c:9818)
==22483==    by 0x2C6FE8: device_set_realized (qdev.c:834)
==22483==    by 0x2D1E3D: property_set_bool (object.c:2076)
==22483==    by 0x2D00B3: object_property_set (object.c:1268)
==22483==    by 0x2D3185: object_property_set_qobject (qom-qobject.c:26)

But I don't know if that is just because the final exit_group of the
main CPU just exits without attempting to clean-up. However it is better
than it was.

--
Alex Bennée