[PATCH] perf topdown: Don't require config1==0 for a slots event

[PATCH] perf topdown: Don't require config1==0 for a slots event

[Zide Chen]

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
       <not counted>      slots
     <not supported>      cpu/slots,metrics_clear=1/
       <not counted>      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: 5b546de9cc17 ("perf topdown: Use attribute to see an event is a topdown metic or slots")
Signed-off-by: Zide Chen <zide.chen@intel.com>
---
 tools/perf/arch/x86/tests/topdown.c | 28 ++++++++++++++++++++++++++++
 tools/perf/arch/x86/util/topdown.c  |  3 +--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
index 2b6f47ce4932..b1ce9f83db00 100644
--- a/tools/perf/arch/x86/tests/topdown.c
+++ b/tools/perf/arch/x86/tests/topdown.c
@@ -229,10 +229,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)
-- 
2.55.0

Re: [PATCH] perf topdown: Don't require config1==0 for a slots event

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf: missing explicit <stdio.h> and <string.h> inclusions
--

commit 197eea3115a0d065dc53e6f96ad2fd38fdb9bc99
Author: Zide Chen <zide.chen@intel.com>

perf topdown: Don't require config1==0 for a slots event

This commit modifies the topdown slots architecture check to no longer require
config1 == 0, fixing an issue where explicit slots events were duplicated in
scheduling. It also adds a new test to verify that an explicit metrics_clear
slots event doesn't trigger extra injections.

> diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
> index 2b6f47ce49324..b1ce9f83db00e 100644
> --- a/tools/perf/arch/x86/tests/topdown.c
> +++ b/tools/perf/arch/x86/tests/topdown.c
[ ... ]
> @@ -229,10 +229,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);

[Severity: Low]
Does this new function test__x86_topdown_metrics_clear() need explicit header
inclusions for musl compatibility?

This code relies on an implicit declaration of <stdio.h> for snprintf().
Missing POSIX headers can cause build failures when compiling the perf tool
with strict libc implementations like musl instead of glibc.

Should <stdio.h> (and potentially <string.h>) be explicitly included at the top
of this file to prevent musl compilation failures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916202740.257376-1-zide.chen@intel.com?part=1

Re: [PATCH] perf topdown: Don't require config1==0 for a slots event

[Chen, Zide]

On 9/16/2026 3:44 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] perf: missing explicit <stdio.h> and <string.h> inclusions
> --
> 
> commit 197eea3115a0d065dc53e6f96ad2fd38fdb9bc99
> Author: Zide Chen <zide.chen@intel.com>
> 
> perf topdown: Don't require config1==0 for a slots event
> 
> This commit modifies the topdown slots architecture check to no longer require
> config1 == 0, fixing an issue where explicit slots events were duplicated in
> scheduling. It also adds a new test to verify that an explicit metrics_clear
> slots event doesn't trigger extra injections.
> 
>> diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
>> index 2b6f47ce49324..b1ce9f83db00e 100644
>> --- a/tools/perf/arch/x86/tests/topdown.c
>> +++ b/tools/perf/arch/x86/tests/topdown.c
> [ ... ]
>> @@ -229,10 +229,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);
> 
> [Severity: Low]
> Does this new function test__x86_topdown_metrics_clear() need explicit header
> inclusions for musl compatibility?
> 
> This code relies on an implicit declaration of <stdio.h> for snprintf().
> Missing POSIX headers can cause build failures when compiling the perf tool
> with strict libc implementations like musl instead of glibc.
> 
> Should <stdio.h> (and potentially <string.h>) be explicitly included at the top
> of this file to prevent musl compilation failures?

Not needed. tools/perf/util/pmu.h, along with debug.h (which is also
included), explicitly pulls in <stdio.h>.

Additionally, this file already contains other calls to snprintf(). If
the required header dependency were missing, those existing uses would
already fail to build.

Re: [PATCH] perf topdown: Don't require config1==0 for a slots event

[Ian Rogers]

On Wed, Sep 16, 2026 at 1:37 PM Zide Chen <zide.chen@intel.com> wrote:
>
> 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
>        <not counted>      slots
>      <not supported>      cpu/slots,metrics_clear=1/
>        <not counted>      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: 5b546de9cc17 ("perf topdown: Use attribute to see an event is a topdown metic or slots")
> Signed-off-by: Zide Chen <zide.chen@intel.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Note, there's a Sashiko warning about a potentially missing #include
for musl compatibility that maybe you or Arnaldo could address.

Thanks,
Ian

> ---
>  tools/perf/arch/x86/tests/topdown.c | 28 ++++++++++++++++++++++++++++
>  tools/perf/arch/x86/util/topdown.c  |  3 +--
>  2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
> index 2b6f47ce4932..b1ce9f83db00 100644
> --- a/tools/perf/arch/x86/tests/topdown.c
> +++ b/tools/perf/arch/x86/tests/topdown.c
> @@ -229,10 +229,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)
> --
> 2.55.0
>

Re: [PATCH] perf topdown: Don't require config1==0 for a slots event

[Ian Rogers]

On Wed, Sep 16, 2026 at 2:52 PM Chen, Zide <zide.chen@intel.com> wrote:
>
> On 9/16/2026 3:44 PM, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Low] perf: missing explicit <stdio.h> and <string.h> inclusions
> > --
> >
> > commit 197eea3115a0d065dc53e6f96ad2fd38fdb9bc99
> > Author: Zide Chen <zide.chen@intel.com>
> >
> > perf topdown: Don't require config1==0 for a slots event
> >
> > This commit modifies the topdown slots architecture check to no longer require
> > config1 == 0, fixing an issue where explicit slots events were duplicated in
> > scheduling. It also adds a new test to verify that an explicit metrics_clear
> > slots event doesn't trigger extra injections.
> >
> >> diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
> >> index 2b6f47ce49324..b1ce9f83db00e 100644
> >> --- a/tools/perf/arch/x86/tests/topdown.c
> >> +++ b/tools/perf/arch/x86/tests/topdown.c
> > [ ... ]
> >> @@ -229,10 +229,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);
> >
> > [Severity: Low]
> > Does this new function test__x86_topdown_metrics_clear() need explicit header
> > inclusions for musl compatibility?
> >
> > This code relies on an implicit declaration of <stdio.h> for snprintf().
> > Missing POSIX headers can cause build failures when compiling the perf tool
> > with strict libc implementations like musl instead of glibc.
> >
> > Should <stdio.h> (and potentially <string.h>) be explicitly included at the top
> > of this file to prevent musl compilation failures?
>
> Not needed. tools/perf/util/pmu.h, along with debug.h (which is also
> included), explicitly pulls in <stdio.h>.
>
> Additionally, this file already contains other calls to snprintf(). If
> the required header dependency were missing, those existing uses would
> already fail to build.

In general we want to avoid transitive includes as they are brittle.
For example, if pmu.h and/or debug.h were changed, it might break
musl.

Thanks,
Ian

Re: [PATCH] perf topdown: Don't require config1==0 for a slots event

[Mi, Dapeng]

LGTM. Thanks.

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

On 9/17/2026 4:27 AM, Zide Chen wrote:
> 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
>        <not counted>      slots
>      <not supported>      cpu/slots,metrics_clear=1/
>        <not counted>      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: 5b546de9cc17 ("perf topdown: Use attribute to see an event is a topdown metic or slots")
> Signed-off-by: Zide Chen <zide.chen@intel.com>
> ---
>  tools/perf/arch/x86/tests/topdown.c | 28 ++++++++++++++++++++++++++++
>  tools/perf/arch/x86/util/topdown.c  |  3 +--
>  2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
> index 2b6f47ce4932..b1ce9f83db00 100644
> --- a/tools/perf/arch/x86/tests/topdown.c
> +++ b/tools/perf/arch/x86/tests/topdown.c
> @@ -229,10 +229,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)

Re: [PATCH] perf topdown: Don't require config1==0 for a slots event

[Chen, Zide]

On 9/16/2026 6:00 PM, Ian Rogers wrote:
> On Wed, Sep 16, 2026 at 2:52 PM Chen, Zide <zide.chen@intel.com> wrote:
>>
>> On 9/16/2026 3:44 PM, sashiko-bot@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>> - [Low] perf: missing explicit <stdio.h> and <string.h> inclusions
>>> --
>>>
>>> commit 197eea3115a0d065dc53e6f96ad2fd38fdb9bc99
>>> Author: Zide Chen <zide.chen@intel.com>
>>>
>>> perf topdown: Don't require config1==0 for a slots event
>>>
>>> This commit modifies the topdown slots architecture check to no longer require
>>> config1 == 0, fixing an issue where explicit slots events were duplicated in
>>> scheduling. It also adds a new test to verify that an explicit metrics_clear
>>> slots event doesn't trigger extra injections.
>>>
>>>> diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
>>>> index 2b6f47ce49324..b1ce9f83db00e 100644
>>>> --- a/tools/perf/arch/x86/tests/topdown.c
>>>> +++ b/tools/perf/arch/x86/tests/topdown.c
>>> [ ... ]
>>>> @@ -229,10 +229,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);
>>>
>>> [Severity: Low]
>>> Does this new function test__x86_topdown_metrics_clear() need explicit header
>>> inclusions for musl compatibility?
>>>
>>> This code relies on an implicit declaration of <stdio.h> for snprintf().
>>> Missing POSIX headers can cause build failures when compiling the perf tool
>>> with strict libc implementations like musl instead of glibc.
>>>
>>> Should <stdio.h> (and potentially <string.h>) be explicitly included at the top
>>> of this file to prevent musl compilation failures?
>>
>> Not needed. tools/perf/util/pmu.h, along with debug.h (which is also
>> included), explicitly pulls in <stdio.h>.
>>
>> Additionally, this file already contains other calls to snprintf(). If
>> the required header dependency were missing, those existing uses would
>> already fail to build.
> 
> In general we want to avoid transitive includes as they are brittle.
> For example, if pmu.h and/or debug.h were changed, it might break
> musl.

OK, then I can add stdio.h and string.h to topdown.c.

> Thanks,
> Ian

[PATCH] perf topdown: Don't require config1==0 for a slots event

[Zide Chen]

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
       <not counted>      slots
     <not supported>      cpu/slots,metrics_clear=1/
       <not counted>      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: 5b546de9cc17 ("perf topdown: Use attribute to see an event is a topdown metic or slots")
Signed-off-by: Zide Chen <zide.chen@intel.com>
Reviewed-by: Ian Rogers <irogers@google.com>
Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---
v2: include stdio.h and string.h for musl compatibility.
---
 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 <errno.h>
+#include <stdio.h>
+#include <string.h>
 #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)
-- 
2.55.0

Re: [PATCH] perf topdown: Don't require config1==0 for a slots event

[sashiko-bot]

> 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
>        <not counted>      slots
>      <not supported>      cpu/slots,metrics_clear=1/
>        <not counted>      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: 5b546de9cc17 ("perf topdown: Use attribute to see an event is a topdown metic or slots")
> Signed-off-by: Zide Chen <zide.chen@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917181016.381994-1-zide.chen@intel.com?part=1

Re: [PATCH] perf topdown: Don't require config1==0 for a slots event

[Arnaldo Carvalho de Melo]

On Wed, Sep 16, 2026 at 02:22:45PM -0700, Ian Rogers wrote:
> On Wed, Sep 16, 2026 at 1:37 PM Zide Chen <zide.chen@intel.com> wrote:
> >
> > 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
> >        <not counted>      slots
> >      <not supported>      cpu/slots,metrics_clear=1/
> >        <not counted>      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: 5b546de9cc17 ("perf topdown: Use attribute to see an event is a topdown metic or slots")
> > Signed-off-by: Zide Chen <zide.chen@intel.com>
> 
> Reviewed-by: Ian Rogers <irogers@google.com>
> 
> Note, there's a Sashiko warning about a potentially missing #include
> for musl compatibility that maybe you or Arnaldo could address.

Merging, will look add address that comment,

Thanks,

- Arnaldo
 
> Thanks,
> Ian
> 
> > ---
> >  tools/perf/arch/x86/tests/topdown.c | 28 ++++++++++++++++++++++++++++
> >  tools/perf/arch/x86/util/topdown.c  |  3 +--
> >  2 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
> > index 2b6f47ce4932..b1ce9f83db00 100644
> > --- a/tools/perf/arch/x86/tests/topdown.c
> > +++ b/tools/perf/arch/x86/tests/topdown.c
> > @@ -229,10 +229,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)
> > --
> > 2.55.0
> >

Re: [PATCH] perf topdown: Don't require config1==0 for a slots event

[Mi, Dapeng]

On 9/19/2026 4:13 AM, Arnaldo Carvalho de Melo wrote:
> On Wed, Sep 16, 2026 at 02:22:45PM -0700, Ian Rogers wrote:
>> On Wed, Sep 16, 2026 at 1:37 PM Zide Chen <zide.chen@intel.com> wrote:
>>> 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
>>>        <not counted>      slots
>>>      <not supported>      cpu/slots,metrics_clear=1/
>>>        <not counted>      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: 5b546de9cc17 ("perf topdown: Use attribute to see an event is a topdown metic or slots")
>>> Signed-off-by: Zide Chen <zide.chen@intel.com>
>> Reviewed-by: Ian Rogers <irogers@google.com>
>>
>> Note, there's a Sashiko warning about a potentially missing #include
>> for musl compatibility that maybe you or Arnaldo could address.
> Merging, will look add address that comment,

@Arnaldo, Zide posted the v2 patch to address the Sashiko comments.
https://lore.kernel.org/all/20260917181016.381994-1-zide.chen@intel.com/

You may directly merge that patch. :) 

Thanks.


>
> Thanks,
>
> - Arnaldo
>  
>> Thanks,
>> Ian
>>
>>> ---
>>>  tools/perf/arch/x86/tests/topdown.c | 28 ++++++++++++++++++++++++++++
>>>  tools/perf/arch/x86/util/topdown.c  |  3 +--
>>>  2 files changed, 29 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/tests/topdown.c
>>> index 2b6f47ce4932..b1ce9f83db00 100644
>>> --- a/tools/perf/arch/x86/tests/topdown.c
>>> +++ b/tools/perf/arch/x86/tests/topdown.c
>>> @@ -229,10 +229,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)
>>> --
>>> 2.55.0
>>>