| Message ID | 20190716140133.8578-1-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,for,4.1] linux-user: unparent CPU object before unref | expand |
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 <alex.bennee@linaro.org> 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: 1836558@bugs.launchpad.net > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > 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... thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> 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 <alex.bennee@linaro.org> 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: 1836558@bugs.launchpad.net >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> 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
On 7/16/19 4:01 PM, Alex Bennée 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 "finalize" > 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: 1836558@bugs.launchpad.net > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > 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(); >
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();
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: 1836558@bugs.launchpad.net Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- linux-user/syscall.c | 1 + 1 file changed, 1 insertion(+) -- 2.20.1