Re: [V4 PATCH] perf parse-events: Specially handle uncore event alias in small groups

[Liang, Kan]

> > +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