linux-kernel - Re: [PATCH v3 11/27] perf pmu: Support 'cycles' and 'branches' inside hybrid PMU

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <a9a2f035-5b5a-be3c-af75-5fffee650a24@linux.intel.com>
Date:   Mon, 12 Apr 2021 10:01:31 +0800
From:   "Jin, Yao" <yao.jin@...ux.intel.com>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     acme@...nel.org, jolsa@...nel.org, peterz@...radead.org,
        mingo@...hat.com, alexander.shishkin@...ux.intel.com,
        Linux-kernel@...r.kernel.org, ak@...ux.intel.com,
        kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH v3 11/27] perf pmu: Support 'cycles' and 'branches' inside
 hybrid PMU

Hi Jiri,

On 4/9/2021 9:48 PM, Jiri Olsa wrote:
> On Mon, Mar 29, 2021 at 03:00:30PM +0800, Jin Yao wrote:
>> On hybrid platform, user may want to enable the hardware event
>> only on one PMU. So following syntax is supported:
>>
>> cpu_core/<hardware event>/
>> cpu_atom/<hardware event>/
>>
>>    # perf stat -e cpu_core/cpu-cycles/ -a -- sleep 1
>>
>>     Performance counter stats for 'system wide':
>>
>>             6,049,336      cpu_core/cpu-cycles/
>>
>>           1.003577042 seconds time elapsed
>>
>> It enables the event 'cpu-cycles' only on cpu_core pmu.
>>
>> But for 'cycles' and 'branches', the syntax doesn't work.
> 
> because the alias is not there.. but there's:
>    cpu/cpu-cycles/
>    cpu/branch-instructions/
> 
> doing the same thing..  what's wrong with that?
> 
> I have a feeling we discussed this in the previous
> version.. did I give up? ;-)
> 

Yes, we discussed this in previous threads. :)

Now I'm fine to keep the original behavior. Because the syntax 'cpu/cycles/' and 'cpu/branches/' are 
not supported by current perf.

> SNIP
> 
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index beff29981101..72e5ae5e868e 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
>> @@ -916,6 +916,35 @@ static int pmu_max_precise(const char *name)
>>   	return max_precise;
>>   }
>>   
>> +static void perf_pmu__add_hybrid_aliases(struct list_head *head)
>> +{
>> +	static struct pmu_event pme_hybrid_fixup[] = {
>> +		{
>> +			.name = "cycles",
>> +			.event = "event=0x3c",
>> +		},
>> +		{
>> +			.name = "branches",
>> +			.event = "event=0xc4",
>> +		},
>> +		{
>> +			.name = 0,
>> +			.event = 0,
>> +		},
> 
> if you really need to access these 2 events with special name,
> why not add it through the json.. let's not have yet another
> place that defines aliases ... also this should be model specific
> no?
> 

Yes, defining in json is a good idea if we really need to support 'cpu/cycles/' and 'cpu/branches/'.

Anyway, I will drop this patch in next version.

Thanks
Jin Yao

> jirka
>