Re: [PATCH v1 08/13] plugins: expand the bb plugin to be thread safe and track per-cpu

[Emilio G. Cota]

On Thu, Jul 09, 2020 at 15:13:22 +0100, Alex Bennée wrote:
> While there isn't any easy way to make the inline counts thread safe

Why not? At least in 64-bit hosts TCG will emit a single write to
update the 64-bit counter.

> we can ensure the callback based ones are. While we are at it we can
> reduce introduce a new option ("idle") to dump a report of the current

s/reduce//

> bb and insn count each time a vCPU enters the idle state.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Dave Bort <dbort@dbort.com>
> 
> ---
> v2
>   - fixup for non-inline linux-user case
>   - minor cleanup and re-factor
> ---
>  tests/plugin/bb.c | 96 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 83 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
> index df19fd359df3..89c373e19cd8 100644
> --- a/tests/plugin/bb.c
> +++ b/tests/plugin/bb.c
> @@ -16,24 +16,67 @@
>  
>  QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>  
> -static uint64_t bb_count;
> -static uint64_t insn_count;
> +typedef struct {
> +    GMutex lock;
> +    int index;
> +    uint64_t bb_count;
> +    uint64_t insn_count;
> +} CPUCount;

Why use a mutex?

Just have a per-vCPU struct that each vCPU thread updates with atomic_write.
Then when we want to print a report we just have to collect the counts
with atomic_read().

Also, consider just adding a comment to bb.c noting that it is not thread-safe,
and having a separate bb-threadsafe.c plugin for patch. The reason is that bb.c 
is
very simple, which is useful to understand the interface.

Thanks,
                E.