Messages in this thread | | | Subject | Re: [V4 PATCH] perf parse-events: Specially handle uncore event alias in small groups | From | "Liang, Kan" <> | Date | Tue, 1 May 2018 08:31:02 -0400 |
| |
>> +static int >> +parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list, >> + struct parse_events_state *parse_state) >> +{ >> + struct perf_evsel *evsel, *leader; >> + uintptr_t *leaders; >> + bool is_leader = true; >> + int i, nr_pmu = 0, total_members, ret = 0; >> + >> + leader = list_first_entry(list, struct perf_evsel, node); >> + evsel = list_last_entry(list, struct perf_evsel, node); >> + total_members = evsel->idx - leader->idx + 1; >> + >> + leaders = calloc(total_members, sizeof(uintptr_t)); >> + if (!leaders) >> + return 0; > > I see.. I meant we should return -ENOMEM in here, but > we don't check it in the called anyway.. so perhaps > > if (WARN_ON(!leaders)) > return 0; >
Sure. I will change it in V5.
> >> + >> + __evlist__for_each_entry(list, evsel) { >> + >> + /* Only split the uncore group which members use alias */ >> + if (!evsel->use_uncore_alias) >> + goto out; >> + >> + /* The events must be from the same uncore block */ >> + if (!is_same_uncore_block(leader->pmu_name, evsel->pmu_name)) >> + goto out; >> + >> + if (!is_leader) >> + continue; >> + /* >> + * If the event's PMU name starts to repeat, it must be a new >> + * event. That can be used to distinguish the leader from >> + * other members, even they have the same event name. >> + */ >> + if ((leader != evsel) && (leader->pmu_name == evsel->pmu_name)) { >> + is_leader = false; >> + continue; >> + } > > I still don't get why you don't 'break' in here.. you set is_leader to false, > then the loop will never get pass the !is_leader condition above >
All the members must use alias, and be from the same uncore block. We have to go through the whole group to check the events.
Thanks, Kan
| |