Emilio G. Cota <c...@braap.org> writes:
> 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.
Well the operation is an add so that is a load/add/store cycle on
non-x86. If we want to do it properly we should expose an atomic add
operation which may be helper based or maybe generate an atomic
operation if the backend can support it easily.
>
>> 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.ben...@linaro.org>
>> Cc: Dave Bort <db...@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.
--
Alex Bennée
