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

[Jin, Yao]

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?

Thanks
Jin Yao