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

Groups > linux.kernel > #1293703

Re: [PATCH 05/10] perf, tools, stat: Basic support for TopDown in perf stat

From Stephane Eranian <eranian@google.com>
Newsgroups linux.kernel
Subject Re: [PATCH 05/10] perf, tools, stat: Basic support for TopDown in perf stat
Date 2015-12-17 10:30 +0100
Message-ID <qGD2O-5Sl-13@gated-at.bofh.it> (permalink)
References <qG8BH-3aD-1@gated-at.bofh.it> <qG8BJ-3aD-21@gated-at.bofh.it> <qGlpf-2W2-3@gated-at.bofh.it> <qGrO1-76X-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw

Andi,

On Wed, Dec 16, 2015 at 1:21 PM, Andi Kleen <andi@firstfloor.org> wrote:
>> > +/*
>> > + * Check whether we can use a group for top down.
>> > + * Without a group may get bad results due to multiplexing.
>> > + */
>>
>> That is not because you have a counter used by the NMI that
>> you cannot group. If HT is off you have plenty of counters to
>> do this.
>
> Such a heuristic wouldn't work on Atom where there are no more
> counters in this case.
>
>> > +static bool check_group(bool *warn)
>> > +{
>> > +       int n;
>> > +
>> > +       if (sysctl__read_int("kernel/nmi_watchdog", &n) < 0)
>> > +               return false;
>> > +       if (n > 0) {
>> > +               *warn = true;
>> > +               return false;
>> > +       }
>> > +       return true;
>> > +}
>> > +
>>
>> I do not like this part very much. You are pulling in x86 specific
>> knowledge into
>> builtin_stat.c. Why not move this into an x86 specific file?
>
> Done.
>
>> >                         err = parse_events(evsel_list, transaction_attrs, NULL);
>> > @@ -1511,6 +1579,36 @@ static int add_default_attributes(void)
>> >                 return 0;
>> >         }
>> >
>> > +       if (topdown_run) {
>> > +               char *str = NULL;
>> > +               bool warn = false;
>> > +
>> > +               filter_events(topdown_attrs, &str, check_group(&warn));
>> > +               if (topdown_attrs[0] && str) {
>> > +                       if (warn)
>> > +                               fprintf(stderr,
>> > +               "nmi_watchdog enabled with topdown. May give wrong results.\n"
>> > +               "Disable with echo 0 > /proc/sys/kernel/nmi_watchdog\n");
>>
>> This is x86 specific. Why not just try it out and in case of error
>> suggest checking
>> if pinned system-wide events exist (such as NMI watchdog on x86). that would
>> be more generic.
>
> That's really complicated, i would have to tear down all state and then
> resubmit all the events. I think just checking the NMI watchdog is good
> enough. I couldn't give a sensible error message for the generic case
> anyways.
>
What about that fancy error reporting extension that was proposed a few months
back? It could help print a sensible error msg. Did it ever make it in?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Add top down metrics to perf stat v2 Andi Kleen <andi@firstfloor.org> - 2015-12-16 02:00 +0100
  [PATCH 07/10] perf, tools, stat: Add extra output of counter values with -v Andi Kleen <andi@firstfloor.org> - 2015-12-16 02:00 +0100
  [PATCH 10/10] x86, perf: Add Top Down events to Intel Atom Andi Kleen <andi@firstfloor.org> - 2015-12-16 02:00 +0100
  [PATCH 04/10] perf, tools, stat: Scale values by unit before metrics Andi Kleen <andi@firstfloor.org> - 2015-12-16 02:00 +0100
  [PATCH 09/10] x86, perf: Add Top Down events to Intel Core Andi Kleen <andi@firstfloor.org> - 2015-12-16 02:00 +0100
  [PATCH 03/10] perf, tools, stat: Avoid fractional digits for integer scales Andi Kleen <andi@firstfloor.org> - 2015-12-16 02:00 +0100
    Re: [PATCH 03/10] perf, tools, stat: Avoid fractional digits for  integer scales Stephane Eranian <eranian@google.com> - 2015-12-16 15:30 +0100
  [PATCH 05/10] perf, tools, stat: Basic support for TopDown in perf stat Andi Kleen <andi@firstfloor.org> - 2015-12-16 02:00 +0100
    Re: [PATCH 05/10] perf, tools, stat: Basic support for TopDown in  perf stat Stephane Eranian <eranian@google.com> - 2015-12-16 15:40 +0100
      Re: [PATCH 05/10] perf, tools, stat: Basic support for TopDown in  perf stat Andi Kleen <andi@firstfloor.org> - 2015-12-16 22:30 +0100
        Re: [PATCH 05/10] perf, tools, stat: Basic support for TopDown in  perf stat Stephane Eranian <eranian@google.com> - 2015-12-17 10:30 +0100
          Re: [PATCH 05/10] perf, tools, stat: Basic support for TopDown in  perf stat Andi Kleen <andi@firstfloor.org> - 2015-12-17 15:10 +0100
  [PATCH 08/10] x86, perf: Support sysfs files depending on SMT status Andi Kleen <andi@firstfloor.org> - 2015-12-16 02:00 +0100
    Re: [PATCH 08/10] x86, perf: Support sysfs files depending on SMT  status Peter Zijlstra <peterz@infradead.org> - 2015-12-16 10:00 +0100
    Re: [PATCH 08/10] x86, perf: Support sysfs files depending on SMT status Stephane Eranian <eranian@google.com> - 2015-12-16 13:50 +0100
      Re: [PATCH 08/10] x86, perf: Support sysfs files depending on SMT  status Andi Kleen <andi@firstfloor.org> - 2015-12-16 17:30 +0100
  [PATCH 01/10] perf, tools: Dont stop PMU parsing on alias parse error Andi Kleen <andi@firstfloor.org> - 2015-12-16 02:00 +0100
    Re: [PATCH 01/10] perf, tools: Dont stop PMU parsing on alias parse  error Jiri Olsa <jolsa@redhat.com> - 2015-12-21 17:10 +0100
  [PATCH 02/10] perf, tools, stat: Force --per-core mode for .agg-per-core aliases Andi Kleen <andi@firstfloor.org> - 2015-12-16 02:00 +0100
    Re: [PATCH 02/10] perf, tools, stat: Force --per-core mode for  .agg-per-core aliases Stephane Eranian <eranian@google.com> - 2015-12-16 15:20 +0100
    Re: [PATCH 02/10] perf, tools, stat: Force --per-core mode for  .agg-per-core aliases Jiri Olsa <jolsa@redhat.com> - 2015-12-21 17:20 +0100
    Re: [PATCH 02/10] perf, tools, stat: Force --per-core mode for  .agg-per-core aliases Jiri Olsa <jolsa@redhat.com> - 2015-12-21 17:20 +0100
  [PATCH 06/10] perf, tools, stat: Add computation of TopDown formulas Andi Kleen <andi@firstfloor.org> - 2015-12-16 02:00 +0100
  Re: Add top down metrics to perf stat v2 Stephane Eranian <eranian@google.com> - 2015-12-17 11:30 +0100
    Re: Add top down metrics to perf stat v2 Andi Kleen <andi@firstfloor.org> - 2015-12-17 15:10 +0100
      Re: Add top down metrics to perf stat v2 Stephane Eranian <eranian@google.com> - 2015-12-18 00:40 +0100
        Re: Add top down metrics to perf stat v2 Andi Kleen <andi@firstfloor.org> - 2015-12-18 03:00 +0100
          Re: Add top down metrics to perf stat v2 Stephane Eranian <eranian@google.com> - 2015-12-18 10:40 +0100
            Re: Add top down metrics to perf stat v2 Andi Kleen <andi@firstfloor.org> - 2015-12-18 22:40 +0100

csiph-web