[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 1/3] linux-user: un-parent OBJECT(cpu) when closing thread
From: |
Alex Bennée |
Subject: |
Re: [PULL 1/3] linux-user: un-parent OBJECT(cpu) when closing thread |
Date: |
Fri, 19 Aug 2022 09:37:38 +0100 |
User-agent: |
mu4e 1.8.9; emacs 28.1.91 |
Richard Henderson <richard.henderson@linaro.org> writes:
> On 8/16/22 05:26, Alex Bennée wrote:
>> While forcing the CPU to unrealize by hand does trigger the clean-up
>> code we never fully free resources because refcount never reaches
>> zero. This is because QOM automatically added objects without an
>> explicit parent to /unattached/, incrementing the refcount.
>> Instead of manually triggering unrealization just unparent the
>> object
>> and let the device machinery deal with that for us.
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/866
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> Message-Id: <20220811151413.3350684-2-alex.bennee@linaro.org>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index f409121202..bfdd60136b 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -8594,7 +8594,13 @@ static abi_long do_syscall1(CPUArchState *cpu_env,
>> int num, abi_long arg1,
>> if (CPU_NEXT(first_cpu)) {
>> TaskState *ts = cpu->opaque;
>> - object_property_set_bool(OBJECT(cpu), "realized",
>> false, NULL);
>> + if (ts->child_tidptr) {
>> + put_user_u32(0, ts->child_tidptr);
>> + do_sys_futex(g2h(cpu, ts->child_tidptr),
>> + FUTEX_WAKE, INT_MAX, NULL, NULL, 0);
>> + }
>> +
>> + object_unparent(OBJECT(cpu));
>
> This has caused a regression in arm/aarch64.
>
> We hard-code ARMCPRegInfo pointers into TranslationBlocks, for calling
> into helper_{get,set}cp_reg{,64}. So we have a race condition between
> whichever cpu thread translates the code first (encoding the pointer),
> and that cpu thread exiting, so that the next execution of the TB
> references a freed data structure.
What is the test case that breaks this? I guess a multi-threaded
sysregs.c would trigger it?
> We shouldn't have N copies of these pointers in the first place. This
> seems like something that ought to be placed on the ARMCPUClass, so
> that it could be shared by each cpu.
>
> But that's going to be a complex fix, so I'm reverting this for rc4.
>
>
> r~
--
Alex Bennée
[PULL 3/3] tests/avocado: apply a band aid to aspeed-evb login, Alex Bennée, 2022/08/16
Re: [PULL for 7.1 0/3] memory leak and testing tweaks, Richard Henderson, 2022/08/16