On Mon, Mar 29, 2021 at 03:00:31PM +0800, Jin Yao wrote:

SNIP

> +                        struct parse_events_state *parse_state)
>  {
>       struct perf_event_attr attr;
>       LIST_HEAD(config_terms);
> @@ -521,7 +526,7 @@ int parse_events_add_cache(struct list_head *list, int 
> *idx,
>  
>       i = parse_events__add_cache_hybrid(list, idx, &attr,
>                                          config_name ? : name, &config_terms,
> -                                        &hybrid);
> +                                        &hybrid, parse_state);
>       if (hybrid)
>               return i;
>  
> @@ -1481,7 +1486,7 @@ int parse_events_add_pmu(struct parse_events_state 
> *parse_state,
>       struct perf_pmu *pmu;
>       struct evsel *evsel;
>       struct parse_events_error *err = parse_state->error;
> -     bool use_uncore_alias;
> +     bool use_uncore_alias, found = false;
>       LIST_HEAD(config_terms);
>  
>       if (verbose > 1) {
> @@ -1530,8 +1535,28 @@ int parse_events_add_pmu(struct parse_events_state 
> *parse_state,
>               }
>       }
>  
> -     if (!parse_state->fake_pmu && perf_pmu__check_alias(pmu, head_config, 
> &info))
> +     if (!parse_state->fake_pmu &&
> +         perf_pmu__check_alias(pmu, head_config, &info, &found)) {
>               return -EINVAL;
> +     }
> +

ok, let's not polute surronding functions and make strict check
on what we want in here.. we are after following events:

        cpu_xxx/L1-dcache/
        cpu_xxx/l1-d|/
        ...
right?

so we are after events with single term in head_config that has name in:

        L1-dcache|l1-d|l1d|L1-data              |
        L1-icache|l1-i|l1i|L1-instruction       |
        LLC|L2                                  |
        dTLB|d-tlb|Data-TLB                     |
        iTLB|i-tlb|Instruction-TLB              |
        branch|branches|bpu|btb|bpc             |
        node

I think that with such direct check the code will be more straight
forward, also let's move it to parse-events-hybrid

> +     if (!parse_state->fake_pmu && head_config && !found &&
> +         perf_pmu__is_hybrid(name)) {
> +             struct parse_events_term *term;
> +             int ret;
> +
> +             list_for_each_entry(term, head_config, list) {
> +                     if (!term->config)
> +                             continue;
> +
> +                     ret = parse_events__with_hybrid_pmu(parse_state,
> +                                                         term->config,
> +                                                         name, &found,
> +                                                         list);

do we need to call the parsing again? could we just call
parse_events__add_cache_hybrid?

jirka


> +                     if (found)
> +                             return ret;
> +             }
> +     }
>  
>       if (verbose > 1) {
>               fprintf(stderr, "After aliases, add event pmu '%s' with '",
> @@ -1605,6 +1630,15 @@ int parse_events_multi_pmu_add(struct 
> parse_events_state *parse_state,
>       struct perf_pmu *pmu = NULL;
>       int ok = 0;
>  

SNIP

Reply via email to