lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 9 Jan 2020 09:48:22 +0100 From: Jiri Olsa <jolsa@...hat.com> To: Andres Freund <andres@...razel.de> Cc: linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...nel.org>, Andi Kleen <ak@...ux.intel.com>, Arnaldo Carvalho de Melo <acme@...hat.com>, Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Michael Petlan <mpetlan@...hat.com>, Namhyung Kim <namhyung@...nel.org>, Peter Zijlstra <peterz@...radead.org>, stable@...r.kernel.org Subject: Re: [PATCH] perf c2c: Fix sorting. On Wed, Jan 08, 2020 at 08:30:30PM -0800, Andres Freund wrote: > Commit 722ddfde366f ("perf tools: Fix time sorting") changed - > correctly so - hist_entry__sort to return int64. Unfortunately several > of the builtin-c2c.c comparison routines only happened to work due the > cast caused by the wrong return type. > > This causes meaningless ordering of both the cacheline list, and the > cacheline details page. E.g a simple > perf c2c record -a sleep 3 > perf c2c report > will result in cacheline table like > ================================================= > Shared Data Cache Line Table > ================================================= > # > # ----------- Cacheline ---------- Total Tot ----- LLC Load Hitm ----- ---- Store Reference ---- --- Load Dram ---- LLC Total ----- Core Load Hit ----- -- LLC Load Hit -- > # Index Address Node PA cnt records Hitm Total Lcl Rmt Total L1Hit L1Miss Lcl Rmt Ld Miss Loads FB L1 L2 Llc Rmt > # ..... .................. .... ...... ....... ....... ....... ....... ....... ....... ....... ....... ........ ........ ....... ....... ....... ....... ....... ........ ........ > # > 0 0x7f0d27ffba00 N/A 0 52 0.12% 13 6 7 12 12 0 0 7 14 40 4 16 0 0 0 > 1 0x7f0d27ff61c0 N/A 0 6353 14.04% 1475 801 674 779 779 0 0 718 1392 5574 1299 1967 0 115 0 > 2 0x7f0d26d3ec80 N/A 0 71 0.15% 16 4 12 13 13 0 0 12 24 58 1 20 0 9 0 > 3 0x7f0d26d3ec00 N/A 0 98 0.22% 23 17 6 19 19 0 0 6 12 79 0 40 0 10 0 > i.e. with the list not being ordered by Total Hitm. > > Fixes: 722ddfde366f ("perf tools: Fix time sorting") > Signed-off-by: Andres Freund <andres@...razel.de> > Cc: Jiri Olsa <jolsa@...nel.org> > Cc: Andi Kleen <ak@...ux.intel.com> > Cc: Arnaldo Carvalho de Melo <acme@...hat.com> > Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com> > Cc: Michael Petlan <mpetlan@...hat.com> > Cc: Namhyung Kim <namhyung@...nel.org> > Cc: Peter Zijlstra <peterz@...radead.org> > Cc: stable@...r.kernel.org # v3.16+ > --- > tools/perf/builtin-c2c.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c > index e69f44941aad..f2e9d2b1b913 100644 > --- a/tools/perf/builtin-c2c.c > +++ b/tools/perf/builtin-c2c.c > @@ -595,8 +595,8 @@ tot_hitm_cmp(struct perf_hpp_fmt *fmt __maybe_unused, > { > struct c2c_hist_entry *c2c_left; > struct c2c_hist_entry *c2c_right; > - unsigned int tot_hitm_left; > - unsigned int tot_hitm_right; > + uint64_t tot_hitm_left; > + uint64_t tot_hitm_right; that change looks right, but I can't see how that could happened because of change in Fixes: tag was the return statement of this function: return tot_hitm_left - tot_hitm_right; considered to be 'unsigned int' and then converted to int64_t, which would treat negative 'unsigned int' as big positive 'int64_t'? thanks, jirka > > c2c_left = container_of(left, struct c2c_hist_entry, he); > c2c_right = container_of(right, struct c2c_hist_entry, he); > @@ -629,7 +629,8 @@ __f ## _cmp(struct perf_hpp_fmt *fmt __maybe_unused, \ > \ > c2c_left = container_of(left, struct c2c_hist_entry, he); \ > c2c_right = container_of(right, struct c2c_hist_entry, he); \ > - return c2c_left->stats.__f - c2c_right->stats.__f; \ > + return (uint64_t) c2c_left->stats.__f - \ > + (uint64_t) c2c_right->stats.__f; \ > } > > #define STAT_FN(__f) \ > @@ -682,7 +683,8 @@ ld_llcmiss_cmp(struct perf_hpp_fmt *fmt __maybe_unused, > c2c_left = container_of(left, struct c2c_hist_entry, he); > c2c_right = container_of(right, struct c2c_hist_entry, he); > > - return llc_miss(&c2c_left->stats) - llc_miss(&c2c_right->stats); > + return (uint64_t) llc_miss(&c2c_left->stats) - > + (uint64_t) llc_miss(&c2c_right->stats); > } > > static uint64_t total_records(struct c2c_stats *stats) > -- > 2.25.0.rc1 >
Powered by blists - more mailing lists