Messages in this thread Patch in this message | | | From | Arnaldo Carvalho de Melo <> | Date | Fri, 31 Jan 2020 09:32:23 +0100 | Subject | Re: [PATCH v5 1/4] perf util: Move block_pair_cmp to block-info |
| |
Em Tue, Jan 28, 2020 at 08:55:53PM +0800, Jin Yao escreveu: > block_pair_cmp() is a function which is used to compare > two blocks. Moving it from builtin-diff.c to block-info.c > to let it can be used by other builtins. > > v4/v5: > ------ > No change. > > v3: > --- > Separate it from original patch for good tracking. > > Signed-off-by: Jin Yao <yao.jin@linux.intel.com> > --- > tools/perf/builtin-diff.c | 17 ----------------- > tools/perf/util/block-info.c | 17 +++++++++++++++++ > tools/perf/util/block-info.h | 2 ++ > 3 files changed, 19 insertions(+), 17 deletions(-) > > diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c > index f8b6ae557d8b..5ff1e21082cb 100644 > --- a/tools/perf/builtin-diff.c > +++ b/tools/perf/builtin-diff.c > @@ -572,23 +572,6 @@ static void init_block_hist(struct block_hist *bh) > bh->valid = true; > } > > -static int block_pair_cmp(struct hist_entry *a, struct hist_entry *b) > -{ > - struct block_info *bi_a = a->block_info; > - struct block_info *bi_b = b->block_info; > - int cmp; > - > - if (!bi_a->sym || !bi_b->sym) > - return -1; > - > - cmp = strcmp(bi_a->sym->name, bi_b->sym->name); > - > - if ((!cmp) && (bi_a->start == bi_b->start) && (bi_a->end == bi_b->end)) > - return 0; > - > - return -1; > -} > - > static struct hist_entry *get_block_pair(struct hist_entry *he, > struct hists *hists_pair) > { > diff --git a/tools/perf/util/block-info.c b/tools/perf/util/block-info.c > index c4b030bf6ec2..f0f38bdd496a 100644 > --- a/tools/perf/util/block-info.c > +++ b/tools/perf/util/block-info.c > @@ -475,3 +475,20 @@ float block_info__total_cycles_percent(struct hist_entry *he) > > return 0.0; > } > + > +int block_pair_cmp(struct hist_entry *a, struct hist_entry *b)
First thing that came to mind was that hist_entry comparision functions had been changed to return int64_t recently, when I went to look at it I found this:
tools/perf/util/block-info.c
int64_t block_info__cmp(struct perf_hpp_fmt *fmt __maybe_unused, struct hist_entry *left, struct hist_entry *right) { struct block_info *bi_l = left->block_info; struct block_info *bi_r = right->block_info; int cmp; . . .
Which look a bit more complete, can you check if that can be used instead or explain why my quick analysis of this is b0rken?
Perhaps we can have a __block_info__cmp() that doesn't receive the perf_hpp_fmt (that isn't even used above) so that the previous use of block_pair_cmp() can be replaced with __block_info__cmp() instead?
Thanks,
- Arnaldo
> +{ > + struct block_info *bi_a = a->block_info; > + struct block_info *bi_b = b->block_info; > + int cmp; > + > + if (!bi_a->sym || !bi_b->sym) > + return -1; > + > + cmp = strcmp(bi_a->sym->name, bi_b->sym->name); > + > + if ((!cmp) && (bi_a->start == bi_b->start) && (bi_a->end == bi_b->end)) > + return 0; > + > + return -1; > +} > diff --git a/tools/perf/util/block-info.h b/tools/perf/util/block-info.h > index bef0d75e9819..4fa91eeae92e 100644 > --- a/tools/perf/util/block-info.h > +++ b/tools/perf/util/block-info.h > @@ -76,4 +76,6 @@ int report__browse_block_hists(struct block_hist *bh, float min_percent, > > float block_info__total_cycles_percent(struct hist_entry *he); > > +int block_pair_cmp(struct hist_entry *a, struct hist_entry *b); > + > #endif /* __PERF_BLOCK_H */ > -- > 2.17.1 >
--
- Arnaldo
| |