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

From: Jiri Olsa
Date: Tue Jan 28 2020 - 04:22:02 EST


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