* Re: [PATCH v2] perf metricgroup: Avoid scanning unnecessary PMUs for identifier match [not found] ` <20260430161725.3131665-1-irogers@google.com> @ 2026-05-05 21:24 ` Ian Rogers 2026-05-06 9:12 ` James Clark 1 sibling, 0 replies; 4+ messages in thread From: Ian Rogers @ 2026-05-05 21:24 UTC (permalink / raw) To: namhyung, acme, james.clark Cc: adrian.hunter, leo.yan, linux-kernel, linux-perf-users, mingo, peterz, thomas.falcon On Thu, Apr 30, 2026 at 9:17 AM Ian Rogers <irogers@google.com> wrote: > > Only uncore PMUs can have an identifier, so add an optimized > perf_pmus__scan routine for that case to avoid all PMU types being > created. > > Signed-off-by: Ian Rogers <irogers@google.com> This passes Sashiko review (green). I didn't copy James Clark's reviewed-by tag given the metricgroup code rewrite to avoid the loop, but I think this is ready to land. Thanks, Ian > --- > tools/perf/util/metricgroup.c | 11 +++-------- > tools/perf/util/pmus.c | 18 +++++++++++++++++- > tools/perf/util/pmus.h | 1 + > 3 files changed, 21 insertions(+), 9 deletions(-) > > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c > index 4db9578efd81..5a489e97c413 100644 > --- a/tools/perf/util/metricgroup.c > +++ b/tools/perf/util/metricgroup.c > @@ -415,14 +415,9 @@ static int metricgroup__sys_event_iter(const struct pmu_metric *pm, > if (!pm->metric_expr || !pm->compat) > return 0; > > - while ((pmu = perf_pmus__scan(pmu))) { > - > - if (!pmu->id || !pmu_uncore_identifier_match(pm->compat, pmu->id)) > - continue; > - > - return d->fn(pm, table, d->data); > - } > - return 0; > + /* Only process with the iterator if there is a a PMU that matches the ID. */ > + pmu = perf_pmus__scan_for_uncore_id(pmu, pm->compat); > + return pmu ? d->fn(pm, table, d->data) : 0; > } > > int metricgroup__for_each_metric(const struct pmu_metrics_table *table, pmu_metric_iter_fn fn, > diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c > index 9a2023ceeefd..5e3f571450fe 100644 > --- a/tools/perf/util/pmus.c > +++ b/tools/perf/util/pmus.c > @@ -409,7 +409,7 @@ struct perf_pmu *perf_pmus__scan_matching_wildcard(struct perf_pmu *pmu, const c > if (!pmu) { > /* > * Core PMUs, other sysfs PMUs and tool PMU can have any name or > - * aren't wother optimizing for. > + * aren't worth optimizing for. > */ > unsigned int to_read_pmus = PERF_TOOL_PMU_TYPE_PE_CORE_MASK | > PERF_TOOL_PMU_TYPE_PE_OTHER_MASK | > @@ -486,6 +486,22 @@ static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu) > return NULL; > } > > +struct perf_pmu *perf_pmus__scan_for_uncore_id(struct perf_pmu *pmu, const char *compat) > +{ > + if (!pmu) { > + /* Only uncore PMUs can have identifiers. */ > + unsigned int to_read_pmus = PERF_TOOL_PMU_TYPE_PE_OTHER_MASK; > + > + pmu_read_sysfs(to_read_pmus); > + pmu = list_prepare_entry(pmu, &other_pmus, list); > + } > + list_for_each_entry_continue(pmu, &other_pmus, list) { > + if (pmu->id && pmu_uncore_identifier_match(compat, pmu->id)) > + return pmu; > + } > + return NULL; > +} > + > const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str) > { > struct perf_pmu *pmu = NULL; > diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h > index 7cb36863711a..0d55edb3f2fc 100644 > --- a/tools/perf/util/pmus.h > +++ b/tools/perf/util/pmus.h > @@ -23,6 +23,7 @@ struct perf_pmu *perf_pmus__scan(struct perf_pmu *pmu); > struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu); > struct perf_pmu *perf_pmus__scan_for_event(struct perf_pmu *pmu, const char *event); > struct perf_pmu *perf_pmus__scan_matching_wildcard(struct perf_pmu *pmu, const char *wildcard); > +struct perf_pmu *perf_pmus__scan_for_uncore_id(struct perf_pmu *pmu, const char *compat); > > const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str); > > -- > 2.54.0.545.g6539524ca2-goog > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] perf metricgroup: Avoid scanning unnecessary PMUs for identifier match [not found] ` <20260430161725.3131665-1-irogers@google.com> 2026-05-05 21:24 ` [PATCH v2] perf metricgroup: Avoid scanning unnecessary PMUs for identifier match Ian Rogers @ 2026-05-06 9:12 ` James Clark 2026-05-14 17:39 ` Ian Rogers 1 sibling, 1 reply; 4+ messages in thread From: James Clark @ 2026-05-06 9:12 UTC (permalink / raw) To: Ian Rogers Cc: adrian.hunter, leo.yan, linux-kernel, linux-perf-users, mingo, peterz, thomas.falcon, namhyung, acme On 30/04/2026 5:17 pm, Ian Rogers wrote: > Only uncore PMUs can have an identifier, so add an optimized > perf_pmus__scan routine for that case to avoid all PMU types being > created. > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/util/metricgroup.c | 11 +++-------- > tools/perf/util/pmus.c | 18 +++++++++++++++++- > tools/perf/util/pmus.h | 1 + > 3 files changed, 21 insertions(+), 9 deletions(-) > > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c > index 4db9578efd81..5a489e97c413 100644 > --- a/tools/perf/util/metricgroup.c > +++ b/tools/perf/util/metricgroup.c > @@ -415,14 +415,9 @@ static int metricgroup__sys_event_iter(const struct pmu_metric *pm, > if (!pm->metric_expr || !pm->compat) > return 0; > > - while ((pmu = perf_pmus__scan(pmu))) { > - > - if (!pmu->id || !pmu_uncore_identifier_match(pm->compat, pmu->id)) > - continue; > - > - return d->fn(pm, table, d->data); > - } > - return 0; > + /* Only process with the iterator if there is a a PMU that matches the ID. */ > + pmu = perf_pmus__scan_for_uncore_id(pmu, pm->compat); > + return pmu ? d->fn(pm, table, d->data) : 0; > } > > int metricgroup__for_each_metric(const struct pmu_metrics_table *table, pmu_metric_iter_fn fn, > diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c > index 9a2023ceeefd..5e3f571450fe 100644 > --- a/tools/perf/util/pmus.c > +++ b/tools/perf/util/pmus.c > @@ -409,7 +409,7 @@ struct perf_pmu *perf_pmus__scan_matching_wildcard(struct perf_pmu *pmu, const c > if (!pmu) { > /* > * Core PMUs, other sysfs PMUs and tool PMU can have any name or > - * aren't wother optimizing for. > + * aren't worth optimizing for. > */ > unsigned int to_read_pmus = PERF_TOOL_PMU_TYPE_PE_CORE_MASK | > PERF_TOOL_PMU_TYPE_PE_OTHER_MASK | > @@ -486,6 +486,22 @@ static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu) > return NULL; > } > > +struct perf_pmu *perf_pmus__scan_for_uncore_id(struct perf_pmu *pmu, const char *compat) > +{ > + if (!pmu) { > + /* Only uncore PMUs can have identifiers. */ > + unsigned int to_read_pmus = PERF_TOOL_PMU_TYPE_PE_OTHER_MASK; > + > + pmu_read_sysfs(to_read_pmus); > + pmu = list_prepare_entry(pmu, &other_pmus, list); > + } > + list_for_each_entry_continue(pmu, &other_pmus, list) { > + if (pmu->id && pmu_uncore_identifier_match(compat, pmu->id)) > + return pmu; > + } > + return NULL; > +} > + > const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str) > { > struct perf_pmu *pmu = NULL; > diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h > index 7cb36863711a..0d55edb3f2fc 100644 > --- a/tools/perf/util/pmus.h > +++ b/tools/perf/util/pmus.h > @@ -23,6 +23,7 @@ struct perf_pmu *perf_pmus__scan(struct perf_pmu *pmu); > struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu); > struct perf_pmu *perf_pmus__scan_for_event(struct perf_pmu *pmu, const char *event); > struct perf_pmu *perf_pmus__scan_matching_wildcard(struct perf_pmu *pmu, const char *wildcard); > +struct perf_pmu *perf_pmus__scan_for_uncore_id(struct perf_pmu *pmu, const char *compat); > > const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str); > Reviewed-by: James Clark <james.clark@linaro.org> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] perf metricgroup: Avoid scanning unnecessary PMUs for identifier match 2026-05-06 9:12 ` James Clark @ 2026-05-14 17:39 ` Ian Rogers 2026-05-14 23:58 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 4+ messages in thread From: Ian Rogers @ 2026-05-14 17:39 UTC (permalink / raw) To: James Clark, acme, namhyung Cc: adrian.hunter, leo.yan, linux-kernel, linux-perf-users, mingo, peterz, thomas.falcon On Wed, May 6, 2026 at 2:12 AM James Clark <james.clark@linaro.org> wrote: > > > > On 30/04/2026 5:17 pm, Ian Rogers wrote: > > Only uncore PMUs can have an identifier, so add an optimized > > perf_pmus__scan routine for that case to avoid all PMU types being > > created. > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > tools/perf/util/metricgroup.c | 11 +++-------- > > tools/perf/util/pmus.c | 18 +++++++++++++++++- > > tools/perf/util/pmus.h | 1 + > > 3 files changed, 21 insertions(+), 9 deletions(-) > > > > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c > > index 4db9578efd81..5a489e97c413 100644 > > --- a/tools/perf/util/metricgroup.c > > +++ b/tools/perf/util/metricgroup.c > > @@ -415,14 +415,9 @@ static int metricgroup__sys_event_iter(const struct pmu_metric *pm, > > if (!pm->metric_expr || !pm->compat) > > return 0; > > > > - while ((pmu = perf_pmus__scan(pmu))) { > > - > > - if (!pmu->id || !pmu_uncore_identifier_match(pm->compat, pmu->id)) > > - continue; > > - > > - return d->fn(pm, table, d->data); > > - } > > - return 0; > > + /* Only process with the iterator if there is a a PMU that matches the ID. */ > > + pmu = perf_pmus__scan_for_uncore_id(pmu, pm->compat); > > + return pmu ? d->fn(pm, table, d->data) : 0; > > } > > > > int metricgroup__for_each_metric(const struct pmu_metrics_table *table, pmu_metric_iter_fn fn, > > diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c > > index 9a2023ceeefd..5e3f571450fe 100644 > > --- a/tools/perf/util/pmus.c > > +++ b/tools/perf/util/pmus.c > > @@ -409,7 +409,7 @@ struct perf_pmu *perf_pmus__scan_matching_wildcard(struct perf_pmu *pmu, const c > > if (!pmu) { > > /* > > * Core PMUs, other sysfs PMUs and tool PMU can have any name or > > - * aren't wother optimizing for. > > + * aren't worth optimizing for. > > */ > > unsigned int to_read_pmus = PERF_TOOL_PMU_TYPE_PE_CORE_MASK | > > PERF_TOOL_PMU_TYPE_PE_OTHER_MASK | > > @@ -486,6 +486,22 @@ static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu) > > return NULL; > > } > > > > +struct perf_pmu *perf_pmus__scan_for_uncore_id(struct perf_pmu *pmu, const char *compat) > > +{ > > + if (!pmu) { > > + /* Only uncore PMUs can have identifiers. */ > > + unsigned int to_read_pmus = PERF_TOOL_PMU_TYPE_PE_OTHER_MASK; > > + > > + pmu_read_sysfs(to_read_pmus); > > + pmu = list_prepare_entry(pmu, &other_pmus, list); > > + } > > + list_for_each_entry_continue(pmu, &other_pmus, list) { > > + if (pmu->id && pmu_uncore_identifier_match(compat, pmu->id)) > > + return pmu; > > + } > > + return NULL; > > +} > > + > > const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str) > > { > > struct perf_pmu *pmu = NULL; > > diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h > > index 7cb36863711a..0d55edb3f2fc 100644 > > --- a/tools/perf/util/pmus.h > > +++ b/tools/perf/util/pmus.h > > @@ -23,6 +23,7 @@ struct perf_pmu *perf_pmus__scan(struct perf_pmu *pmu); > > struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu); > > struct perf_pmu *perf_pmus__scan_for_event(struct perf_pmu *pmu, const char *event); > > struct perf_pmu *perf_pmus__scan_matching_wildcard(struct perf_pmu *pmu, const char *wildcard); > > +struct perf_pmu *perf_pmus__scan_for_uncore_id(struct perf_pmu *pmu, const char *compat); > > > > const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str); > > > > Reviewed-by: James Clark <james.clark@linaro.org> Ping. Thanks, Ian ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] perf metricgroup: Avoid scanning unnecessary PMUs for identifier match 2026-05-14 17:39 ` Ian Rogers @ 2026-05-14 23:58 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 4+ messages in thread From: Arnaldo Carvalho de Melo @ 2026-05-14 23:58 UTC (permalink / raw) To: Ian Rogers Cc: James Clark, namhyung, adrian.hunter, leo.yan, linux-kernel, linux-perf-users, mingo, peterz, thomas.falcon On Thu, May 14, 2026 at 10:39:08AM -0700, Ian Rogers wrote: > On Wed, May 6, 2026 at 2:12 AM James Clark <james.clark@linaro.org> wrote: > > On 30/04/2026 5:17 pm, Ian Rogers wrote: > > > Only uncore PMUs can have an identifier, so add an optimized > > > perf_pmus__scan routine for that case to avoid all PMU types being > > > created. > > > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > > --- > > > tools/perf/util/metricgroup.c | 11 +++-------- > > > tools/perf/util/pmus.c | 18 +++++++++++++++++- > > > tools/perf/util/pmus.h | 1 + > > > 3 files changed, 21 insertions(+), 9 deletions(-) > > > > > > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c > > > index 4db9578efd81..5a489e97c413 100644 > > > --- a/tools/perf/util/metricgroup.c > > > +++ b/tools/perf/util/metricgroup.c > > > @@ -415,14 +415,9 @@ static int metricgroup__sys_event_iter(const struct pmu_metric *pm, > > > if (!pm->metric_expr || !pm->compat) > > > return 0; > > > > > > - while ((pmu = perf_pmus__scan(pmu))) { > > > - > > > - if (!pmu->id || !pmu_uncore_identifier_match(pm->compat, pmu->id)) > > > - continue; > > > - > > > - return d->fn(pm, table, d->data); > > > - } > > > - return 0; > > > + /* Only process with the iterator if there is a a PMU that matches the ID. */ > > > + pmu = perf_pmus__scan_for_uncore_id(pmu, pm->compat); > > > + return pmu ? d->fn(pm, table, d->data) : 0; > > > } > > > > > > int metricgroup__for_each_metric(const struct pmu_metrics_table *table, pmu_metric_iter_fn fn, > > > diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c > > > index 9a2023ceeefd..5e3f571450fe 100644 > > > --- a/tools/perf/util/pmus.c > > > +++ b/tools/perf/util/pmus.c > > > @@ -409,7 +409,7 @@ struct perf_pmu *perf_pmus__scan_matching_wildcard(struct perf_pmu *pmu, const c > > > if (!pmu) { > > > /* > > > * Core PMUs, other sysfs PMUs and tool PMU can have any name or > > > - * aren't wother optimizing for. > > > + * aren't worth optimizing for. > > > */ > > > unsigned int to_read_pmus = PERF_TOOL_PMU_TYPE_PE_CORE_MASK | > > > PERF_TOOL_PMU_TYPE_PE_OTHER_MASK | > > > @@ -486,6 +486,22 @@ static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu) > > > return NULL; > > > } > > > > > > +struct perf_pmu *perf_pmus__scan_for_uncore_id(struct perf_pmu *pmu, const char *compat) > > > +{ > > > + if (!pmu) { > > > + /* Only uncore PMUs can have identifiers. */ > > > + unsigned int to_read_pmus = PERF_TOOL_PMU_TYPE_PE_OTHER_MASK; > > > + > > > + pmu_read_sysfs(to_read_pmus); > > > + pmu = list_prepare_entry(pmu, &other_pmus, list); > > > + } > > > + list_for_each_entry_continue(pmu, &other_pmus, list) { > > > + if (pmu->id && pmu_uncore_identifier_match(compat, pmu->id)) > > > + return pmu; > > > + } > > > + return NULL; > > > +} > > > + > > > const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str) > > > { > > > struct perf_pmu *pmu = NULL; > > > diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h > > > index 7cb36863711a..0d55edb3f2fc 100644 > > > --- a/tools/perf/util/pmus.h > > > +++ b/tools/perf/util/pmus.h > > > @@ -23,6 +23,7 @@ struct perf_pmu *perf_pmus__scan(struct perf_pmu *pmu); > > > struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu); > > > struct perf_pmu *perf_pmus__scan_for_event(struct perf_pmu *pmu, const char *event); > > > struct perf_pmu *perf_pmus__scan_matching_wildcard(struct perf_pmu *pmu, const char *wildcard); > > > +struct perf_pmu *perf_pmus__scan_for_uncore_id(struct perf_pmu *pmu, const char *compat); > > > > > > const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str); > > > > > > > Reviewed-by: James Clark <james.clark@linaro.org> > > Ping. Thanks, applied to perf-tools-next, for v7.2. - Arnaldo ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-14 23:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <acyWR2GAWIY7TKyD@google.com>
[not found] ` <20260430161725.3131665-1-irogers@google.com>
2026-05-05 21:24 ` [PATCH v2] perf metricgroup: Avoid scanning unnecessary PMUs for identifier match Ian Rogers
2026-05-06 9:12 ` James Clark
2026-05-14 17:39 ` Ian Rogers
2026-05-14 23:58 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox