linux-kernel - Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric

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: <20170208215103.GP26852@two.firstfloor.org>
Date:   Wed, 8 Feb 2017 13:51:05 -0800
From:   Andi Kleen <andi@...stfloor.org>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Andi Kleen <andi@...stfloor.org>, acme@...nel.org,
        jolsa@...nel.org, linux-kernel@...r.kernel.org,
        Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric

On Wed, Feb 08, 2017 at 12:31:34PM +0100, Jiri Olsa wrote:
> On Fri, Jan 27, 2017 at 06:03:45PM -0800, Andi Kleen wrote:
> > From: Andi Kleen <ak@...ux.intel.com>
> > 
> > Add generic infrastructure to perf stat to output ratios for "MetricExpr"
> > entries in the event lists. Many events are more useful as ratios
> > than in raw form, typically some count in relation to total ticks.
> > 
> > Transfer the MetricExpr information from the alias to the evsel.
> > 
> > We mark the events that need to be collected for MetricExpr, and also
> > link the events using them with a pointer. The code is careful
> > to always prefer the right event in the same group to minimize
> > multiplexing errors. At the moment only a single relation is supported.
> > 
> > Then add a rblist to the stat shadow code that remembers stats based
> > on the cpu and context.
> > 
> > Then finally update and retrieve and print these values similarly to the
> > existing hardcoded perf metrics. We use the simple expression parser
> > added earlier to evaluate the expression.
> > 
> > Normally we just output the result without further commentary,
> > but for --metric-only this would lead to empty columns. So for this
> > case use the original event as description.
> > 
> > So far there is no attempt to automatically add the MetricExpr event,
> > if it is missing, however we suggest it to the user.
> > 
> > $ perf stat -a -I 1000 -e '{unc_p_clockticks,unc_p_freq_max_os_cycles}'
> >      1.000228813        800,139,950      unc_p_clockticks
> >      1.000228813        789,833,783      unc_p_freq_max_os_cycles  #     98.7
> >      2.000654229        800,308,990      unc_p_clockticks
> >      2.000654229        396,214,238      unc_p_freq_max_os_cycles  #     49.5
> 
> [jolsa@...va perf]$ ./perf stat -a -I 1000 -e '{unc_p_clockticks,unc_p_freq_max_os_cycles}'
> invalid or unsupported event: '{unc_p_clockticks,unc_p_freq_max_os_cycles}'
> Run 'perf list' for a list of valid events
> 
> could you show an example of the MetricExpr?

It's in the event list branch

https://git.kernel.org/cgit/linux/kernel/git/ak/linux-misc.git/log/?h=perf/intel-uncore-json-files-3

All the metrics currently do is just the same as DividedBy earlier:
generate a percentage out of a count, typically based on clock ticks.

+        "MetricExpr": "(UNC_M_POWER_CHANNEL_PPD / UNC_M_CLOCKTICKS) *
100.",



> 
> it's part of the event record, what if you wanted to have 2 or more metrics defined for event?

Would need multiple copies of the event.

> 
> who defines those expressions?

It's metrics used internally by Intel.

> 
> what if you dont provide the necessary events needed for the expression?

Then perf prints a warning suggesting the events.

It's currently a TODO to add them automatically. Could be added,
but the patch was already complex, so I didn't add it.

It's somewhat complicated because you would need to avoid
duplicates and have to handle groups correctly. perf stat
doesn't have the necessarily knowledge to fully understand
the constraints on groups. 

Then the extra event may not fit into the group, and it seemed
saner to let the user decide what to do then, instead of
generating a possible unschedulable group.

So it's not that easy to do it automatically.

> > 
> > $ perf stat -a -I 1000 -e '{unc_p_clockticks,unc_p_freq_max_os_cycles}' --metric-only
> >      1.000206740     48.0
> >      2.000451543     48.1
> 
> how does user know what those number stand for?

I assume it was obvious enough that it is the percentage. Could
add something more to the event description.

> 
> is there a way for user to display the metric expression?

Only in the source, but could be added to perf list -v

For most users metrics are much more useful than raw numbers.
I think they would rather consider raw numbers to be a "hack
with no concept"

-Andi