Re: [PATCH v4 3/4] perf util: Flexible to set block info output formats

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

On 1/28/2020 5:21 PM, Jiri Olsa wrote:
> On Mon, Jan 27, 2020 at 08:31:23PM +0800, Jin, Yao wrote:
>>
>>
>> On 1/20/2020 11:00 PM, Jin, Yao wrote:
>>>
>>>
>>> On 1/20/2020 5:47 PM, Jiri Olsa wrote:
>>>> On Thu, Jan 16, 2020 at 03:29:03AM +0800, Jin Yao wrote:
>>>>
>>>> SNIP
>>>>
>>>>> +                   block_hpps, nr_hpps);
>>>>> -    perf_hpp_list__register_sort_field(&bh->block_list,
>>>>> -        &block_fmts[PERF_HPP_REPORT__BLOCK_TOTAL_CYCLES_PCT].fmt);
>>>>> +    /* Sort by the first fmt */
>>>>> +    perf_hpp_list__register_sort_field(&bh->block_list,
>>>>> &block_fmts[0].fmt);
>>>>>    }
>>>>> -static void process_block_report(struct hists *hists,
>>>>> -                 struct block_report *block_report,
>>>>> -                 u64 total_cycles)
>>>>> +static int process_block_report(struct hists *hists,
>>>>> +                struct block_report *block_report,
>>>>> +                u64 total_cycles, int *block_hpps,
>>>>> +                int nr_hpps)
>>>>>    {
>>>>>        struct rb_node *next = rb_first_cached(&hists->entries);
>>>>>        struct block_hist *bh = &block_report->hist;
>>>>>        struct hist_entry *he;
>>>>> -    init_block_hist(bh, block_report->fmts);
>>>>> +    if (nr_hpps > PERF_HPP_REPORT__BLOCK_MAX_INDEX)
>>>>
>>>> hum, should be '>=' above.. ?
>>>>
>>>> jirka
>>>>
>>>
>>> '=' should be OK.
>>>
>>> enum {
>>>       PERF_HPP_REPORT__BLOCK_TOTAL_CYCLES_PCT,
>>>       PERF_HPP_REPORT__BLOCK_LBR_CYCLES,
>>>       PERF_HPP_REPORT__BLOCK_CYCLES_PCT,
>>>       PERF_HPP_REPORT__BLOCK_AVG_CYCLES,
>>>       PERF_HPP_REPORT__BLOCK_RANGE,
>>>       PERF_HPP_REPORT__BLOCK_DSO,
>>>       PERF_HPP_REPORT__BLOCK_MAX_INDEX
>>> };
>>>
>>> PERF_HPP_REPORT__BLOCK_MAX_INDEX is 6.
>>>
>>> If nr_hpps is 6, for example, block_hpps[] is,
>>>
>>>           int block_hpps[6] = {
>>>               PERF_HPP_REPORT__BLOCK_TOTAL_CYCLES_PCT,
>>>               PERF_HPP_REPORT__BLOCK_LBR_CYCLES,
>>>               PERF_HPP_REPORT__BLOCK_CYCLES_PCT,
>>>               PERF_HPP_REPORT__BLOCK_AVG_CYCLES,
>>>               PERF_HPP_REPORT__BLOCK_RANGE,
>>>               PERF_HPP_REPORT__BLOCK_DSO,
>>>           };
>>>
>>>           block_info__create_report(session->evlist,
>>>                         rep->total_cycles,
>>>                                             block_hpps, 6,
>>>                                             &rep->nr_block_reports);
>>>
>>> That should be legal.
>>>
>>> Thanks
>>> Jin Yao
>>
>> Hi Jiri,
>>
>> Does this explanation make sense?
> 
> ok, make sense
> 
> thanks,
> jirka
> 

Thanks Jiri!

I will post v5 which only contains the change of dropping the 
unnecessary release code.

Thanks
Jin Yao