Re: [PATCH 3/3] plugins: avoid failing plugin when CPU is inited several times

[Emilio G. Cota]

Hi Alex,

On Tue, May 12, 2020 at 21:11:46 +0100, Alex Bennée wrote:
> 
> Emilio G. Cota <address@hidden> writes:
> 
> > On Mon, May 11, 2020 at 18:53:19 +0300, Nikolay Igotti wrote:
> >> Attached to the mail counter.c when running with attached test.c compiled
> >> to Linux standalone binary shows failing assert, unless the patch is
> >> applied.
> >
> > I didn't get the attachment. Can you paste the code at the end of your
> > reply?
> 
> So the problem is we are recycling cpu_index:
> 
>   21:01:54 [alex@zen:~/l/q/b/all.plugin] plugins/next|●2✚3…(+8 insertions(+)) 
> 127 + ./x86_64-linux-user/qemu-x86_64 
> ./tests/tcg/x86_64-linux-user/threadcount
>   cpu_list_add: 0x5605598579f0
>   cpu_get_free_index: 0
>   cpu_list_add: 0x5605598c6010
>   cpu_get_free_index: 1
>   cpu_list_add: 0x5605598ec330
>   cpu_get_free_index: 2
>   cpu_list_add: 0x560559912a30
>   cpu_get_free_index: 3
>   cpu_list_add: 0x560559939130
>   cpu_get_free_index: 4
>   cpu_list_add: 0x56055995d360
>   cpu_get_free_index: 1
>   **
>   
> ERROR:/home/alex/lsrc/qemu.git/plugins/core.c:205:qemu_plugin_vcpu_init_hook: 
> assertion failed: (success)
>   qemu:handle_cpu_signal received signal outside vCPU context @ 
> pc=0x7fa29c702613
> 
> However I'm confused as to how. The cpu_index is really just a function
> of the number of CPUs in the cpus list and I can't see where we free
> them up. We never call cpu_common_unrealizefn so the list should never
> shrink. We should just end up with a slowly growing list of now dead CPU
> states.
> 
> I'm giving up for now but will have a look again in the morning if no
> one else has figured out what is going on by then.

I just got this fixed, only to realise that you already fixed it weeks ago :-)
(I didn't see your fixes because I'm not subscribed to the list.)

My changes are here https://github.com/cota/qemu/tree/cpu_index
Please feel free to take anything from there that you find valuable, e.g.
the use of a bitmap for tracking cpu_index'es and the addition of a unit test
(note that the test is fixed in the last commit).

Thanks,

                Emilio