linux-kernel - Re: [PATCH 09/10] perf tools: Compute other 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200628215957.GO2988321@krava>
Date:   Sun, 28 Jun 2020 23:59:57 +0200
From:   Jiri Olsa <jolsa@...hat.com>
To:     Ian Rogers <irogers@...gle.com>
Cc:     Jiri Olsa <jolsa@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Michael Petlan <mpetlan@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Kajol Jain <kjain@...ux.ibm.com>,
        John Garry <john.garry@...wei.com>,
        "Paul A. Clarke" <pc@...ibm.com>,
        Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH 09/10] perf tools: Compute other metrics

On Fri, Jun 26, 2020 at 02:24:38PM -0700, Ian Rogers wrote:

SNIP

> > +
> > +                                       if (expr__get_id(ctx, lookup, &data) || !data) {
> >                                                 pr_debug("%s not found\n", $1);
> >                                                 free($1);
> >                                                 YYABORT;
> >                                         }
> > +
> > +                                       pr_debug2("lookup: is_other %d, counted %d: %s\n",
> > +                                                 data->is_other, data->other.counted, lookup);
> > +
> > +                                       if (data->is_other && !data->other.counted) {
> > +                                               data->other.counted = true;
> > +                                               if (expr__parse(&data->val, ctx, data->other.metric_expr, 1)) {
> 
> Ah, so this handles the problem the referenced metric isn't calculated
> and calculates it - with the sharing of events this doesn't impose
> extra pmu cost. Do we need to worry about detecting recursion? For
> example:
> 
> "MetricName": "Foo",
> "MetricExpr": "1/metric:Foo",

right, we should add some recursion check,
I'll lcheck on it

> 
> It seems unfortunate to have the MetricExpr calculated twice, but it

hum, not sure what you mean by twice.. we do that just once for
each involved metric and store the value.. the metric is also
processed before for 'other' metrics

jirka

> is understandable. Is it also a property that referenced/other metrics
> won't be reported individually? Perhaps these are sub-metrics?

> 
> Thanks,
> Ian
> 
> > +                                                       pr_debug("%s failed to count\n", $1);
> > +                                                       free($1);
> > +                                                       YYABORT;
> > +                                               }
> > +                                       }
> > +
> >                                         $$ = data->val;
> >                                         free($1);
> >                                 }
> > --
> > 2.25.4
> >
>