Re: [PATCH v1 3/9] perf util: Reconstruct rblist for supporting per-thread shadow stats

["Jin, Yao" <yao.jin@linux.intel.com>]

On 11/21/2017 11:17 PM, Jiri Olsa wrote:
> On Mon, Nov 20, 2017 at 10:43:38PM +0800, Jin Yao wrote:
>> In current stat-shadow.c, the rblist deleting is ignored.
>>
>> The patch reconstructs the codes of rblist init/free, and adds
>> the implementation to node_delete method of rblist.
>>
>> This patch also does:
>>
>> 1. Add the ctx/type/stat into rbtree keys because we will
>> use this rbtree to maintain shadow metrics to replace original
>> a couple of static arrays for supporting per-thread shadow stats.
>>
>> 2. Create a static runtime_stat variable 'rt_stat' which
>> will log the shadow metrics by default.
>>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>> ---
>>   tools/perf/util/stat-shadow.c | 62 ++++++++++++++++++++++++++++++++++++++++---
>>   tools/perf/util/stat.h        |  2 ++
>>   2 files changed, 60 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
>> index 5853901..045e129 100644
>> --- a/tools/perf/util/stat-shadow.c
>> +++ b/tools/perf/util/stat-shadow.c
>> @@ -40,12 +40,16 @@ static struct stats runtime_aperf_stats[NUM_CTX][MAX_NR_CPUS];
>>   static struct rblist runtime_saved_values;
>>   static bool have_frontend_stalled;
>>   
>> +static struct runtime_stat rt_stat;
>>   struct stats walltime_nsecs_stats;
>>   
>>   struct saved_value {
>>   	struct rb_node rb_node;
>>   	struct perf_evsel *evsel;
>> +	enum stat_type type;
>> +	int ctx;
>>   	int cpu;
>> +	struct runtime_stat *stat;
>>   	struct stats stats;
>>   };
>>   
>> @@ -58,6 +62,23 @@ static int saved_value_cmp(struct rb_node *rb_node, const void *entry)
>>   
>>   	if (a->cpu != b->cpu)
>>   		return a->cpu - b->cpu;
>> +
>> +	if (a->type != b->type)
>> +		return a->type - b->type;
>> +
>> +	if (a->ctx != b->ctx)
>> +		return a->ctx - b->ctx;
>> +
> 
> could you please comment in here on the cases where
> evsel is defined and when not
> 
> jirka
> 

Thanks for reminding.

I will add comments to describe the cases where and when the evsel is 
set or not.

Thanks
Jin Yao

>> +	if (a->evsel == NULL && b->evsel == NULL) {
>> +		if (a->stat == b->stat)
>> +			return 0;
>> +
>> +		if ((char *)a->stat < (char *)b->stat)
>> +			return -1;
>> +
>> +		return 1;
>> +	}
>> +
> 
> SNIP
>