linux-kernel - Re: [PATCH v1 03/22] perf metrics: Don't scale counts going into metrics

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] [day] [month] [year] [list]

Message-ID: <CAP-5=fXnzHBuNORVCznEi6yhoJXXD6bVaevw6qygsGf8BgmiWA@mail.gmail.com>
Date:   Mon, 3 Oct 2022 18:34:14 -0700
From:   Ian Rogers <irogers@...gle.com>
To:     Xing Zhengjun <zhengjun.xing@...ux.intel.com>
Cc:     Kan Liang <kan.liang@...ux.intel.com>,
        Andi Kleen <ak@...ux.intel.com>, perry.taylor@...el.com,
        caleb.biggers@...el.com, kshipra.bopardikar@...el.com,
        samantha.alt@...el.com, ahmad.yasin@...el.com,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        John Garry <john.garry@...wei.com>,
        James Clark <james.clark@....com>,
        Kajol Jain <kjain@...ux.ibm.com>,
        Thomas Richter <tmricht@...ux.ibm.com>,
        Miaoqian Lin <linmq006@...il.com>,
        Florian Fischer <florian.fischer@...q.space>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v1 03/22] perf metrics: Don't scale counts going into metrics

On Thu, Sep 29, 2022 at 1:49 AM Xing Zhengjun
<zhengjun.xing@...ux.intel.com> wrote:
>
> On 9/28/2022 3:21 PM, Ian Rogers wrote:
> > Counts are scaled prior to going into saved_value, reverse the scaling
> > so that metrics don't double scale values.
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> >   tools/perf/util/stat-shadow.c | 9 +++++++--
> >   1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
> > index 9e1eddeff21b..b5cedd37588f 100644
> > --- a/tools/perf/util/stat-shadow.c
> > +++ b/tools/perf/util/stat-shadow.c
> > @@ -865,11 +865,16 @@ static int prepare_metric(struct evsel **metric_events,
> >                       if (!v)
> >                               break;
> >                       stats = &v->stats;
> > -                     scale = 1.0;
> > +                     /*
> > +                      * If an event was scaled during stat gathering, reverse
> > +                      * the scale before computing the metric.
> > +                      */
> > +                     scale = 1.0 / metric_events[i]->scale;
> > +
> This look likes not work for kernel side events like
> /sys/devices/uncore_imc_*/events/cas_count_read(write).

I've not been able to reproduce this. I've tried:

  {
    "MetricName": "IanTest",
    "MetricExpr": "uncore_imc@..._count_read@ * 6.103515625e-5 +
uncore_imc@..._count_write@ * 6.103515625e-5"
  }

where it scales the two counters and sums them. I see:

$ perf stat -M IanTest -a sleep 1

Performance counter stats for 'system wide':

           298.70 MiB  uncore_imc/cas_count_write/      #   721.00
IanTest
           429.64 MiB  uncore_imc/cas_count_read/

      1.004877710 seconds time elapsed

So the metric's value nearly matches the sum. If the scaled values had
been used the sum would have been something like 0.044454345703124995.

>From the code, this is the only place a metric's "ID" (event/counter)
is associated with a value, so I would be confused if there were a
code path where reversing the scaling wasn't happening.

Thanks,
Ian

> >                       source_count = evsel__source_count(metric_events[i]);
> >
> >                       if (v->metric_other)
> > -                             metric_total = v->metric_total;
> > +                             metric_total = v->metric_total * scale;
> >               }
> >               n = strdup(evsel__metric_id(metric_events[i]));
> >               if (!n)
>
> --
> Zhengjun Xing