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

["Liang, Kan" <kan.liang@linux.intel.com>]

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