Re: [PATCH v2 11/27] perf parse-events: Support hardware events inside PMU

From: "Liang, Kan" <kan.liang@linux.intel.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Jiri Olsa <jolsa@redhat.com>, Mark Rutland <mark.rutland@arm.com>
Cc: "Jin, Yao" <yao.jin@linux.intel.com>,
	jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com,
	alexander.shishkin@linux.intel.com, Linux-kernel@vger.kernel.org,
	ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH v2 11/27] perf parse-events: Support hardware events inside PMU
Date: Thu, 18 Mar 2021 14:16:55 -0400	[thread overview]
Message-ID: <c1320d04-fcc1-562c-5429-4649a80d9bbb@linux.intel.com> (raw)
In-Reply-To: <YFNT8YpR7bnCjmdf@kernel.org>

On 3/18/2021 9:21 AM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Mar 18, 2021 at 01:16:37PM +0100, Jiri Olsa escreveu:
>> On Wed, Mar 17, 2021 at 10:42:45AM -0300, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Mar 17, 2021 at 08:17:52PM +0800, Jin, Yao escreveu:
>>>> I'm OK to only support 'cpu_core/cpu-cycles/' or 'cpu_atom/cpu-cycles/'. But
>>>> what would we do for cache event?
> 
>>>> 'perf stat -e LLC-loads' is OK, but 'perf stat -e cpu/LLC-loads/' is not supported currently.
> 
>>>> For hybrid platform, user may only want to enable the LLC-loads on core CPUs
>>>> or on atom CPUs. That's reasonable. While if we don't support the pmu style
>>>> event, how to satisfy this requirement?
> 
>>>> If we can support the pmu style event, we can also use the same way for
>>>> cpu_core/cycles/. At least it's not a bad thing, right? :)
> 
>>> While we're discussing, do we really want to use the "core" and "atom"
>>> terms here? I thought cpu/cycles/ would be ok for the main (Big) CPU and
>>> that we should come up with some short name for the "litle" CPUs.
> 
>>> Won't we have the same situation with ARM where we want to know the
>>> number of cycles spent on a BIG core and also on a little one?
> 
>>> Perhaps 'cycles' should mean all cycles, and then we use 'big/cycles/' and
>>> 'little/cycles/'?
> 
>> do arm servers already export multiple pmus like this?
>> I did not notice
> 
> I haven't checked, but AFAIK this BIG/Little kind of arch started there,
> Mark?

Here is the cover letter of the ARM big.little patch set. ARM also 
exports multiple PMUs, e.g., armv7_cortex_a15 and armv7_cortex_a7.
https://lore.kernel.org/lkml/1431533549-27715-1-git-send-email-mark.rutland@arm.com/

We follow a similar way to handle the Intel hybrid PMUs. The naming rule 
is also similar, "cpu_" + CPU type.

We don't use the old name "cpu" for the main CPU type, because we want 
to make sure every software updated for the hybrid architecture. 
Otherwise, the old script with "cpu//" can still run on a hybrid 
architecture. Users cannot notice that the monitored scope is already 
implicitly changed. The results may be not what they want.

Thanks,
Kan

> 
> - Arnaldo
>   
>> it'd be definitely great to have some unite way for this,
>> so far we have the hybrid pmu detection and support in
>> hw events like cycles/instructions.. which should be easy
>> to follow on arm
>>
>> there's also support to have these events on specific pmu
>> pmu/cycles/ , which I still need to check on