lkml.org 
[lkml]   [2021]   [Feb]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [drm/i915/gt] 8c3b1ba0e7: perf-sanity-tests.Parse_event_definition_strings.fail
    On Thu, Feb 25, 2021 at 07:50:46PM +0100, Jiri Olsa wrote:
    > On Thu, Feb 25, 2021 at 07:16:44PM +0100, Jiri Olsa wrote:
    > > On Thu, Feb 25, 2021 at 02:33:57PM +0100, Jiri Olsa wrote:
    > > > On Thu, Feb 25, 2021 at 04:06:23PM +0800, Jin, Yao wrote:
    > > > > Hi Chris, Arnaldo, Jiri,
    > > > >
    > > > > We observe the parsing error for "software/xxx/" on some platforms.
    > > > >
    > > > > For example,
    > > > >
    > > > > # perf stat -e software/r1a/ -a -- sleep 1
    > > > > event syntax error: 'software/r1a/'
    > > > > \___ parser error
    > > > > Run 'perf list' for a list of valid events
    > > > >
    > > > > Usage: perf stat [<options>] [<command>]
    > > > >
    > > > > -e, --event <event> event selector. use 'perf list' to list available events
    > > > >
    > > > > And perf test is failed too.
    > > > >
    > > > > # perf test 6 -vv
    > > > > ...
    > > > > running test 4 'software/r1a/'failed to parse event 'software/r1a/', err 1, str 'parser error'
    > > > > event syntax error: 'software/r1a/'
    > > > > \___ parser error
    > > > >
    > > > > running test 4 'software/r0x1a/'failed to parse event 'software/r0x1a/', err 1, str 'parser error'
    > > > > event syntax error: 'software/r0x1a/'
    > > > > \___ parser error
    > > > > ...
    > > > >
    > > > > The issue should be the conflict between event "i915/software-gt-awake-time/" and pmu "software".
    > > > >
    > > > > # perf list | grep i915/software
    > > > > i915/software-gt-awake-time/ [Kernel PMU event]
    > > > >
    > > > > Perf supports the event format as "prefix-xxx-suffix", so the "software"
    > > > > string in "software-gt-awake-time" is added to perf_pmu_events_list as
    > > > > PMU_EVENT_SYMBOL_PREFIX. When parsing the string such as "software/xxx/",
    > > > > it’s parsed as PMU_EVENT_SYMBOL_PREFIX for "software", then it would error
    > > > > out in next processing.
    > > > >
    > > > > So the easy way is to rename "software-gt-awake-time" to "gt-awake-time",
    > > > > right? Otherwise we have to hardcode something in perf tool.
    > > > >
    > > > > What do you think of this issue and the solution?
    > > > >
    > > > > BTW, as the robot reported, it's related to the commit 8c3b1ba0e7ea
    > > > > ("drm/i915/gt: Track the overall awake/busy time").
    > > >
    > > > yes, I think you're right.. I'll try to reproduce and check on this
    > > >
    > > > I'm still not clear why is that parsing fail if those 2 events are
    > > > distinguished by - and / ... but yep, we are asking for trouble in
    > > > here ;-)
    > > >
    > > > I think we could either prioritaze one or add extra check when we
    > > > add prefix-xxx-suffix events and do not add ones that have conflict..
    > > > that seems like best solution to me now
    > >
    > > would patch below work for you?
    > >
    > > jirka
    >
    > and now with more fixes after I ran tests ;-)
    >
    > we should perhaps add also same check for non '-' aliases, I'll check

    and one more version ;-)

    jirka


    ---
    diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
    index 42c84adeb2fb..d9e795056d15 100644
    --- a/tools/perf/util/parse-events.c
    +++ b/tools/perf/util/parse-events.c
    @@ -2028,30 +2028,49 @@ static void perf_pmu__parse_init(void)
    perf_pmu_events_list_num = -1;
    return;
    }
    - perf_pmu_events_list = malloc(sizeof(struct perf_pmu_event_symbol) * len);
    + perf_pmu_events_list = zalloc(sizeof(struct perf_pmu_event_symbol) * len);
    if (!perf_pmu_events_list)
    return;
    - perf_pmu_events_list_num = len;

    len = 0;
    pmu = NULL;
    while ((pmu = perf_pmu__scan(pmu)) != NULL) {
    list_for_each_entry(alias, &pmu->aliases, list) {
    struct perf_pmu_event_symbol *p = perf_pmu_events_list + len;
    - char *tmp = strchr(alias->name, '-');
    + char *prefix = strdup(alias->name);
    + char *tmp = strchr(prefix, '-');
    +
    + if (tmp != NULL)
    + *tmp = 0;
    +
    + /*
    + * Skip any prefix that conflicts with pmu name,
    + * otherwise the event parser gets confused.
    + */
    + if (perf_pmu__find(prefix)) {
    + pr_debug2("perf_pmu__parse_init: skipping conflicting alias '%s'\n",
    + alias->name);
    + free(prefix);
    + continue;
    + }

    if (tmp != NULL) {
    - SET_SYMBOL(strndup(alias->name, tmp - alias->name),
    - PMU_EVENT_SYMBOL_PREFIX);
    + SET_SYMBOL(prefix, PMU_EVENT_SYMBOL_PREFIX);
    p++;
    SET_SYMBOL(strdup(++tmp), PMU_EVENT_SYMBOL_SUFFIX);
    len += 2;
    } else {
    - SET_SYMBOL(strdup(alias->name), PMU_EVENT_SYMBOL);
    + SET_SYMBOL(prefix, PMU_EVENT_SYMBOL);
    len++;
    }
    }
    }
    +
    + /* unlikely, but still.. */
    + if (!len)
    + goto err;
    + perf_pmu_events_list_num = len;
    +
    qsort(perf_pmu_events_list, len,
    sizeof(struct perf_pmu_event_symbol), comp_pmu);

    \
     
     \ /
      Last update: 2021-02-25 20:06    [W:2.988 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site