Miguel Luis <miguel.l...@oracle.com> writes:
> Hi!
>
> On 04/12/2023 18:40, Philippe Mathieu-Daudé wrote:
>> Unplugging vCPU triggers the following assertion in
>> tcg_register_thread():
>>
>> 796 void tcg_register_thread(void)
>> 797 {
>> ...
>> 812 /* Claim an entry in tcg_ctxs */
>> 813 n = qatomic_fetch_inc(&tcg_cur_ctxs);
>> 814 g_assert(n < tcg_max_ctxs);
>>
>> Implement and use tcg_unregister_thread() so when a
>> vCPU is unplugged, the tcg_cur_ctxs refcount is
>> decremented.
>
>
> I've had addressed this issue before (posted at [1]) and had exercised
> it with vCPU hotplug/unplug patches for ARM although I was not sure about what
> would be needed to be done regarding plugins on the context of
> tcg_unregister_thread. I guess they would need to be freed also?
Good catch. They should indeed.
>
>
>> Reported-by: Michal Suchánek <msucha...@suse.de>
>> Suggested-by: Stefan Hajnoczi <stefa...@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
>> ---
<snip>
>> void tcg_register_thread(void)
>> {
>> TCGContext *s = g_malloc(sizeof(*s));
>> @@ -814,6 +821,16 @@ void tcg_register_thread(void)
>>
>> tcg_ctx = s;
>> }
>> +
>> +void tcg_unregister_thread(void)
>> +{
>> + unsigned int n;
>> +
>> + n = qatomic_fetch_dec(&tcg_cur_ctxs);
>> + g_free(tcg_ctxs[n]);
Perhaps a bit of re-factoring and we could have a tcg_alloc_context and
tcg_free_context to keep everything together?
>
>
> Is the above off-by-one?
>
>
>> + qatomic_set(&tcg_ctxs[n], NULL);
>> +}
>> +
>
> Thank you
>
> Miguel
>
> [1]:
> https://lore.kernel.org/qemu-devel/20230926103654.34424-5-salil.me...@huawei.com/
>
>
>> #endif /* !CONFIG_USER_ONLY */
>>
>> /* pool based memory allocation */
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
