lkml.org 
[lkml]   [2015]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 05/10] perf, tools, stat: Basic support for TopDown in perf stat
From
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?


\
 
 \ /
  Last update: 2015-12-17 10:41    [W:0.057 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site