On Wed, Dec 06, 2023 at 03:49:28PM +0000, Miguel Luis wrote:
>
>
> > On 6 Dec 2023, at 14:25, Michal Suchánek <msucha...@suse.de> wrote:
> >
> > On Wed, Dec 06, 2023 at 01:17:08PM -0100, Miguel Luis wrote:
> >> 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?
> >
> > Doesn't it have the same problem that it will randomly free some context
> > which is not necessarily associated with the unplugged CPU?
> >
> > Consider machine with 4 CPUs, they are likely added in order - cpu0
> > getting context0, cpu1 context1, etc.
> >
> > Unplug CPU 1. Given that context 3 is top the would be unallocated by
> > the decrement, or am I missing something?
> >
>
> I think you’re right and I share of the same opinion that matching a tcg
> thread
> to a vCPU would be handy to solve this and maybe sorting tcg_ctxs after
> unregistering the thread.
Tried to apply the patch. It does not crash right away, and due to virsh
limitation I get only one (8-thread) core to hotplug so it did survive a few
hotplug cycles. After a while of hotplugging it crashed, anyway.
Given the atomic_dec there is probably no expectation that the
processing is sequential.
Thanks
Michal
