public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <andi@firstfloor.org>
To: Stephane Eranian <eranian@google.com>
Cc: Andi Kleen <andi@firstfloor.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Jiri Olsa <jolsa@kernel.org>, Ingo Molnar <mingo@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Namhyung Kim <namhyung@kernel.org>
Subject: Re: Add top down metrics to perf stat v2
Date: Fri, 18 Dec 2015 22:38:55 +0100	[thread overview]
Message-ID: <20151218213854.GD15533@two.firstfloor.org> (raw)
In-Reply-To: <CABPqkBRftsHEAEwgCn3i3=mfk9fjh5r4MycdjHKRka5voTj9JA@mail.gmail.com>

On Fri, Dec 18, 2015 at 01:31:18AM -0800, Stephane Eranian wrote:
> >> Why force --per-core when HT is on. I know you you need to aggregate
> >> per core, but
> >> you could still display globally. And then if user requests
> >> --per-core, then display per core.
> >
> > Global TopDown doesn't make much sense. Suppose you have two programs
> > running on different cores, one frontend bound and one backend bound.
> > What would the union of the two mean? And you may well end up
> > with sums of ratios which are >100%.
> >
> How could that be if you consider that the machine is N-wide and not just 4-wide
> anymore?
> 
> How what you are describing here is different when HT is off?

I was talking about cores, not CPU threads.

With global aggregation we would aggregate data from different cores,
which is highly dubious for TopDown.

CPU threads on a core are of course aggregated, that is why the patchkit
forces --per-core with HT on.

> If you force --per-core with HT-on, then you need to force it too when
> HT is off so that  you get a similar per core breakdown. In the HT on
> case, each Sx-Cy represents 2 threads, compared to 1 in the non HT
> case.Right now, you have non-HT reporting global, HT reporting per-core.
> That does not make much sense to me.

Ok.  I guess can force --per-core in this case too. This would simplify
things because can get rid of the agg-per-core attribute.

> >> but it would be clearer and simpler to interpret to users.
> >
> > Same problem as above.
> >
> >>
> >> One bug I found when testing is that if you do with HT-on:
> >>
> >> $ perf stat -a --topdown -I 1000 --metric-only sleep 100
> >> Then you get data for frontend and backend but nothing for retiring or
> >> bad speculation.
> >
> > You see all the columns, but no data in some?
> >
> yes, and I don't like that. It is confusing especially when you do not
> know the threshold.
> Why are you suppressing the 'retiring' data when it is at 25% (1/4 of
> the maximum possible)
> when I am running a simple noploop? 25% is a sign of underutilization,
> that could be useful too.

It's what the TopDown specification uses and the paper describes. 

The thresholds are needed when you have more than one level because
the lower levels become meaningless if their parents didn't cross the
threshold. Otherwise you may report something that looks like a
bottle neck, but isn't.

Given there is currently only level 1 in the patchkit, but if we ever
add more levels absolutely need thresholds. So it's better to have
them from Day 1.

Utilization should be reported separately. TopDown cannot give
utilization because it doesn't know about idle time.

I can report - for empty fields if it helps you.  It's not clear
to me why empty fields in CSV are a problem.

I don't think colors are useful here, this would have the problem
described above.

> 
> > That's intended: the percentage is only printed when it crosses a
> > threshold. That's part of the top down specification.
> >
> I don't like that. I would rather see all the percentages.
> My remark applies to non topdown metrics as well, such as IPC.
> Clearly the IPC is awkward to use. You need to know you need to
> measure cycles, instructions to get ipc with --metric-only. Again,

Well it's the default (perf stat --metric-only), or with -d*, and it works fine
with --transaction too.

If you think there should be more predefined sets of metrics
that's fine for me too, but it would be a separate
patch.


-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

      reply	other threads:[~2015-12-18 21:38 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-16  0:54 Add top down metrics to perf stat v2 Andi Kleen
2015-12-16  0:54 ` [PATCH 01/10] perf, tools: Dont stop PMU parsing on alias parse error Andi Kleen
2015-12-21 16:08   ` Jiri Olsa
2015-12-16  0:54 ` [PATCH 02/10] perf, tools, stat: Force --per-core mode for .agg-per-core aliases Andi Kleen
2015-12-16 14:16   ` Stephane Eranian
2015-12-21 16:14   ` Jiri Olsa
2015-12-21 16:16   ` Jiri Olsa
2015-12-16  0:54 ` [PATCH 03/10] perf, tools, stat: Avoid fractional digits for integer scales Andi Kleen
2015-12-16 14:22   ` Stephane Eranian
2015-12-16  0:54 ` [PATCH 04/10] perf, tools, stat: Scale values by unit before metrics Andi Kleen
2015-12-16  0:54 ` [PATCH 05/10] perf, tools, stat: Basic support for TopDown in perf stat Andi Kleen
2015-12-16 14:32   ` Stephane Eranian
2015-12-16 21:21     ` Andi Kleen
2015-12-17  9:26       ` Stephane Eranian
2015-12-17 14:03         ` Andi Kleen
2015-12-16  0:54 ` [PATCH 06/10] perf, tools, stat: Add computation of TopDown formulas Andi Kleen
2015-12-16  0:54 ` [PATCH 07/10] perf, tools, stat: Add extra output of counter values with -v Andi Kleen
2015-12-16  0:54 ` [PATCH 08/10] x86, perf: Support sysfs files depending on SMT status Andi Kleen
2015-12-16  8:53   ` Peter Zijlstra
2015-12-16 12:48   ` Stephane Eranian
2015-12-16 16:26     ` Andi Kleen
2015-12-16  0:54 ` [PATCH 09/10] x86, perf: Add Top Down events to Intel Core Andi Kleen
2015-12-16  0:54 ` [PATCH 10/10] x86, perf: Add Top Down events to Intel Atom Andi Kleen
2015-12-17 10:27 ` Add top down metrics to perf stat v2 Stephane Eranian
2015-12-17 14:01   ` Andi Kleen
2015-12-17 23:31     ` Stephane Eranian
2015-12-18  1:55       ` Andi Kleen
2015-12-18  9:31         ` Stephane Eranian
2015-12-18 21:38           ` Andi Kleen [this message]

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=20151218213854.GD15533@two.firstfloor.org \
    --to=andi@firstfloor.org \
    --cc=acme@kernel.org \
    --cc=eranian@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.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