linux-kernel - Re: [PATCH v2 01/11] perf c2c: Add dimensions for total load hit

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210111074945.GC3241@leoy-ThinkPad-X240s>
Date:   Mon, 11 Jan 2021 15:49:45 +0800
From:   Leo Yan <leo.yan@...aro.org>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jiri Olsa <jolsa@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Ian Rogers <irogers@...gle.com>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Joe Mario <jmario@...hat.com>, David Ahern <dsahern@...il.com>,
        Don Zickus <dzickus@...hat.com>, Al Grant <Al.Grant@....com>,
        James Clark <james.clark@....com>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 01/11] perf c2c: Add dimensions for total load hit

Hi Namhyung,

On Wed, Jan 06, 2021 at 04:38:01PM +0900, Namhyung Kim wrote:
> Hi,
> 
> On Sun, Dec 13, 2020 at 10:39 PM Leo Yan <leo.yan@...aro.org> wrote:
> >
> > Arm SPE trace data doesn't support HITM, but we still want to explore
> > "perf c2c" tool to analyze cache false sharing.  If without HITM tag,
> > the tool cannot give out accurate result for cache false sharing, a
> > candidate solution is to sort the total load operations and connect with
> > the threads info, e.g. if multiple threads hit the same cache line for
> > many times, this can give out the hint that it's likely to cause cache
> > false sharing issue.
> >
> > Unlike having HITM tag, the proposed solution is not accurate and might
> > introduce false positive reporting, but it's a pragmatic approach for
> > detecting false sharing if memory event doesn't support HITM.
> >
> > To sort with the cache line hit, this patch adds dimensions for total
> > load hit and the associated percentage calculation.
> >
> > Signed-off-by: Leo Yan <leo.yan@...aro.org>
> > ---
> >  tools/perf/builtin-c2c.c | 112 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 112 insertions(+)
> >
> > diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> > index c5babeaa3b38..3d5a2dc8b4fd 100644
> > --- a/tools/perf/builtin-c2c.c
> > +++ b/tools/perf/builtin-c2c.c
> > @@ -615,6 +615,47 @@ tot_hitm_cmp(struct perf_hpp_fmt *fmt __maybe_unused,
> >         return tot_hitm_left - tot_hitm_right;
> >  }
> >
> > +#define TOT_LD_HIT(stats)              \
> > +       ((stats)->ld_fbhit +            \
> > +        (stats)->ld_l1hit +            \
> > +        (stats)->ld_l2hit +            \
> > +        (stats)->ld_llchit +           \
> > +        (stats)->lcl_hitm +            \
> > +        (stats)->rmt_hitm +            \
> > +        (stats)->rmt_hit)
> 
> It doesn't need to be a macro, why not use a static inline function?

Yes, will change to use static inline function.

As explained with Jiri, this patch series is mainly for Arm SPE, but
so far we have a known issue for store operation, thus the store
operation cannot be shown properly in the single cache view of perf
c2c tool.  For this reason, I will firstly send the refactoring patches
in next version, but your comments for patches 01, 02, 03, 10, 11 will
be addressed if later upstream them.

Thanks a lot for review,
Leo