[PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1

[PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1

[Ian Rogers]

Leo reported the NO_JEVENTS=1 build being broken and this highlighted
it being missing from the build-test, add it.

Reported-by: Leo Yan <leo.yan@arm.com>
Closes: https://lore.kernel.org/linux-perf-users/20260205175250.GC3529712@e132581.arm.com/
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/make | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/tests/make b/tools/perf/tests/make
index eb41516c0562..6587dc326d1b 100644
--- a/tools/perf/tests/make
+++ b/tools/perf/tests/make
@@ -70,6 +70,7 @@ make_python_perf_so := $(python_perf_so)
 make_debug          := DEBUG=1
 make_nondistro      := BUILD_NONDISTRO=1
 make_extra_tests    := EXTRA_TESTS=1
+make_no_jevents     := NO_JEVENTS=1
 make_jevents_all    := JEVENTS_ARCH=all
 make_no_bpf_skel    := BUILD_BPF_SKEL=0
 make_gen_vmlinux_h  := GEN_VMLINUX_H=1
@@ -144,6 +145,7 @@ ifneq ($(new_libbfd),)
 run += make_nondistro
 endif
 run += make_extra_tests
+run += make_no_jevents
 run += make_jevents_all
 run += make_no_bpf_skel
 run += make_gen_vmlinux_h
-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists

[Ian Rogers]

The failure to find a table of metrics with a CPUID shouldn't early
exit as the metric code will now also consider the default table.
When searching for a metric or metric group,
pmu_metrics_table__for_each_metric considers all tables and so the
caller doesn't need to switch the table to do this.

Fixes: c7adeb0974f1 ("perf jevents: Add set of common metrics based on default ones")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/metricgroup.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 40a1e14de418..46bf4dfeebc8 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -1562,8 +1562,6 @@ int metricgroup__parse_groups(struct evlist *perf_evlist,
 {
 	const struct pmu_metrics_table *table = pmu_metrics_table__find();
 
-	if (!table)
-		return -EINVAL;
 	if (hardware_aware_grouping)
 		pr_debug("Use hardware aware grouping instead of traditional metric grouping method\n");
 
@@ -1601,22 +1599,16 @@ static int metricgroup__has_metric_or_groups_callback(const struct pmu_metric *p
 
 bool metricgroup__has_metric_or_groups(const char *pmu, const char *metric_or_groups)
 {
-	const struct pmu_metrics_table *tables[2] = {
-		pmu_metrics_table__find(),
-		pmu_metrics_table__default(),
-	};
+	const struct pmu_metrics_table *table = pmu_metrics_table__find();
 	struct metricgroup__has_metric_data data = {
 		.pmu = pmu,
 		.metric_or_groups = metric_or_groups,
 	};
 
-	for (size_t i = 0; i < ARRAY_SIZE(tables); i++) {
-		if (pmu_metrics_table__for_each_metric(tables[i],
-							metricgroup__has_metric_or_groups_callback,
-							&data))
-			return true;
-	}
-	return false;
+	return pmu_metrics_table__for_each_metric(table,
+						  metricgroup__has_metric_or_groups_callback,
+						  &data)
+		? true : false;
 }
 
 static int metricgroup__topdown_max_level_callback(const struct pmu_metric *pm,
-- 
2.53.0.rc2.204.g2597b5adb4-goog

Re: [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1

[Leo Yan]

On Thu, Feb 05, 2026 at 10:36:02AM -0800, Ian Rogers wrote:
> Leo reported the NO_JEVENTS=1 build being broken and this highlighted
> it being missing from the build-test, add it.

To be clear, I did not say the build is broken.

The issue I reported is that `perf stat` is broken, both with
NO_JEVENTS=1 enabled and without that configuration.

With updating the commit log, this patch LGTM:

Reviewed-by: Leo Yan <leo.yan@arm.com>

> Reported-by: Leo Yan <leo.yan@arm.com>
> Closes: https://lore.kernel.org/linux-perf-users/20260205175250.GC3529712@e132581.arm.com/
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/make | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> index eb41516c0562..6587dc326d1b 100644
> --- a/tools/perf/tests/make
> +++ b/tools/perf/tests/make
> @@ -70,6 +70,7 @@ make_python_perf_so := $(python_perf_so)
>  make_debug          := DEBUG=1
>  make_nondistro      := BUILD_NONDISTRO=1
>  make_extra_tests    := EXTRA_TESTS=1
> +make_no_jevents     := NO_JEVENTS=1
>  make_jevents_all    := JEVENTS_ARCH=all
>  make_no_bpf_skel    := BUILD_BPF_SKEL=0
>  make_gen_vmlinux_h  := GEN_VMLINUX_H=1
> @@ -144,6 +145,7 @@ ifneq ($(new_libbfd),)
>  run += make_nondistro
>  endif
>  run += make_extra_tests
> +run += make_no_jevents
>  run += make_jevents_all
>  run += make_no_bpf_skel
>  run += make_gen_vmlinux_h
> -- 
> 2.53.0.rc2.204.g2597b5adb4-goog
> 

Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists

[Leo Yan]

On Thu, Feb 05, 2026 at 10:36:03AM -0800, Ian Rogers wrote:
> The failure to find a table of metrics with a CPUID shouldn't early
> exit as the metric code will now also consider the default table.
> When searching for a metric or metric group,
> pmu_metrics_table__for_each_metric considers all tables and so the
> caller doesn't need to switch the table to do this.
> 
> Fixes: c7adeb0974f1 ("perf jevents: Add set of common metrics based on default ones")
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/metricgroup.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 40a1e14de418..46bf4dfeebc8 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -1562,8 +1562,6 @@ int metricgroup__parse_groups(struct evlist *perf_evlist,
>  {
>  	const struct pmu_metrics_table *table = pmu_metrics_table__find();
>  
> -	if (!table)
> -		return -EINVAL;
>  	if (hardware_aware_grouping)
>  		pr_debug("Use hardware aware grouping instead of traditional metric grouping method\n");
>  
> @@ -1601,22 +1599,16 @@ static int metricgroup__has_metric_or_groups_callback(const struct pmu_metric *p
>  
>  bool metricgroup__has_metric_or_groups(const char *pmu, const char *metric_or_groups)
>  {
> -	const struct pmu_metrics_table *tables[2] = {
> -		pmu_metrics_table__find(),
> -		pmu_metrics_table__default(),
> -	};
> +	const struct pmu_metrics_table *table = pmu_metrics_table__find();

Here should be:

  const struct pmu_metrics_table *table = pmu_metrics_table__default();

With this change, the default metrics can work on my side:

 Performance counter stats for 'sleep 1':

                 1      context-switches                 #    434.8 cs/sec  cs_per_second     
                 0      cpu-migrations                   #      0.0 migrations/sec  migrations_per_second
                70      page-faults                      #  30439.1 faults/sec  page_faults_per_second
              2.30 msec task-clock                       #      0.0 CPUs  CPUs_utilized       
             17398      armv8_pmuv3_0/branch-misses/     #      4.3 %  branch_miss_rate       
            401746      armv8_pmuv3_0/branches/          #    174.7 M/sec  branch_frequency   
           1778245      armv8_pmuv3_0/cpu-cycles/        #      0.8 GHz  cycles_frequency     
           2341698      armv8_pmuv3_0/instructions/      #      1.3 instructions  insn_per_cycle
            680491      armv8_pmuv3_0/stalled-cycles-frontend/ #     0.38 frontend_cycles_idle      
            380360      armv8_pmuv3_0/stalled-cycles-backend/ #     0.21 backend_cycles_idle       
            380360      armv8_pmuv3_0/stalled-cycles-backend/ #     0.29 stalled_cycles_per_instruction

With above change:

Tested-by: Leo Yan <leo.yan@arm.com>


>  	struct metricgroup__has_metric_data data = {
>  		.pmu = pmu,
>  		.metric_or_groups = metric_or_groups,
>  	};
>  
> -	for (size_t i = 0; i < ARRAY_SIZE(tables); i++) {
> -		if (pmu_metrics_table__for_each_metric(tables[i],
> -							metricgroup__has_metric_or_groups_callback,
> -							&data))
> -			return true;
> -	}
> -	return false;
> +	return pmu_metrics_table__for_each_metric(table,
> +						  metricgroup__has_metric_or_groups_callback,
> +						  &data)
> +		? true : false;
>  }
>  
>  static int metricgroup__topdown_max_level_callback(const struct pmu_metric *pm,
> -- 
> 2.53.0.rc2.204.g2597b5adb4-goog
> 
> 

Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists

[Leo Yan]

On Fri, Feb 06, 2026 at 10:45:43AM +0000, Leo Yan wrote:
> On Thu, Feb 05, 2026 at 10:36:03AM -0800, Ian Rogers wrote:
> > The failure to find a table of metrics with a CPUID shouldn't early
> > exit as the metric code will now also consider the default table.
> > When searching for a metric or metric group,
> > pmu_metrics_table__for_each_metric considers all tables and so the
> > caller doesn't need to switch the table to do this.
> > 
> > Fixes: c7adeb0974f1 ("perf jevents: Add set of common metrics based on default ones")
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/util/metricgroup.c | 18 +++++-------------
> >  1 file changed, 5 insertions(+), 13 deletions(-)
> > 
> > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> > index 40a1e14de418..46bf4dfeebc8 100644
> > --- a/tools/perf/util/metricgroup.c
> > +++ b/tools/perf/util/metricgroup.c
> > @@ -1562,8 +1562,6 @@ int metricgroup__parse_groups(struct evlist *perf_evlist,
> >  {
> >  	const struct pmu_metrics_table *table = pmu_metrics_table__find();
> >  
> > -	if (!table)
> > -		return -EINVAL;
> >  	if (hardware_aware_grouping)
> >  		pr_debug("Use hardware aware grouping instead of traditional metric grouping method\n");
> >  
> > @@ -1601,22 +1599,16 @@ static int metricgroup__has_metric_or_groups_callback(const struct pmu_metric *p
> >  
> >  bool metricgroup__has_metric_or_groups(const char *pmu, const char *metric_or_groups)
> >  {
> > -	const struct pmu_metrics_table *tables[2] = {
> > -		pmu_metrics_table__find(),
> > -		pmu_metrics_table__default(),
> > -	};
> > +	const struct pmu_metrics_table *table = pmu_metrics_table__find();
> 
> Here should be:
> 
>   const struct pmu_metrics_table *table = pmu_metrics_table__default();

Or, I think you should not change metricgroup__has_metric_or_groups().
The change only in metricgroup__parse_groups() can fix my issue.

Thanks,
Leo

Re: [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1

[Arnaldo Carvalho de Melo]

On Fri, Feb 06, 2026 at 08:53:49AM +0000, Leo Yan wrote:
> On Thu, Feb 05, 2026 at 10:36:02AM -0800, Ian Rogers wrote:
> > Leo reported the NO_JEVENTS=1 build being broken and this highlighted
> > it being missing from the build-test, add it.
> 
> To be clear, I did not say the build is broken.
> 
> The issue I reported is that `perf stat` is broken, both with
> NO_JEVENTS=1 enabled and without that configuration.
> 
> With updating the commit log, this patch LGTM:
> 
> Reviewed-by: Leo Yan <leo.yan@arm.com>

Thanks, applied to perf-tools-next,

- Arnaldo
 
> > Reported-by: Leo Yan <leo.yan@arm.com>
> > Closes: https://lore.kernel.org/linux-perf-users/20260205175250.GC3529712@e132581.arm.com/
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/tests/make | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> > index eb41516c0562..6587dc326d1b 100644
> > --- a/tools/perf/tests/make
> > +++ b/tools/perf/tests/make
> > @@ -70,6 +70,7 @@ make_python_perf_so := $(python_perf_so)
> >  make_debug          := DEBUG=1
> >  make_nondistro      := BUILD_NONDISTRO=1
> >  make_extra_tests    := EXTRA_TESTS=1
> > +make_no_jevents     := NO_JEVENTS=1
> >  make_jevents_all    := JEVENTS_ARCH=all
> >  make_no_bpf_skel    := BUILD_BPF_SKEL=0
> >  make_gen_vmlinux_h  := GEN_VMLINUX_H=1
> > @@ -144,6 +145,7 @@ ifneq ($(new_libbfd),)
> >  run += make_nondistro
> >  endif
> >  run += make_extra_tests
> > +run += make_no_jevents
> >  run += make_jevents_all
> >  run += make_no_bpf_skel
> >  run += make_gen_vmlinux_h
> > -- 
> > 2.53.0.rc2.204.g2597b5adb4-goog
> > 

Re: [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1

[Arnaldo Carvalho de Melo]

On Fri, Feb 06, 2026 at 08:53:49AM +0000, Leo Yan wrote:
> On Thu, Feb 05, 2026 at 10:36:02AM -0800, Ian Rogers wrote:
> > Leo reported the NO_JEVENTS=1 build being broken and this highlighted
> > it being missing from the build-test, add it.
> 
> To be clear, I did not say the build is broken.
> 
> The issue I reported is that `perf stat` is broken, both with
> NO_JEVENTS=1 enabled and without that configuration.
> 
> With updating the commit log, this patch LGTM:

I did it and will apply the Reviewed-by to patch 2/2 too, ok?

- Arnaldo
 
> Reviewed-by: Leo Yan <leo.yan@arm.com>
> 
> > Reported-by: Leo Yan <leo.yan@arm.com>
> > Closes: https://lore.kernel.org/linux-perf-users/20260205175250.GC3529712@e132581.arm.com/
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/tests/make | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> > index eb41516c0562..6587dc326d1b 100644
> > --- a/tools/perf/tests/make
> > +++ b/tools/perf/tests/make
> > @@ -70,6 +70,7 @@ make_python_perf_so := $(python_perf_so)
> >  make_debug          := DEBUG=1
> >  make_nondistro      := BUILD_NONDISTRO=1
> >  make_extra_tests    := EXTRA_TESTS=1
> > +make_no_jevents     := NO_JEVENTS=1
> >  make_jevents_all    := JEVENTS_ARCH=all
> >  make_no_bpf_skel    := BUILD_BPF_SKEL=0
> >  make_gen_vmlinux_h  := GEN_VMLINUX_H=1
> > @@ -144,6 +145,7 @@ ifneq ($(new_libbfd),)
> >  run += make_nondistro
> >  endif
> >  run += make_extra_tests
> > +run += make_no_jevents
> >  run += make_jevents_all
> >  run += make_no_bpf_skel
> >  run += make_gen_vmlinux_h
> > -- 
> > 2.53.0.rc2.204.g2597b5adb4-goog
> > 

Re: [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1

[Leo Yan]

On Fri, Feb 06, 2026 at 11:59:25AM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Feb 06, 2026 at 08:53:49AM +0000, Leo Yan wrote:
> > On Thu, Feb 05, 2026 at 10:36:02AM -0800, Ian Rogers wrote:
> > > Leo reported the NO_JEVENTS=1 build being broken and this highlighted
> > > it being missing from the build-test, add it.
> > 
> > To be clear, I did not say the build is broken.
> > 
> > The issue I reported is that `perf stat` is broken, both with
> > NO_JEVENTS=1 enabled and without that configuration.
> > 
> > With updating the commit log, this patch LGTM:
> 
> I did it and will apply the Reviewed-by to patch 2/2 too, ok?

Yes, this is fine for me.

Just remind, in patch 02, based on test, we only need to apply the
change in metricgroup__parse_groups() for removing the "!table" check.

The change for metricgroup__has_metric_or_groups() is not quite right,
as we should keep the searching metrics with
pmu_metrics_table__default().

Thanks,
Leo

Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists

[Ian Rogers]

On Fri, Feb 6, 2026 at 4:33 AM Leo Yan <leo.yan@arm.com> wrote:
>
> On Fri, Feb 06, 2026 at 10:45:43AM +0000, Leo Yan wrote:
> > On Thu, Feb 05, 2026 at 10:36:03AM -0800, Ian Rogers wrote:
> > > The failure to find a table of metrics with a CPUID shouldn't early
> > > exit as the metric code will now also consider the default table.
> > > When searching for a metric or metric group,
> > > pmu_metrics_table__for_each_metric considers all tables and so the
> > > caller doesn't need to switch the table to do this.
> > >
> > > Fixes: c7adeb0974f1 ("perf jevents: Add set of common metrics based on default ones")
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > >  tools/perf/util/metricgroup.c | 18 +++++-------------
> > >  1 file changed, 5 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> > > index 40a1e14de418..46bf4dfeebc8 100644
> > > --- a/tools/perf/util/metricgroup.c
> > > +++ b/tools/perf/util/metricgroup.c
> > > @@ -1562,8 +1562,6 @@ int metricgroup__parse_groups(struct evlist *perf_evlist,
> > >  {
> > >     const struct pmu_metrics_table *table = pmu_metrics_table__find();
> > >
> > > -   if (!table)
> > > -           return -EINVAL;
> > >     if (hardware_aware_grouping)
> > >             pr_debug("Use hardware aware grouping instead of traditional metric grouping method\n");
> > >
> > > @@ -1601,22 +1599,16 @@ static int metricgroup__has_metric_or_groups_callback(const struct pmu_metric *p
> > >
> > >  bool metricgroup__has_metric_or_groups(const char *pmu, const char *metric_or_groups)
> > >  {
> > > -   const struct pmu_metrics_table *tables[2] = {
> > > -           pmu_metrics_table__find(),
> > > -           pmu_metrics_table__default(),
> > > -   };
> > > +   const struct pmu_metrics_table *table = pmu_metrics_table__find();
> >
> > Here should be:
> >
> >   const struct pmu_metrics_table *table = pmu_metrics_table__default();
>
> Or, I think you should not change metricgroup__has_metric_or_groups().
> The change only in metricgroup__parse_groups() can fix my issue.

This is wrong and the patch I tested I believe to be correct. The
table here is CPUID associated table and all the metric code will
bottom out in metricgroup__for_each_metric where the CPUID table is
the table parameter (its a parameter because of testing where we force
the machine/model to "testarch"):
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/metricgroup.c?h=perf-tools-next#n430
```
int metricgroup__for_each_metric(const struct pmu_metrics_table
*table, pmu_metric_iter_fn fn, void *data)
{
...
const struct pmu_metrics_table *tables[2] = {
    table,
    pmu_metrics_table__default(),
};

for (size_t i = 0; i < ARRAY_SIZE(tables); i++) {
    int ret;

    if (!tables[i])
        continue;
```
If you make the table here the default table then you will process the
default table twice in metricgroup__for_each_metric which will mean
double metric look ups, etc. So I think the patch is correct and
passed my testing both with and without NO_JEVENTS=1.

Thanks,
Ian

> Thanks,
> Leo

Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists

[Leo Yan]

On Fri, Feb 06, 2026 at 10:57:54AM -0800, Ian Rogers wrote:

[...]

> > > >  bool metricgroup__has_metric_or_groups(const char *pmu, const char *metric_or_groups)
> > > >  {
> > > > -   const struct pmu_metrics_table *tables[2] = {
> > > > -           pmu_metrics_table__find(),
> > > > -           pmu_metrics_table__default(),
> > > > -   };
> > > > +   const struct pmu_metrics_table *table = pmu_metrics_table__find();
> > >
> > > Here should be:
> > >
> > >   const struct pmu_metrics_table *table = pmu_metrics_table__default();
> >
> > Or, I think you should not change metricgroup__has_metric_or_groups().
> > The change only in metricgroup__parse_groups() can fix my issue.
> 
> This is wrong and the patch I tested I believe to be correct.

It is very likely you are not working on the issue same as mine.

> The table here is CPUID associated table and all the metric code will

As you said, the key point is CPUID.  On my board, as no corresponding
json file to support the CPU variant, it fails to find any table based
on the CPUID and pmu_metrics_table__find() always returns NULL.

The failure flow is:

  add_default_events()
    if (!metricgroup__has_metric_or_groups(pmu, default_metricgroup_names[i]))
        continue;
     ^^^
     Always return false

As a result, metricgroup__parse_groups() has no chance to be invoked
and no any events are added to the ev list.

This is why I suggested to keep default table in
metricgroup__has_metric_or_groups(), it can rallback to find default
table when CPUID is not supported.

> bottom out in metricgroup__for_each_metric where the CPUID table is
> the table parameter (its a parameter because of testing where we force
> the machine/model to "testarch"):
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/metricgroup.c?h=perf-tools-next#n430
> ```
> int metricgroup__for_each_metric(const struct pmu_metrics_table
> *table, pmu_metric_iter_fn fn, void *data)
> {
> ...
> const struct pmu_metrics_table *tables[2] = {
>     table,
>     pmu_metrics_table__default(),
> };
> 
> for (size_t i = 0; i < ARRAY_SIZE(tables); i++) {
>     int ret;
> 
>     if (!tables[i])
>         continue;
> ```
> If you make the table here the default table then you will process the
> default table twice in metricgroup__for_each_metric which will mean
> double metric look ups, etc.

In my case, it will not have double metric issue, as
pmu_metrics_table__find() cannot find any table, a NULL table pointer
will be passed to metricgroup__for_each_metric(), then only
pmu_metrics_table__default() will be used.

As a side question, using a single CPUID to look up metrics can be
error-prone on asymmetric systems (such as Arm big.LITTLE).  Since only
the first CPU ID is used to search the metrics table, could this cause
issues for other CPU variants?

Thanks,
Leo

Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists

[Ian Rogers]

On Fri, Feb 6, 2026 at 1:50 PM Leo Yan <leo.yan@arm.com> wrote:
>
> On Fri, Feb 06, 2026 at 10:57:54AM -0800, Ian Rogers wrote:
>
> [...]
>
> > > > >  bool metricgroup__has_metric_or_groups(const char *pmu, const char *metric_or_groups)
> > > > >  {
> > > > > -   const struct pmu_metrics_table *tables[2] = {
> > > > > -           pmu_metrics_table__find(),
> > > > > -           pmu_metrics_table__default(),
> > > > > -   };
> > > > > +   const struct pmu_metrics_table *table = pmu_metrics_table__find();
> > > >
> > > > Here should be:
> > > >
> > > >   const struct pmu_metrics_table *table = pmu_metrics_table__default();
> > >
> > > Or, I think you should not change metricgroup__has_metric_or_groups().
> > > The change only in metricgroup__parse_groups() can fix my issue.
> >
> > This is wrong and the patch I tested I believe to be correct.
>
> It is very likely you are not working on the issue same as mine.
>
> > The table here is CPUID associated table and all the metric code will
>
> As you said, the key point is CPUID.  On my board, as no corresponding
> json file to support the CPU variant, it fails to find any table based
> on the CPUID and pmu_metrics_table__find() always returns NULL.
>
> The failure flow is:
>
>   add_default_events()
>     if (!metricgroup__has_metric_or_groups(pmu, default_metricgroup_names[i]))
>         continue;
>      ^^^
>      Always return false
>
> As a result, metricgroup__parse_groups() has no chance to be invoked
> and no any events are added to the ev list.
>
> This is why I suggested to keep default table in
> metricgroup__has_metric_or_groups(), it can rallback to find default
> table when CPUID is not supported.
>
> > bottom out in metricgroup__for_each_metric where the CPUID table is
> > the table parameter (its a parameter because of testing where we force
> > the machine/model to "testarch"):
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/metricgroup.c?h=perf-tools-next#n430
> > ```
> > int metricgroup__for_each_metric(const struct pmu_metrics_table
> > *table, pmu_metric_iter_fn fn, void *data)
> > {
> > ...
> > const struct pmu_metrics_table *tables[2] = {
> >     table,
> >     pmu_metrics_table__default(),
> > };
> >
> > for (size_t i = 0; i < ARRAY_SIZE(tables); i++) {
> >     int ret;
> >
> >     if (!tables[i])
> >         continue;
> > ```
> > If you make the table here the default table then you will process the
> > default table twice in metricgroup__for_each_metric which will mean
> > double metric look ups, etc.
>
> In my case, it will not have double metric issue, as
> pmu_metrics_table__find() cannot find any table, a NULL table pointer
> will be passed to metricgroup__for_each_metric(), then only
> pmu_metrics_table__default() will be used.

Which is the table you want to be used to such an extent that you are
proposing changing this patch to put it twice in the tables array in
the snippet of code above. Also note that the code skips the NULL,
failed to look up table, which is why this patch just needs to avoid
the early return on a missing table to address the problem of the
default table not being considered.

> As a side question, using a single CPUID to look up metrics can be
> error-prone on asymmetric systems (such as Arm big.LITTLE).  Since only
> the first CPU ID is used to search the metrics table, could this cause
> issues for other CPU variants?

There's logic to use the PMU for the cpumask and then to get the CPUID
for the ARM CPU associated with the CPUs in the cpumask. It is likely
bitrotten, but the problem has had some work on it in the past. It is
untested, to my knowledge.

Thanks,
Ian

> Thanks,
> Leo

Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists

[Ian Rogers]

On Fri, Feb 6, 2026 at 2:16 PM Ian Rogers <irogers@google.com> wrote:
>
> On Fri, Feb 6, 2026 at 1:50 PM Leo Yan <leo.yan@arm.com> wrote:
> >
> > On Fri, Feb 06, 2026 at 10:57:54AM -0800, Ian Rogers wrote:
> >
> > [...]
> >
> > > > > >  bool metricgroup__has_metric_or_groups(const char *pmu, const char *metric_or_groups)
> > > > > >  {
> > > > > > -   const struct pmu_metrics_table *tables[2] = {
> > > > > > -           pmu_metrics_table__find(),
> > > > > > -           pmu_metrics_table__default(),
> > > > > > -   };
> > > > > > +   const struct pmu_metrics_table *table = pmu_metrics_table__find();
> > > > >
> > > > > Here should be:
> > > > >
> > > > >   const struct pmu_metrics_table *table = pmu_metrics_table__default();
> > > >
> > > > Or, I think you should not change metricgroup__has_metric_or_groups().
> > > > The change only in metricgroup__parse_groups() can fix my issue.
> > >
> > > This is wrong and the patch I tested I believe to be correct.
> >
> > It is very likely you are not working on the issue same as mine.
> >
> > > The table here is CPUID associated table and all the metric code will
> >
> > As you said, the key point is CPUID.  On my board, as no corresponding
> > json file to support the CPU variant, it fails to find any table based
> > on the CPUID and pmu_metrics_table__find() always returns NULL.
> >
> > The failure flow is:
> >
> >   add_default_events()
> >     if (!metricgroup__has_metric_or_groups(pmu, default_metricgroup_names[i]))
> >         continue;
> >      ^^^
> >      Always return false
> >
> > As a result, metricgroup__parse_groups() has no chance to be invoked
> > and no any events are added to the ev list.
> >
> > This is why I suggested to keep default table in
> > metricgroup__has_metric_or_groups(), it can rallback to find default
> > table when CPUID is not supported.
> >
> > > bottom out in metricgroup__for_each_metric where the CPUID table is
> > > the table parameter (its a parameter because of testing where we force
> > > the machine/model to "testarch"):
> > > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/metricgroup.c?h=perf-tools-next#n430
> > > ```
> > > int metricgroup__for_each_metric(const struct pmu_metrics_table
> > > *table, pmu_metric_iter_fn fn, void *data)
> > > {
> > > ...
> > > const struct pmu_metrics_table *tables[2] = {
> > >     table,
> > >     pmu_metrics_table__default(),
> > > };
> > >
> > > for (size_t i = 0; i < ARRAY_SIZE(tables); i++) {
> > >     int ret;
> > >
> > >     if (!tables[i])
> > >         continue;
> > > ```
> > > If you make the table here the default table then you will process the
> > > default table twice in metricgroup__for_each_metric which will mean
> > > double metric look ups, etc.
> >
> > In my case, it will not have double metric issue, as
> > pmu_metrics_table__find() cannot find any table, a NULL table pointer
> > will be passed to metricgroup__for_each_metric(), then only
> > pmu_metrics_table__default() will be used.
>
> Which is the table you want to be used to such an extent that you are
> proposing changing this patch to put it twice in the tables array in
> the snippet of code above. Also note that the code skips the NULL,
> failed to look up table, which is why this patch just needs to avoid
> the early return on a missing table to address the problem of the
> default table not being considered.
>
> > As a side question, using a single CPUID to look up metrics can be
> > error-prone on asymmetric systems (such as Arm big.LITTLE).  Since only
> > the first CPU ID is used to search the metrics table, could this cause
> > issues for other CPU variants?
>
> There's logic to use the PMU for the cpumask and then to get the CPUID
> for the ARM CPU associated with the CPUs in the cpumask. It is likely
> bitrotten, but the problem has had some work on it in the past. It is
> untested, to my knowledge.

Ah, the issue is that metricgroup__has_metric_or_groups is using
pmu_metrics_table__for_each_metric and not
metricgroup__for_each_metric:
 ```
-       return pmu_metrics_table__for_each_metric(table,
-
metricgroup__has_metric_or_groups_callback,
-                                                 &data)
+       return metricgroup__for_each_metric(table,
+
metricgroup__has_metric_or_groups_callback,
+
```
I'll send this out.

Thanks,
Ian

> Thanks,
> Ian
>
> > Thanks,
> > Leo

[PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups

[Ian Rogers]

Use metricgroup__for_each_metric rather than
pmu_metrics_table__for_each_metric that combines the default metric
table with, a potentially empty, CPUID table.

Fixes: cee275edcdb1 ("perf metricgroup: Don't early exit if no CPUID table exists")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/metricgroup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 46bf4dfeebc8..7e39d469111b 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -1605,9 +1605,9 @@ bool metricgroup__has_metric_or_groups(const char *pmu, const char *metric_or_gr
 		.metric_or_groups = metric_or_groups,
 	};
 
-	return pmu_metrics_table__for_each_metric(table,
-						  metricgroup__has_metric_or_groups_callback,
-						  &data)
+	return metricgroup__for_each_metric(table,
+					    metricgroup__has_metric_or_groups_callback,
+					    &data)
 		? true : false;
 }
 
-- 
2.53.0.239.g8d8fc8a987-goog

Re: [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups

[Leo Yan]

On Fri, Feb 06, 2026 at 04:49:56PM -0800, Ian Rogers wrote:
> Use metricgroup__for_each_metric rather than
> pmu_metrics_table__for_each_metric that combines the default metric
> table with, a potentially empty, CPUID table.
> 
> Fixes: cee275edcdb1 ("perf metricgroup: Don't early exit if no CPUID table exists")
> Signed-off-by: Ian Rogers <irogers@google.com>

Reviewed-by: Leo Yan <leo.yan@arm.com>
Tested-by: Leo Yan <leo.yan@arm.com>

Re: [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups

[Ian Rogers]

On Mon, Feb 9, 2026 at 1:22 AM Leo Yan <leo.yan@arm.com> wrote:
>
> On Fri, Feb 06, 2026 at 04:49:56PM -0800, Ian Rogers wrote:
> > Use metricgroup__for_each_metric rather than
> > pmu_metrics_table__for_each_metric that combines the default metric
> > table with, a potentially empty, CPUID table.
> >
> > Fixes: cee275edcdb1 ("perf metricgroup: Don't early exit if no CPUID table exists")
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> Reviewed-by: Leo Yan <leo.yan@arm.com>
> Tested-by: Leo Yan <leo.yan@arm.com>

Thanks! Sorry again for the earlier confusion on this.

Ian

Re: [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups

[Leo Yan]

On Mon, Feb 09, 2026 at 03:06:27PM -0800, Ian Rogers wrote:
> On Mon, Feb 9, 2026 at 1:22 AM Leo Yan <leo.yan@arm.com> wrote:
> >
> > On Fri, Feb 06, 2026 at 04:49:56PM -0800, Ian Rogers wrote:
> > > Use metricgroup__for_each_metric rather than
> > > pmu_metrics_table__for_each_metric that combines the default metric
> > > table with, a potentially empty, CPUID table.
> > >
> > > Fixes: cee275edcdb1 ("perf metricgroup: Don't early exit if no CPUID table exists")
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> >
> > Reviewed-by: Leo Yan <leo.yan@arm.com>
> > Tested-by: Leo Yan <leo.yan@arm.com>
> 
> Thanks! Sorry again for the earlier confusion on this.

No worries, thanks for quick fix.

Re: [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups

[Namhyung Kim]

On Fri, 06 Feb 2026 16:49:56 -0800, Ian Rogers wrote:
> Use metricgroup__for_each_metric rather than
> pmu_metrics_table__for_each_metric that combines the default metric
> table with, a potentially empty, CPUID table.
> 
> 
Applied to perf-tools-next, thanks!

Best regards,
Namhyung