From 0824af00a8d2ede0d8bc70bdc0b493ecfd167900 Mon Sep 17 00:00:00 2001 From: Zide Chen Date: Wed, 16 Sep 2026 13:27:40 -0700 Subject: [PATCH] perf topdown: Don't require config1==0 for a slots event arch_is_topdown_slots() requires config1 == 0, so it misidentified "cpu/slots,metrics_clear=1/" as not being the slots event, and an additional slots event gets inserted, which fails event scheduling since the group now needs two fixed counter 3. $ perf stat -e "{cpu/slots,metrics_clear=1/,cpu/topdown-retiring/}" -- sleep 1 WARNING: events were regrouped to match PMUs slots cpu/slots,metrics_clear=1/ cpu/topdown-retiring/ Drop the config1 == 0 check from arch_is_topdown_slots(). This is safe: config1 has no bearing on the event's identifying config value (TOPDOWN_SLOTS, 0x0400). The attr.type == PERF_TYPE_RAW check is what actually matters: only the core PMU is registered with PERF_TYPE_RAW, and on x86, 0x400 is guaranteed to be slots event on core CPU. arch_is_topdown_metrics() keeps its own config1 == 0 check, since genuine metrics events have no "metrics_clear" and config1 remains 0. Add a test verifying that an explicit "slots,metrics_clear=1" event in a group does not trigger an extra slots event injection. Fixes: 5b546de9cc177936 ("perf topdown: Use attribute to see an event is a topdown metic or slots") Reviewed-by: Dapeng Mi Reviewed-by: Ian Rogers Signed-off-by: Zide Chen Cc: Namhyung Kim [ Add missing stdio.h and string.h as noticed by Sashiko ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/arch/x86/tests/topdown.c | 30 +++++++++++++++++++++++++++++ tools/perf/arch/x86/util/topdown.c | 3 +-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c index 2b6f47ce4932..520075329c37 100644 --- a/tools/perf/arch/x86/tests/topdown.c +++ b/tools/perf/arch/x86/tests/topdown.c @@ -1,5 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include +#include +#include #include "arch-tests.h" #include "../util/topdown.h" #include "debug.h" @@ -229,10 +231,38 @@ static int test__x86_topdown_slots_injection(struct test_suite *test __maybe_unu return TEST_OK; } +/* + * An explicit "slots,metrics_clear=1" event is still the slots event and + * must not cause an extra slots event to be injected into the group. + */ +static int test__x86_topdown_metrics_clear(struct test_suite *test __maybe_unused, + int subtest __maybe_unused) +{ + struct perf_pmu *pmu; + char event_str[128]; + int ret; + + if (!topdown_sys_has_perf_metrics()) + return TEST_OK; + + pmu = perf_pmus__find_by_type(PERF_TYPE_RAW); + if (!pmu || !perf_pmu__has_format(pmu, "metrics_clear")) + return TEST_OK; + + snprintf(event_str, sizeof(event_str), + "{%s/slots,metrics_clear=1/,%s/topdown-retiring/}", + pmu->name, pmu->name); + ret = test_sort(event_str, 2, 1); + TEST_ASSERT_EQUAL("explicit metrics_clear slots event isn't duplicated", ret, TEST_OK); + + return TEST_OK; +} + static struct test_case x86_topdown_tests[] = { TEST_CASE("topdown events", x86_topdown), TEST_CASE("topdown sorting", x86_topdown_sorting), TEST_CASE("topdown slots injection", x86_topdown_slots_injection), + TEST_CASE("topdown metrics_clear no extra slots", x86_topdown_metrics_clear), { .name = NULL, } }; diff --git a/tools/perf/arch/x86/util/topdown.c b/tools/perf/arch/x86/util/topdown.c index bafd285119d7..64bc64e335a2 100644 --- a/tools/perf/arch/x86/util/topdown.c +++ b/tools/perf/arch/x86/util/topdown.c @@ -37,8 +37,7 @@ bool topdown_sys_has_perf_metrics(void) bool arch_is_topdown_slots(const struct evsel *evsel) { return evsel->core.attr.type == PERF_TYPE_RAW && - evsel->core.attr.config == TOPDOWN_SLOTS && - evsel->core.attr.config1 == 0; + evsel->core.attr.config == TOPDOWN_SLOTS; } bool arch_is_topdown_metrics(const struct evsel *evsel)