linux-kernel - Re: [PATCH v2 5/5] perf metric: Don't compute unused events.

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

Message-ID: <20201120193501.GO1475102@krava>
Date:   Fri, 20 Nov 2020 20:35:01 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Ian Rogers <irogers@...gle.com>
Cc:     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>,
        Namhyung Kim <namhyung@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andi Kleen <ak@...ux.intel.com>,
        Jin Yao <yao.jin@...ux.intel.com>,
        John Garry <john.garry@...wei.com>,
        Paul Clarke <pc@...ibm.com>, kajoljain <kjain@...ux.ibm.com>,
        Stephane Eranian <eranian@...gle.com>,
        Sandeep Dasgupta <sdasgup@...gle.com>,
        linux-perf-users <linux-perf-users@...r.kernel.org>
Subject: Re: [PATCH v2 5/5] perf metric: Don't compute unused events.

On Thu, Nov 19, 2020 at 01:37:15PM -0800, Ian Rogers wrote:
> On Thu, Nov 19, 2020 at 12:59 PM Jiri Olsa <jolsa@...hat.com> wrote:
> 
> > On Tue, Nov 17, 2020 at 09:03:35PM -0800, Ian Rogers wrote:
> >
> > SNIP
> >
> > > +                     ids__free($1.ids);
> > > +                     ids__free($3.ids);
> > > +             }
> > > +     } else {
> > > +             $$.val = NAN;
> > > +             $$.ids = ids__union($1.ids, $3.ids);
> > > +     }
> > > +}
> > > +| expr '*' expr
> > > +{
> > > +     if (!compute_ids || (isfinite($1.val) && isfinite($3.val))) {
> > > +             $$.val = $1.val * $3.val;
> > > +             $$.ids = NULL;
> > > +             if (compute_ids) {
> > > +                     ids__free($1.ids);
> > > +                     ids__free($3.ids);

thanks for the explanation below, really nice, one more question ;-)

why do we need to call ids__free in here, for compute_ids and constants
case in here it should be always NULL, no?

> > > +             }
> > > +     } else {
> > > +             $$.val = NAN;
> > > +             $$.ids = ids__union($1.ids, $3.ids);
> > > +     }
> > > +}
> > > +| expr '/' expr
> > > +{
> > > +     if (fpclassify($3.val) == FP_ZERO) {
> > > +             pr_debug("division by zero\n");
> > > +             YYABORT;
> > > +     } else if (!compute_ids || (isfinite($1.val) && isfinite($3.val)))
> > {
> > > +             $$.val = $1.val / $3.val;
> > > +             $$.ids = NULL;
> >
> > hum, I'm confused with this.. compute_ids with finite values?
> > why do we erase ids then? also val should be NAN then, no?
> > could you please put in some comment with reasoning?
> >
> 
> Each expr parsing step needs to create a val and an ids. If we're not
> computing ids then we know we don't need ids. If the values are both
> constants (aka finite), again we don't need ids as  we can just divide. It
> is invariant that if you have ids then the value is NAN which isn't finite.
> So when computing ids we may want to constant evaluate:
> 
> event1 if 0.5 > 1.0/3.0 else event2
> 
> which is quite hypothetical but the idea is to have a general approach for
> all the operators. The simple lattice is something like:
> 
> Constants: 0.0, 1.0, ....
>                     \.    |.     /
> Bottom:         NAN
> 
> Bottom means we really have a set of values and we don't know which it
> could be. So:
> 
> 3.0 if event1 > 10.0 else 4.0
> 
> has possible values of 3.0 or 4.0 which we could represent with the set
> {3.0, 4.0}, but because the lattice is simple we just say bottom, meaning
> the set of all values - which is conservatively correct as 3.0 and 4.0 are
> in the set of all values. It would be incorrect to say the value was 3.0.
> Even with a simple lattice we could represent that:
> 
> 3.0 if event1 > 10.0 else 3.0
> 
> evaluates to 3.0 and so there is no need to measure event1. Note, this
> peephole optimization isn't performed here, just the peephole optimization
> that if the condition is true or false we can remove events.
> 
> The code in general needs to handle the compute_ids case and the evaluation
> case. So what the code is trying to do is propagate constants with sets of
> ids in the compute_ids case, or just evaluate in the other case. NAN is
> used as a bottom just for simplicity and to avoid inventing a type lattice
> abstraction.

could you please put it in comment, will be helpful for future ;-)

thanks,
jirka