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

[Leo Yan]

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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> >  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