diff mbox series

[v1,1/2] perf arm-spe: Support multiple Arm SPE PMUs

Message ID 20240623133437.222736-2-leo.yan@arm.com (mailing list archive)
State New, archived
Headers show
Series perf mem: Support multiple Arm SPE PMUs | expand

Commit Message

Leo Yan June 23, 2024, 1:34 p.m. UTC
A platform can have more than one Arm SPE PMU. For example, a system
with multiple clusters may have each cluster enabled with its own Arm
SPE instance. In such case, the PMU devices will be named 'arm_spe_0',
'arm_spe_1', and so on.

Currently, the tool only supports 'arm_spe_0'. This commit extends
support to multiple Arm SPE PMUs by detecting the substring 'arm_spe'.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/arch/arm/util/pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ian Rogers June 24, 2024, 4:16 p.m. UTC | #1
On Sun, Jun 23, 2024 at 6:34 AM Leo Yan <leo.yan@arm.com> wrote:
>
> A platform can have more than one Arm SPE PMU. For example, a system
> with multiple clusters may have each cluster enabled with its own Arm
> SPE instance. In such case, the PMU devices will be named 'arm_spe_0',
> 'arm_spe_1', and so on.
>
> Currently, the tool only supports 'arm_spe_0'. This commit extends
> support to multiple Arm SPE PMUs by detecting the substring 'arm_spe'.
>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  tools/perf/arch/arm/util/pmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c
> index 8b7cb68ba1a8..29cfa1e427ed 100644
> --- a/tools/perf/arch/arm/util/pmu.c
> +++ b/tools/perf/arch/arm/util/pmu.c
> @@ -27,7 +27,7 @@ void perf_pmu__arch_init(struct perf_pmu *pmu __maybe_unused)
>                 pmu->selectable = true;
>                 pmu->is_uncore = false;
>                 pmu->perf_event_attr_init_default = arm_spe_pmu_default_config;
> -               if (!strcmp(pmu->name, "arm_spe_0"))
> +               if (strstr(pmu->name, "arm_spe"))

Why not use strstarts?

Thanks,
Ian


>                         pmu->mem_events = perf_mem_events_arm;
>         } else if (strstarts(pmu->name, HISI_PTT_PMU_NAME)) {
>                 pmu->selectable = true;
> --
> 2.34.1
>
Leo Yan June 25, 2024, 4:49 p.m. UTC | #2
Hi Ian,

On 6/24/24 17:16, Ian Rogers wrote:
> On Sun, Jun 23, 2024 at 6:34 AM Leo Yan <leo.yan@arm.com> wrote:
>>
>> A platform can have more than one Arm SPE PMU. For example, a system
>> with multiple clusters may have each cluster enabled with its own Arm
>> SPE instance. In such case, the PMU devices will be named 'arm_spe_0',
>> 'arm_spe_1', and so on.
>>
>> Currently, the tool only supports 'arm_spe_0'. This commit extends
>> support to multiple Arm SPE PMUs by detecting the substring 'arm_spe'.
>>
>> Signed-off-by: Leo Yan <leo.yan@arm.com>
>> ---
>>   tools/perf/arch/arm/util/pmu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c
>> index 8b7cb68ba1a8..29cfa1e427ed 100644
>> --- a/tools/perf/arch/arm/util/pmu.c
>> +++ b/tools/perf/arch/arm/util/pmu.c
>> @@ -27,7 +27,7 @@ void perf_pmu__arch_init(struct perf_pmu *pmu __maybe_unused)
>>                  pmu->selectable = true;
>>                  pmu->is_uncore = false;
>>                  pmu->perf_event_attr_init_default = arm_spe_pmu_default_config;
>> -               if (!strcmp(pmu->name, "arm_spe_0"))
>> +               if (strstr(pmu->name, "arm_spe"))
> 
> Why not use strstarts?

Indeed, strstarts() is better, will spin for this.

Thank for suggestion.
Leo
Namhyung Kim June 27, 2024, 10:37 p.m. UTC | #3
Hello Leo,

On Tue, Jun 25, 2024 at 05:49:16PM +0100, Leo Yan wrote:
> Hi Ian,
> 
> On 6/24/24 17:16, Ian Rogers wrote:
> > On Sun, Jun 23, 2024 at 6:34 AM Leo Yan <leo.yan@arm.com> wrote:
> > > 
> > > A platform can have more than one Arm SPE PMU. For example, a system
> > > with multiple clusters may have each cluster enabled with its own Arm
> > > SPE instance. In such case, the PMU devices will be named 'arm_spe_0',
> > > 'arm_spe_1', and so on.
> > > 
> > > Currently, the tool only supports 'arm_spe_0'. This commit extends
> > > support to multiple Arm SPE PMUs by detecting the substring 'arm_spe'.
> > > 
> > > Signed-off-by: Leo Yan <leo.yan@arm.com>
> > > ---
> > >   tools/perf/arch/arm/util/pmu.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c
> > > index 8b7cb68ba1a8..29cfa1e427ed 100644
> > > --- a/tools/perf/arch/arm/util/pmu.c
> > > +++ b/tools/perf/arch/arm/util/pmu.c
> > > @@ -27,7 +27,7 @@ void perf_pmu__arch_init(struct perf_pmu *pmu __maybe_unused)
> > >                  pmu->selectable = true;
> > >                  pmu->is_uncore = false;
> > >                  pmu->perf_event_attr_init_default = arm_spe_pmu_default_config;
> > > -               if (!strcmp(pmu->name, "arm_spe_0"))
> > > +               if (strstr(pmu->name, "arm_spe"))
> > 
> > Why not use strstarts?
> 
> Indeed, strstarts() is better, will spin for this.
> 
> Thank for suggestion.

Probably we need to check the last underscore too to prevent potential
name clashes..

Thanks,
Namhyung
diff mbox series

Patch

diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c
index 8b7cb68ba1a8..29cfa1e427ed 100644
--- a/tools/perf/arch/arm/util/pmu.c
+++ b/tools/perf/arch/arm/util/pmu.c
@@ -27,7 +27,7 @@  void perf_pmu__arch_init(struct perf_pmu *pmu __maybe_unused)
 		pmu->selectable = true;
 		pmu->is_uncore = false;
 		pmu->perf_event_attr_init_default = arm_spe_pmu_default_config;
-		if (!strcmp(pmu->name, "arm_spe_0"))
+		if (strstr(pmu->name, "arm_spe"))
 			pmu->mem_events = perf_mem_events_arm;
 	} else if (strstarts(pmu->name, HISI_PTT_PMU_NAME)) {
 		pmu->selectable = true;