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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d2b94d2c-1bbb-bf7c-2aa6-102660bf4a7d@linux.intel.com>
Date:   Tue, 1 May 2018 08:31:02 -0400
From:   "Liang, Kan" <kan.liang@...ux.intel.com>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     acme@...nel.org, mingo@...hat.com, peterz@...radead.org,
        linux-kernel@...r.kernel.org, namhyung@...nel.org,
        ganapatrao.kulkarni@...ium.com, zhangshaokun@...ilicon.com,
        yao.jin@...ux.intel.com, will.deacon@....com, ak@...ux.intel.com,
        agustinv@...eaurora.org
Subject: Re: [V4 PATCH] perf parse-events: Specially handle uncore event alias
 in small groups


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