[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <11ed193e-75f9-c060-b757-350612b694aa@linux.intel.com>
Date: Fri, 9 Apr 2021 11:24:07 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: mingo@...nel.org, linux-kernel@...r.kernel.org, acme@...nel.org,
tglx@...utronix.de, bp@...en8.de, namhyung@...nel.org,
jolsa@...hat.com, ak@...ux.intel.com, yao.jin@...ux.intel.com,
alexander.shishkin@...ux.intel.com, adrian.hunter@...el.com,
ricardo.neri-calderon@...ux.intel.com,
Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH V5 21/25] perf: Introduce PERF_TYPE_HARDWARE_PMU and
PERF_TYPE_HW_CACHE_PMU
On 4/9/2021 5:21 AM, Peter Zijlstra wrote:
> On Mon, Apr 05, 2021 at 08:11:03AM -0700, kan.liang@...ux.intel.com wrote:
>> From: Kan Liang <kan.liang@...ux.intel.com>
>>
>> Current Hardware events and Hardware cache events have special perf
>> types, PERF_TYPE_HARDWARE and PERF_TYPE_HW_CACHE. The two types don't
>> pass the PMU type in the user interface. For a hybrid system, the perf
>> subsystem doesn't know which PMU the events belong to. The first capable
>> PMU will always be assigned to the events. The events never get a chance
>> to run on the other capable PMUs.
>>
>> Add a PMU aware version PERF_TYPE_HARDWARE_PMU and
>> PERF_TYPE_HW_CACHE_PMU. The PMU type ID is stored at attr.config[40:32].
>> Support the new types for X86.
>
> Obviously ARM would need the same, but also, I don't think I see the
> need to introduce new types. AFAICT there is nothing that stops this
> scheme from working for the existing types.
>
> Also, pmu type is 32bit, not 8bit.
>
> So how about something like this?
>
> ---
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 3f7f89ea5e51..074c7687d466 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -260,15 +260,16 @@ struct perf_event;
> /**
> * pmu::capabilities flags
> */
> -#define PERF_PMU_CAP_NO_INTERRUPT 0x01
> -#define PERF_PMU_CAP_NO_NMI 0x02
> -#define PERF_PMU_CAP_AUX_NO_SG 0x04
> -#define PERF_PMU_CAP_EXTENDED_REGS 0x08
> -#define PERF_PMU_CAP_EXCLUSIVE 0x10
> -#define PERF_PMU_CAP_ITRACE 0x20
> -#define PERF_PMU_CAP_HETEROGENEOUS_CPUS 0x40
> -#define PERF_PMU_CAP_NO_EXCLUDE 0x80
> -#define PERF_PMU_CAP_AUX_OUTPUT 0x100
> +#define PERF_PMU_CAP_NO_INTERRUPT 0x0001
> +#define PERF_PMU_CAP_NO_NMI 0x0002
> +#define PERF_PMU_CAP_AUX_NO_SG 0x0004
> +#define PERF_PMU_CAP_EXTENDED_REGS 0x0008
> +#define PERF_PMU_CAP_EXCLUSIVE 0x0010
> +#define PERF_PMU_CAP_ITRACE 0x0020
> +#define PERF_PMU_CAP_HETEROGENEOUS_CPUS 0x0040
> +#define PERF_PMU_CAP_NO_EXCLUDE 0x0080
> +#define PERF_PMU_CAP_AUX_OUTPUT 0x0100
> +#define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0200
>
> struct perf_output_handle;
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index f07943183041..910a0666ebfe 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -11113,14 +11113,21 @@ static struct pmu *perf_init_event(struct perf_event *event)
> * are often aliases for PERF_TYPE_RAW.
> */
> type = event->attr.type;
> - if (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE)
> - type = PERF_TYPE_RAW;
> + if (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE) {
> + type = event->attr.config >> 32;
> + if (!type)
> + type = PERF_TYPE_RAW;
> + }
For the old tool, the default PMU will be the big core. I think it's OK
for X86.
Since only the low 32 bit of event->attr.config contains the 'real'
config value, I think all the ARCHs will do event->attr.config &=
0xffffffff. Maybe we should move it to the generic code.
+ if (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE) {
+ type = event->attr.config >> 32;
+ if (!type)
+ type = PERF_TYPE_RAW;
+ else
+ event->attr.config &= 0xffffffff;
> > again:
> rcu_read_lock();
> pmu = idr_find(&pmu_idr, type);
> rcu_read_unlock();
> if (pmu) {
> + if (event->attr.type != type && type != PERF_TYPE_RAW &&
> + !(pmu->capabilities & PERF_PMU_CAP_EXTENDED_HW_TYPE))
> + goto fail;
> +
> ret = perf_try_init_event(pmu, event);
> if (ret == -ENOENT && event->attr.type != type) {
> type = event->attr.type;
I don't think we want to go through all available PMUs again if users
already specify a PMU.
I update the patch a little bit. (Not test yet. I will do some tests then.)
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 3158cbc..4f5f9a9 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2175,6 +2175,7 @@ static int __init init_hw_perf_events(void)
hybrid_pmu->pmu.type = -1;
hybrid_pmu->pmu.attr_update = x86_pmu.attr_update;
hybrid_pmu->pmu.capabilities |= PERF_PMU_CAP_HETEROGENEOUS_CPUS;
+ hybrid_pmu->pmu.capabilities |= PERF_PMU_CAP_EXTENDED_HW_TYPE;
err = perf_pmu_register(&hybrid_pmu->pmu, hybrid_pmu->name,
(hybrid_pmu->cpu_type == hybrid_big) ? PERF_TYPE_RAW : -1);
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index b832e09..391bfb7 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -269,6 +269,7 @@ struct perf_event;
#define PERF_PMU_CAP_HETEROGENEOUS_CPUS 0x40
#define PERF_PMU_CAP_NO_EXCLUDE 0x80
#define PERF_PMU_CAP_AUX_OUTPUT 0x100
+#define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x200
struct perf_output_handle;
diff --git a/include/uapi/linux/perf_event.h
b/include/uapi/linux/perf_event.h
index ad15e40..7ec80ac9 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -38,6 +38,20 @@ enum perf_type_id {
};
/*
+ * attr.config layout for type PERF_TYPE_HARDWARE and PERF_TYPE_HW_CACHE
+ * PERF_TYPE_HARDWARE: 0xEE000000AA
+ * AA: hardware event ID
+ * EE: PMU type ID
+ * PERF_TYPE_HW_CACHE: 0xEE00DDCCBB
+ * BB: hardware cache ID
+ * CC: hardware cache op ID
+ * DD: hardware cache op result ID
+ * EE: PMU type ID
+ * If the PMU type ID is 0, the PERF_TYPE_RAW will be applied.
+ */
+#define PERF_HW_EVENT_MASK 0xffffffff
+
+/*
* Generalized performance event event_id types, used by the
* attr.event_id parameter of the sys_perf_event_open()
* syscall:
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f079431..9d9a792 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11093,6 +11093,8 @@ static int perf_try_init_event(struct pmu *pmu,
struct perf_event *event)
return ret;
}
+#define PERF_EXTENDED_HW_TYPE (event->attr.config >> 32)
+
static struct pmu *perf_init_event(struct perf_event *event)
{
int idx, type, ret;
@@ -11113,16 +11115,25 @@ static struct pmu *perf_init_event(struct
perf_event *event)
* are often aliases for PERF_TYPE_RAW.
*/
type = event->attr.type;
- if (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE)
- type = PERF_TYPE_RAW;
+ if (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE) {
+ type = PERF_EXTENDED_HW_TYPE;
+ if (!type)
+ type = PERF_TYPE_RAW;
+ else
+ event->attr.config &= PERF_HW_EVENT_MASK;
+ }
again:
rcu_read_lock();
pmu = idr_find(&pmu_idr, type);
rcu_read_unlock();
if (pmu) {
+ if (event->attr.type != type && type != PERF_TYPE_RAW &&
+ !(pmu->capabilities & PERF_PMU_CAP_EXTENDED_HW_TYPE))
+ goto fail;
+
ret = perf_try_init_event(pmu, event);
- if (ret == -ENOENT && event->attr.type != type) {
+ if (ret == -ENOENT && event->attr.type != type &&
!PERF_EXTENDED_HW_TYPE) {
type = event->attr.type;
goto again;
}
@@ -11143,6 +11154,7 @@ static struct pmu *perf_init_event(struct
perf_event *event)
goto unlock;
}
}
+fail:
pmu = ERR_PTR(-ENOENT);
unlock:
srcu_read_unlock(&pmus_srcu, idx);
Thanks,
Kan
Powered by blists - more mailing lists