linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andi Kleen <ak@linux.intel.com>
To: Ian Rogers <irogers@google.com>
Cc: namhyung@kernel.org, linux-perf-users@vger.kernel.org, acme@kernel.org
Subject: Re: Perf stat regression from d15480a3d67
Date: Thu, 22 Jun 2023 02:20:16 -0700	[thread overview]
Message-ID: <ZJQSUHLe9Cqpw21n@tassilo> (raw)
In-Reply-To: <CAP-5=fWZAcXcE3CP-V7t=gSehO-AacHJWhjHjtA5nCd5B2-n1A@mail.gmail.com>

> Can you provide an example that is breaking?

I only have a very long auto generated example[1], because it's not
easy to make a non scheduling event and it's also dependent on the system. The example 
works on a SKX.

> 
> Wrt event order, cases like:
> ```
> $ sudo perf stat -e '{data_read,data_write}' -a --no-merge sleep 1
> 
> Performance counter stats for 'system wide':
> 
>          1,960.81 MiB  data_read [uncore_imc_free_running_1]
>            438.48 MiB  data_write [uncore_imc_free_running_1]
>          1,961.85 MiB  data_read [uncore_imc_free_running_0]
>            438.79 MiB  data_write [uncore_imc_free_running_0]
> 
>       1.001127356 seconds time elapsed
> ```
> Reorder events so that the PMUs match (ie the -e order would 2x
> data_read followed by 2x data_write). This covers more cases now so
> that perf metric (aka topdown) events aren't broken when coming out of
> metrics:
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/parse-events.c?h=perf-tools-next#n2139

How is any tool supposed to handle arbitary output reordering like
that? Perf is not just for humans, but also for tools.

It cannot just look up the events by name because there might be multiple
instances of the same event in different groups.

The only reliable way to match is to match on the original order,
but that totally breaks down with these changes.

The same actually applies to humans reading the output too,
if it's ambigious for the tool it will be ambigious to the human too.
perf stat shouldn't reorder events ever.

> 
> Given the original commit message there seems little point to give the
> 0 counts, but perhaps it is something that could be done on the more
> tool oriented CSV and JSON output formats.

It's not 0 counts, it's <not supported>. But even 0 counts can 
be important.

We should always output when something doesn't count so that
the user knows something is wrong. Hiding problems is always a bad idea
and not an improvement.


-Andi

[1] Long example for SKX:

perf stat -o p3 -x\; -e '{cpu/event=0xc0,umask=0x0/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x0,umask=0x3/u,cpu/event=0x3c,umask=0x2/u,cpu/event=0x3c,umask=0x1/u,cpu/event=0xb1,umask=0x1/u,cpu/event=0xb1,umask=0x2,cmask=1/u},msr/tsc/,duration_time,dummy,uncore_imc/event=0x4,umask=0x3/,uncore_imc/event=0x4,umask=0xc/,cs:u,minor-faults:u,major-faults:u,migrations:u,{cycles:u,cpu/event=0x0,umask=0x3/u,cpu/event=0xc2,umask=0x2/u,cpu/event=0xc0,umask=0x0/u,cpu/event=0xc4,umask=0x40/u,cpu/event=0xb1,umask=0x1/u,cpu/event=0xe,umask=0x1/u},{cpu/event=0xc2,umask=0x2/u,cpu/event=0xc2,umask=0x2,cmask=1/u,cpu/event=0x3c,umask=0x0/ku,cpu/event=0xc0,umask=0x0/ku},{cpu/event=0x79,umask=0x8/u,cpu/event=0xa8,umask=0x1/u,cpu/event=0x79,umask=0x4/u,cpu/event=0x79,umask=0x30/u,cpu/event=0xc0,umask=0x0/u,cpu/event=0x3c,umask=0x0/u},{cpu/event=0x10,umask=0x20/u,cpu/event=0x10,umask=0x80/u,cpu/event=0x10,umask=0x10/u,cpu/event=0x10,umask=0x40/u},{cpu/event=0x11,umask=0x2/u,cpu/event=0x11,umask=0x1/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u,cpu/event=0x3c,umask=0x1/u},{cpu/event=0x10,umask=0x20/u,cpu/event=0x10,umask=0x80/u,cpu/event=0x10,umask=0x10/u,cpu/event=0x10,umask=0x40/u,cpu/event=0x3c,umask=0x0/u},{cpu/event=0x3c,umask=0x0/ku,cpu/event=0x3c,umask=0x1/u,cpu/event=0xc2,umask=0x2/u,cpu/event=0xd,umask=0x3,cmask=1/u},{cpu/event=0x9c,umask=0x1/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u,cpu/event=0x3c,umask=0x1/u,cpu/event=0xc2,umask=0x2/u},{cpu/event=0xe,umask=0x1/u,cpu/event=0xc2,umask=0x2/u,cpu/event=0xd,umask=0x3,cmask=1/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u},{cpu/event=0x9c,umask=0x1/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u,cpu/event=0x3c,umask=0x1/u,cpu/event=0xe,umask=0x1/u},{cpu/event=0x3c,umask=0x0/u,cpu/event=0x9c,umask=0x1,cmask=4/u,cpu/event=0x3c,umask=0x2/u,cpu/event=0x3c,umask=0x1/u,cpu/event=0x9c,umask=0x1/u,cpu/event=0xc0,umask=0x0/u},{cpu/event=0xc2,umask=0x2/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u,cpu/event=0x3c,umask=0x1/u,cpu/event=0xd,umask=0x3,cmask=1/u},{cpu/event=0xc2,umask=0x2/u,cpu/event=0xe,umask=0x1/u,cpu/event=0x79,umask=0x30/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u},{cpu/event=0xe,umask=0x1/u,cpu/event=0xc2,umask=0x2/u,cpu/event=0xd,umask=0x3,cmask=1/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u},{cpu/event=0xc5,umask=0x0/u,cpu/event=0xc3,umask=0x1,edge=1,cmask=1/u,cpu/event=0x3c,umask=0x1/u},dummy,{cpu/event=0xe,umask=0x1/u,cpu/event=0xc2,umask=0x2/u,cpu/event=0xd,umask=0x3,cmask=1/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u},{cpu/event=0x9c,umask=0x1/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u,cpu/event=0x3c,umask=0x1/u,cpu/event=0xe,umask=0x1/u},dummy,{cpu/event=0xa3,umask=0x6,cmask=6/u,cpu/event=0xa2,umask=0x8/u,cpu/event=0xa3,umask=0x4,cmask=4/u,cpu/event=0xb1,umask=0x1,cmask=1/u},{cpu/event=0xb1,umask=0x1,cmask=3/u,cpu/event=0xb1,umask=0x1,cmask=2/u,cpu/event=0xc0,umask=0x0/u,cpu/event=0x5e,umask=0x1/u,cpu/event=0x9c,umask=0x1,cmask=4/u},{cpu/event=0x9c,umask=0x1/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u,cpu/event=0x3c,umask=0x1/u,cpu/event=0xe,umask=0x1/u},{cpu/event=0xc2,umask=0x2/u,cpu/event=0xd,umask=0x3,cmask=1/u},dummy,dummy,emulation-faults,dummy,{cpu/event=0xa3,umask=0x6,cmask=6/u,cpu/event=0xa2,umask=0x8/u,cpu/event=0xa3,umask=0x4,cmask=4/u,cpu/event=0xb1,umask=0x1,cmask=1/u},{cpu/event=0xb1,umask=0x1,cmask=3/u,cpu/event=0xb1,umask=0x1,cmask=2/u,cpu/event=0xc0,umask=0x0/u,cpu/event=0x5e,umask=0x1/u,cpu/event=0x9c,umask=0x1,cmask=4/u},{cpu/event=0x79,umask=0x30,edge=1,cmask=1/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x87,umask=0x1/u,cpu/event=0xab,umask=0x2/u,cpu/event=0xa2,umask=0x8/u},{cpu/event=0x85,umask=0x10/u,cpu/event=0x85,umask=0x4/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x8,umask=0x10/u,cpu/event=0x8,umask=0x4/u},{cpu/event=0xc5,umask=0x0/u,cpu/event=0xc3,umask=0x1,edge=1,cmask=1/u,cpu/event=0xe6,umask=0x1f/u,cpu/event=0x3c,umask=0x0/u},{cpu/event=0xd1,umask=0x4/u,cpu/event=0xd1,umask=0x20/u,cpu/event=0xa3,umask=0x5,cmask=5/u,cpu/event=0x3c,umask=0x0/u},{cpu/event=0x14,umask=0x1/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u,cpu/event=0x3c,umask=0x1/u,cpu/event=0x11,umask=0x2/u},{cpu/event=0xc2,umask=0x2/u,cpu/event=0xe,umask=0x1/u,cpu/event=0x79,umask=0x30/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x3c,umask=0x2/u},{cpu/event=0xc2,umask=0x2/u,cpu/event=0x10,umask=0x1/u,cpu/event=0xb1,umask=0x1/u,cpu/event=0x10,umask=0x20/u},{cpu/event=0x10,umask=0x80/u,cpu/event=0x10,umask=0x10/u,cpu/event=0x10,umask=0x40/u,cpu/event=0x11,umask=0x1/u},{cpu/event=0x3c,umask=0x0/u,cpu/event=0x9c,umask=0x1,cmask=4/u,cpu/event=0x3c,umask=0x2/u,cpu/event=0x3c,umask=0x1/u},dummy,{cpu/event=0xa3,umask=0x4,cmask=4/u,cpu/event=0xb1,umask=0x1,cmask=1/u,cpu/event=0xb1,umask=0x1,cmask=3/u,cpu/event=0xb1,umask=0x1,cmask=2/u},{cpu/event=0xc0,umask=0x0/u,cpu/event=0x5e,umask=0x1/u,cpu/event=0xa2,umask=0x8/u,cpu/event=0xa3,umask=0x6,cmask=6/u,cpu/event=0x3c,umask=0x0/u,cpu/event=0x60,umask=0x8,cmask=6/u},{cpu/event=0x3c,umask=0x0/u,cpu/event=0x60,umask=0x8,cmask=1/u,cpu/event=0x60,umask=0x8,cmask=6/u},{cpu/event=0xc2,umask=0x2/u,cpu/event=0x10,umask=0x1/u,cpu/event=0xb1,umask=0x1/u},{cpu/event=0x10,umask=0x20/u,cpu/event=0x10,umask=0x80/u,cpu/event=0xb1,umask=0x1/u,cpu/event=0x10,umask=0x10/u},emulation-faults,{cpu/event=0x10,umask=0x10/u,cpu/event=0x10,umask=0x40/u,cpu/event=0x11,umask=0x1/u,cpu/event=0x11,umask=0x2/u},emulation-faults,{cpu/event=0x11,umask=0x2/u,cpu/event=0x11,umask=0x1/u,cpu/event=0xb1,umask=0x1/u}' ~/pmu/pmu-tools/workloads/BC1s
grep uncore p3

  reply	other threads:[~2023-06-22  9:29 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-22  1:39 Perf stat regression from d15480a3d67 Andi Kleen
2023-06-22  6:10 ` Ian Rogers
2023-06-22  9:20   ` Andi Kleen [this message]
2023-06-22 13:41     ` Ian Rogers
2023-06-22 19:09     ` Namhyung Kim
2023-06-22 20:15       ` Andi Kleen
2023-06-23  0:08         ` Namhyung Kim
2023-06-23 17:04           ` Andi Kleen
2023-06-23 18:35           ` Andi Kleen
2023-06-23 18:53             ` Ian Rogers
2023-06-23 23:04               ` Namhyung Kim
2023-06-24 17:59                 ` Andi Kleen
2023-06-26  5:33                   ` Ian Rogers
2023-06-26 23:09                   ` Namhyung Kim
2023-06-26 23:52                     ` Andi Kleen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZJQSUHLe9Cqpw21n@tassilo \
    --to=ak@linux.intel.com \
    --cc=acme@kernel.org \
    --cc=irogers@google.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).