Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1661160 > unrolled thread

Re: [PATCH] perf, tools, script: Allow adding and removing fields

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] perf, tools, script: Allow adding and removing fields Milian Wolff <milian.wolff@kdab.com> - 2017-06-08 15:10 +0200
    Re: [PATCH] perf, tools, script: Allow adding and removing fields Andi Kleen <ak@linux.intel.com> - 2017-06-09 05:00 +0200
      Re: [PATCH] perf, tools, script: Allow adding and removing fields Milian Wolff <milian.wolff@kdab.com> - 2017-06-09 11:20 +0200
        Re: [PATCH] perf, tools, script: Allow adding and removing fields Andi Kleen <andi@firstfloor.org> - 2017-06-11 21:10 +0200

#1661160 — Re: [PATCH] perf, tools, script: Allow adding and removing fields

[Multipart message — attachments visible in raw view] — view raw

On Friday, June 2, 2017 5:48:10 PM CEST Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> With perf script it is common that we just want to add or remove a field.
> Currently this requires figuring out the long list of default fields and
> specifying them first, and then adding/removing the new field.
> 
> This patch adds a new + - syntax to merely add or remove fields,
> that allows more succint and clearer command lines
> 
> For example to remove the comm field from PMU samples:
> 
> Previously
> 
> perf script -F tid,cpu,time,event,sym,ip,dso,period
> 	    0 [000] 504345.383126:          1 cycles:  ffffffff90060c66
> native_write_msr ([kernel.kallsyms])
> 
> with the new syntax
> 
> perf script -F -comm
> 	    0 [000] 504345.383126:          1 cycles:  ffffffff90060c66
> native_write_msr ([kernel.kallsyms])
> 
> The new syntax cannot be mixed with normal overriding.

Tested-by: Milian Wolff <milian.wolff@kdab.com>

Works a charm.

But I notice that this functionality is missing in other places too. Most 
notably, I would like to be able to configure `perf stat` in a similar way. 
Such that one could do:

perf stat -e +cache-misses

Instead of

perf stat -e <whatever the defaults are>,cache-misses

While at it, I also think it would be tremendously useful to get wildcard 
matching behavior in all perf tools. Currently, I can do:

perf list "topdown-*"

List of pre-defined events (to be used in -e):

  topdown-fetch-bubbles OR cpu/topdown-fetch-bubbles/ [Kernel PMU event]
  topdown-recovery-bubbles OR cpu/topdown-recovery-bubbles/ [Kernel PMU event]
  topdown-slots-issued OR cpu/topdown-slots-issued/  [Kernel PMU event]
  topdown-slots-retired OR cpu/topdown-slots-retired/ [Kernel PMU event]
  topdown-total-slots OR cpu/topdown-total-slots/    [Kernel PMU event]

But I cannot do:

$ perf record -e "topdown-*" ls
event syntax error: 'topdown-*'
                            \___ parser error
Run 'perf list' for a list of valid events

But of course I'm aware that these are all separate issues. But Andi, you seem 
to be working a lot on polishing perf. I'd really appreciate it if you could 
also implement the above two suggestions. Otherwise I'll try to see when I get 
the time to do it myself.

Cheers

-- 
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

[toc] | [next] | [standalone]

#1661884

On Thu, Jun 08, 2017 at 02:59:58PM +0200, Milian Wolff wrote:
> But I notice that this functionality is missing in other places too. Most 
> notably, I would like to be able to configure `perf stat` in a similar way. 
> Such that one could do:
> 
> perf stat -e +cache-misses
> 
> Instead of
> 
> perf stat -e <whatever the defaults are>,cache-misses

The defaults are not great, so I'm not sure that is super useful.

It's probably better to assemble reasonable groups, perhaps
with groups of metrics.

> But I cannot do:
> 
> $ perf record -e "topdown-*" ls
> event syntax error: 'topdown-*'

That's actually good because the current topdown events are not useful to sample

Usually you need to have at least some idea about the events you're collecting,
and also for non trivial collections you need groups to get good results.

I've been thinking about adding MetricGroups to the json files, that
would allow to assemble reasonable groups. But it still wouldn't be wildcard.

For a few things wildcards are useful, e.g. I implemented it recently
for PMUs so that uncore PMUs are easier to handle.

-Andi

[toc] | [prev] | [next] | [standalone]

#1662146

On Freitag, 9. Juni 2017 04:52:43 CEST Andi Kleen wrote:
> On Thu, Jun 08, 2017 at 02:59:58PM +0200, Milian Wolff wrote:
> > But I notice that this functionality is missing in other places too. Most
> > notably, I would like to be able to configure `perf stat` in a similar
> > way.
> > Such that one could do:
> > 
> > perf stat -e +cache-misses
> > 
> > Instead of
> > 
> > perf stat -e <whatever the defaults are>,cache-misses
> 
> The defaults are not great, so I'm not sure that is super useful.
> 
> It's probably better to assemble reasonable groups, perhaps
> with groups of metrics.
> 
> > But I cannot do:
> > 
> > $ perf record -e "topdown-*" ls
> > event syntax error: 'topdown-*'
> 
> That's actually good because the current topdown events are not useful to
> sample

Can you elaborate? I assume it's because you actually want to sample on 
instructions, and then group it together with the topdown events and 
potentially other counters like instructions?

> Usually you need to have at least some idea about the events you're
> collecting, and also for non trivial collections you need groups to get
> good results.

Yes, sure. But replace `record` with `stat` in the above and my point still 
stands.

> I've been thinking about adding MetricGroups to the json files, that
> would allow to assemble reasonable groups. But it still wouldn't be
> wildcard.
> 
> For a few things wildcards are useful, e.g. I implemented it recently
> for PMUs so that uncore PMUs are easier to handle.

I just noticed that I can actually use wildcards for tracepoints:

perf trace --no-syscalls --event "ext4:*"

And I think the same should be doable for PMU events with perf stat, but 
currently isn't:

$ perf stat -e "topdown*" ls
invalid or unsupported event: 'topdown*'
$ perf stat -e "branch*" ls
invalid or unsupported event: 'branch*'
$ perf stat -e "cache*" ls
invalid or unsupported event: 'cache*'

Bye

-- 
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

[toc] | [prev] | [next] | [standalone]

#1663136

On Fri, Jun 09, 2017 at 11:13:11AM +0200, Milian Wolff wrote:
> > > But I cannot do:
> > > 
> > > $ perf record -e "topdown-*" ls
> > > event syntax error: 'topdown-*'
> > 
> > That's actually good because the current topdown events are not useful to
> > sample
> 
> Can you elaborate? I assume it's because you actually want to sample on 
> instructions, and then group it together with the topdown events and 
> potentially other counters like instructions?

The topdown-* events are inputs to a formula. But you cannot directly
sample for the formula.

What you can do is to compute the formuals from counts, determine 
the bottlenecks and then sample for events which look for the
bottlebeck. For example FRONTEND_* for Frontend Bound.
These events are generally different.

toplev in pmu-tools implements this automatically, but it's a bit
too complicated for standard perf.

> 
> > Usually you need to have at least some idea about the events you're
> > collecting, and also for non trivial collections you need groups to get
> > good results.
> 
> Yes, sure. But replace `record` with `stat` in the above and my point still 
> stands

The comment was for stat.

-Andi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web