* [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 @ 2026-02-05 18:36 Ian Rogers 2026-02-05 18:36 ` [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists Ian Rogers 2026-02-06 8:53 ` [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 Leo Yan 0 siblings, 2 replies; 17+ messages in thread From: Ian Rogers @ 2026-02-05 18:36 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark, linux-perf-users, linux-kernel Cc: Leo Yan 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists 2026-02-05 18:36 [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 Ian Rogers @ 2026-02-05 18:36 ` Ian Rogers 2026-02-06 10:45 ` Leo Yan 2026-02-06 8:53 ` [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 Leo Yan 1 sibling, 1 reply; 17+ messages in thread From: Ian Rogers @ 2026-02-05 18:36 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark, linux-perf-users, linux-kernel 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists 2026-02-05 18:36 ` [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists Ian Rogers @ 2026-02-06 10:45 ` Leo Yan 2026-02-06 12:33 ` Leo Yan 0 siblings, 1 reply; 17+ messages in thread From: Leo Yan @ 2026-02-06 10:45 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, linux-perf-users, linux-kernel 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 > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists 2026-02-06 10:45 ` Leo Yan @ 2026-02-06 12:33 ` Leo Yan 2026-02-06 18:57 ` Ian Rogers 0 siblings, 1 reply; 17+ messages in thread From: Leo Yan @ 2026-02-06 12:33 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, linux-perf-users, linux-kernel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists 2026-02-06 12:33 ` Leo Yan @ 2026-02-06 18:57 ` Ian Rogers 2026-02-06 21:50 ` Leo Yan 0 siblings, 1 reply; 17+ messages in thread From: Ian Rogers @ 2026-02-06 18:57 UTC (permalink / raw) To: Leo Yan Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, linux-perf-users, linux-kernel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists 2026-02-06 18:57 ` Ian Rogers @ 2026-02-06 21:50 ` Leo Yan 2026-02-06 22:16 ` Ian Rogers 0 siblings, 1 reply; 17+ messages in thread From: Leo Yan @ 2026-02-06 21:50 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, linux-perf-users, linux-kernel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists 2026-02-06 21:50 ` Leo Yan @ 2026-02-06 22:16 ` Ian Rogers 2026-02-07 0:14 ` Ian Rogers 0 siblings, 1 reply; 17+ messages in thread From: Ian Rogers @ 2026-02-06 22:16 UTC (permalink / raw) To: Leo Yan Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, linux-perf-users, linux-kernel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists 2026-02-06 22:16 ` Ian Rogers @ 2026-02-07 0:14 ` Ian Rogers 2026-02-07 0:49 ` [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups Ian Rogers 0 siblings, 1 reply; 17+ messages in thread From: Ian Rogers @ 2026-02-07 0:14 UTC (permalink / raw) To: Leo Yan Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, linux-perf-users, linux-kernel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups 2026-02-07 0:14 ` Ian Rogers @ 2026-02-07 0:49 ` Ian Rogers 2026-02-09 9:22 ` Leo Yan 2026-02-24 17:57 ` Namhyung Kim 0 siblings, 2 replies; 17+ messages in thread From: Ian Rogers @ 2026-02-07 0:49 UTC (permalink / raw) To: irogers Cc: acme, adrian.hunter, alexander.shishkin, james.clark, jolsa, leo.yan, linux-kernel, linux-perf-users, mingo, namhyung, peterz 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups 2026-02-07 0:49 ` [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups Ian Rogers @ 2026-02-09 9:22 ` Leo Yan 2026-02-09 23:06 ` Ian Rogers 2026-02-24 17:57 ` Namhyung Kim 1 sibling, 1 reply; 17+ messages in thread From: Leo Yan @ 2026-02-09 9:22 UTC (permalink / raw) To: Ian Rogers Cc: acme, adrian.hunter, alexander.shishkin, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, namhyung, peterz 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> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups 2026-02-09 9:22 ` Leo Yan @ 2026-02-09 23:06 ` Ian Rogers 2026-02-10 9:11 ` Leo Yan 0 siblings, 1 reply; 17+ messages in thread From: Ian Rogers @ 2026-02-09 23:06 UTC (permalink / raw) To: Leo Yan Cc: acme, adrian.hunter, alexander.shishkin, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, namhyung, peterz 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups 2026-02-09 23:06 ` Ian Rogers @ 2026-02-10 9:11 ` Leo Yan 0 siblings, 0 replies; 17+ messages in thread From: Leo Yan @ 2026-02-10 9:11 UTC (permalink / raw) To: Ian Rogers Cc: acme, adrian.hunter, alexander.shishkin, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, namhyung, peterz 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups 2026-02-07 0:49 ` [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups Ian Rogers 2026-02-09 9:22 ` Leo Yan @ 2026-02-24 17:57 ` Namhyung Kim 1 sibling, 0 replies; 17+ messages in thread From: Namhyung Kim @ 2026-02-24 17:57 UTC (permalink / raw) To: Ian Rogers Cc: acme, adrian.hunter, alexander.shishkin, james.clark, jolsa, leo.yan, linux-kernel, linux-perf-users, mingo, peterz 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 2026-02-05 18:36 [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 Ian Rogers 2026-02-05 18:36 ` [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists Ian Rogers @ 2026-02-06 8:53 ` Leo Yan 2026-02-06 14:57 ` Arnaldo Carvalho de Melo 2026-02-06 14:59 ` Arnaldo Carvalho de Melo 1 sibling, 2 replies; 17+ messages in thread From: Leo Yan @ 2026-02-06 8:53 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, linux-perf-users, linux-kernel 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 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 2026-02-06 8:53 ` [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 Leo Yan @ 2026-02-06 14:57 ` Arnaldo Carvalho de Melo 2026-02-06 14:59 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 17+ messages in thread From: Arnaldo Carvalho de Melo @ 2026-02-06 14:57 UTC (permalink / raw) To: Leo Yan Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, linux-perf-users, linux-kernel 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 > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 2026-02-06 8:53 ` [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 Leo Yan 2026-02-06 14:57 ` Arnaldo Carvalho de Melo @ 2026-02-06 14:59 ` Arnaldo Carvalho de Melo 2026-02-06 16:09 ` Leo Yan 1 sibling, 1 reply; 17+ messages in thread From: Arnaldo Carvalho de Melo @ 2026-02-06 14:59 UTC (permalink / raw) To: Leo Yan Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, linux-perf-users, linux-kernel 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 > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 2026-02-06 14:59 ` Arnaldo Carvalho de Melo @ 2026-02-06 16:09 ` Leo Yan 0 siblings, 0 replies; 17+ messages in thread From: Leo Yan @ 2026-02-06 16:09 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, linux-perf-users, linux-kernel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-02-24 17:57 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-05 18:36 [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 Ian Rogers 2026-02-05 18:36 ` [PATCH v1 2/2] perf metricgroup: Don't early exit if no CPUID table exists Ian Rogers 2026-02-06 10:45 ` Leo Yan 2026-02-06 12:33 ` Leo Yan 2026-02-06 18:57 ` Ian Rogers 2026-02-06 21:50 ` Leo Yan 2026-02-06 22:16 ` Ian Rogers 2026-02-07 0:14 ` Ian Rogers 2026-02-07 0:49 ` [PATCH v1] perf metricgroup: Fix metricgroup__has_metric_or_groups Ian Rogers 2026-02-09 9:22 ` Leo Yan 2026-02-09 23:06 ` Ian Rogers 2026-02-10 9:11 ` Leo Yan 2026-02-24 17:57 ` Namhyung Kim 2026-02-06 8:53 ` [PATCH v1 1/2] perf tests: build-test coverage for NO_JEVENTS=1 Leo Yan 2026-02-06 14:57 ` Arnaldo Carvalho de Melo 2026-02-06 14:59 ` Arnaldo Carvalho de Melo 2026-02-06 16:09 ` Leo Yan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox