Re: [PATCH v9 01/13] accel/tcg: introduce TBStatistics structure

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 10/7/19 11:28 AM, Alex Bennée wrote:
>> From: "Vanderson M. do Rosario" <vandersonmr2@gmail.com>
>> 
>> To store statistics for each TB, we created a TBStatistics structure
>> which is linked with the TBs. TBStatistics can stay alive after
>> tb_flush and be relinked to a regenerated TB. So the statistics can
>> be accumulated even through flushes.
>> 
>> The goal is to have all present and future qemu/tcg statistics and
>> meta-data stored in this new structure.
>> 
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>> Signed-off-by: Vanderson M. do Rosario <vandersonmr2@gmail.com>
>> Message-Id: <20190829173437.5926-2-vandersonmr2@gmail.com>
>> [AJB: fix git author, review comments]
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> 
>> ---
>> AJB
>>   - move tcg_collect_tb_stats inside tb-stats.c
>>   - add spdx header
>>   - drop tb from tbstats and associated functions
>> ---
>
> The only quibble I have is with
>
>> +void init_tb_stats_htable_if_not(void);
>
> If not what?
>
> I can only imagine that this phrase is intended to finish "if not initialized".
>  But I think it's clearer to just call this "init_tb_stats_htable".

Fixed.

>
>> +void enable_collect_tb_stats(void)
>> +{
>> +    init_tb_stats_htable_if_not();
>
> Why do we need to do this again, since we did this in tb_htable_init?

This is the route if we dynamically enable tb-stats with an already
running system emulation.

-- 
Alex Bennée