* [PATCH] perf/x86: Fix open counting event error
@ 2025-04-23 6:47 Luo Gengkun
2025-04-23 13:51 ` Liang, Kan
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Luo Gengkun @ 2025-04-23 6:47 UTC (permalink / raw)
To: peterz
Cc: mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa,
irogers, adrian.hunter, kan.liang, tglx, bp, dave.hansen, x86,
hpa, ravi.bangoria, linux-perf-users, linux-kernel
Perf doesn't work at perf stat for hardware events:
$perf stat -- sleep 1
Performance counter stats for 'sleep 1':
16.44 msec task-clock # 0.016 CPUs utilized
2 context-switches # 121.691 /sec
0 cpu-migrations # 0.000 /sec
54 page-faults # 3.286 K/sec
<not supported> cycles
<not supported> instructions
<not supported> branches
<not supported> branch-misses
The reason is that the check in x86_pmu_hw_config for sampling event is
unexpectedly applied to the counting event.
Fixes: 88ec7eedbbd2 ("perf/x86: Fix low freqency setting issue")
Signed-off-by: Luo Gengkun <luogengkun@huaweicloud.com>
---
arch/x86/events/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 6866cc5acb0b..3a4f031d2f44 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -629,7 +629,7 @@ int x86_pmu_hw_config(struct perf_event *event)
if (event->attr.type == event->pmu->type)
event->hw.config |= x86_pmu_get_event_config(event);
- if (!event->attr.freq && x86_pmu.limit_period) {
+ if (is_sampling_event(event) && !event->attr.freq && x86_pmu.limit_period) {
s64 left = event->attr.sample_period;
x86_pmu.limit_period(event, &left);
if (left > event->attr.sample_period)
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] perf/x86: Fix open counting event error
2025-04-23 6:47 [PATCH] perf/x86: Fix open counting event error Luo Gengkun
@ 2025-04-23 13:51 ` Liang, Kan
2025-04-24 6:52 ` Ravi Bangoria
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Liang, Kan @ 2025-04-23 13:51 UTC (permalink / raw)
To: Luo Gengkun, peterz
Cc: mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa,
irogers, adrian.hunter, tglx, bp, dave.hansen, x86, hpa,
ravi.bangoria, linux-perf-users, linux-kernel
On 2025-04-23 2:47 a.m., Luo Gengkun wrote:
> Perf doesn't work at perf stat for hardware events:
>
> $perf stat -- sleep 1
> Performance counter stats for 'sleep 1':
> 16.44 msec task-clock # 0.016 CPUs utilized
> 2 context-switches # 121.691 /sec
> 0 cpu-migrations # 0.000 /sec
> 54 page-faults # 3.286 K/sec
> <not supported> cycles
> <not supported> instructions
> <not supported> branches
> <not supported> branch-misses
>
> The reason is that the check in x86_pmu_hw_config for sampling event is
> unexpectedly applied to the counting event.
>
> Fixes: 88ec7eedbbd2 ("perf/x86: Fix low freqency setting issue")
> Signed-off-by: Luo Gengkun <luogengkun@huaweicloud.com>
Yes, it should only be applied for the sampling event. The
event->attr.freq is always 0 in the counting mode.
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Thanks,
Kan> ---
> arch/x86/events/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 6866cc5acb0b..3a4f031d2f44 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -629,7 +629,7 @@ int x86_pmu_hw_config(struct perf_event *event)
> if (event->attr.type == event->pmu->type)
> event->hw.config |= x86_pmu_get_event_config(event);
>
> - if (!event->attr.freq && x86_pmu.limit_period) {
> + if (is_sampling_event(event) && !event->attr.freq && x86_pmu.limit_period) {
> s64 left = event->attr.sample_period;
> x86_pmu.limit_period(event, &left);
> if (left > event->attr.sample_period)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf/x86: Fix open counting event error
2025-04-23 6:47 [PATCH] perf/x86: Fix open counting event error Luo Gengkun
2025-04-23 13:51 ` Liang, Kan
@ 2025-04-24 6:52 ` Ravi Bangoria
2025-04-24 17:15 ` Liang, Kan
2025-04-24 16:20 ` Ingo Molnar
2025-04-24 18:22 ` [tip: perf/urgent] perf/x86: Fix non-sampling (counting) events on certain x86 platforms tip-bot2 for Luo Gengkun
3 siblings, 1 reply; 10+ messages in thread
From: Ravi Bangoria @ 2025-04-24 6:52 UTC (permalink / raw)
To: Luo Gengkun, peterz@infradead.org
Cc: mingo@redhat.com, acme@kernel.org, namhyung@kernel.org,
mark.rutland@arm.com, alexander.shishkin@linux.intel.com,
jolsa@kernel.org, irogers@google.com, adrian.hunter@intel.com,
kan.liang@linux.intel.com, tglx@linutronix.de, bp@alien8.de,
dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
Bangoria, Ravikumar
On 23-Apr-25 12:17 PM, Luo Gengkun wrote:
> Perf doesn't work at perf stat for hardware events:
>
> $perf stat -- sleep 1
> Performance counter stats for 'sleep 1':
> 16.44 msec task-clock # 0.016 CPUs utilized
> 2 context-switches # 121.691 /sec
> 0 cpu-migrations # 0.000 /sec
> 54 page-faults # 3.286 K/sec
> <not supported> cycles
> <not supported> instructions
> <not supported> branches
> <not supported> branch-misses
Wondering if it is worth to add this in perf test. Something like
below?
--- a/tools/perf/tests/shell/stat.sh
+++ b/tools/perf/tests/shell/stat.sh
@@ -16,6 +16,24 @@ test_default_stat() {
echo "Basic stat command test [Success]"
}
+test_stat_count() {
+ echo "stat count test"
+
+ if ! perf list | grep -q "cpu-cycles OR cycles"
+ then
+ echo "stat count test [Skipped cpu-cycles event missing]"
+ return
+ fi
+
+ if perf stat -e cycles true 2>&1 | grep -E -q "<not supported>"
+ then
+ echo "stat count test [Failed]"
+ err=1
+ return
+ fi
+ echo "stat count test [Success]"
+}
+
test_stat_record_report() {
echo "stat record and report test"
if ! perf stat record -o - true | perf stat report -i - 2>&1 | \
@@ -201,6 +219,7 @@ test_hybrid() {
}
test_default_stat
+test_stat_count
test_stat_record_report
test_stat_record_script
test_stat_repeat_weak_groups
---
Thanks,
Ravi
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf/x86: Fix open counting event error
2025-04-23 6:47 [PATCH] perf/x86: Fix open counting event error Luo Gengkun
2025-04-23 13:51 ` Liang, Kan
2025-04-24 6:52 ` Ravi Bangoria
@ 2025-04-24 16:20 ` Ingo Molnar
2025-04-24 17:08 ` Liang, Kan
2025-04-24 18:22 ` [tip: perf/urgent] perf/x86: Fix non-sampling (counting) events on certain x86 platforms tip-bot2 for Luo Gengkun
3 siblings, 1 reply; 10+ messages in thread
From: Ingo Molnar @ 2025-04-24 16:20 UTC (permalink / raw)
To: Luo Gengkun
Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
jolsa, irogers, adrian.hunter, kan.liang, tglx, bp, dave.hansen,
x86, hpa, ravi.bangoria, linux-perf-users, linux-kernel
* Luo Gengkun <luogengkun@huaweicloud.com> wrote:
> Perf doesn't work at perf stat for hardware events:
>
> $perf stat -- sleep 1
> Performance counter stats for 'sleep 1':
> 16.44 msec task-clock # 0.016 CPUs utilized
> 2 context-switches # 121.691 /sec
> 0 cpu-migrations # 0.000 /sec
> 54 page-faults # 3.286 K/sec
> <not supported> cycles
> <not supported> instructions
> <not supported> branches
> <not supported> branch-misses
>
> The reason is that the check in x86_pmu_hw_config for sampling event is
> unexpectedly applied to the counting event.
>
> Fixes: 88ec7eedbbd2 ("perf/x86: Fix low freqency setting issue")
> Signed-off-by: Luo Gengkun <luogengkun@huaweicloud.com>
> ---
> arch/x86/events/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 6866cc5acb0b..3a4f031d2f44 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -629,7 +629,7 @@ int x86_pmu_hw_config(struct perf_event *event)
> if (event->attr.type == event->pmu->type)
> event->hw.config |= x86_pmu_get_event_config(event);
>
> - if (!event->attr.freq && x86_pmu.limit_period) {
> + if (is_sampling_event(event) && !event->attr.freq && x86_pmu.limit_period) {
Hm, so how come it works here, on an affected x86 system:
$ perf stat -- sleep 1
Performance counter stats for 'sleep 1':
0.64 msec task-clock:u # 0.001 CPUs utilized
0 context-switches:u # 0.000 /sec
0 cpu-migrations:u # 0.000 /sec
73 page-faults:u # 114.063 K/sec
325,849 instructions:u # 0.56 insn per cycle
# 0.88 stalled cycles per insn
580,323 cycles:u # 0.907 GHz
286,348 stalled-cycles-frontend:u # 49.34% frontend cycles idle
72,623 branches:u # 113.474 M/sec
4,713 branch-misses:u # 6.49% of all branches
?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf/x86: Fix open counting event error
2025-04-24 16:20 ` Ingo Molnar
@ 2025-04-24 17:08 ` Liang, Kan
2025-04-24 18:14 ` Ingo Molnar
0 siblings, 1 reply; 10+ messages in thread
From: Liang, Kan @ 2025-04-24 17:08 UTC (permalink / raw)
To: Ingo Molnar, Luo Gengkun
Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
jolsa, irogers, adrian.hunter, tglx, bp, dave.hansen, x86, hpa,
ravi.bangoria, linux-perf-users, linux-kernel
On 2025-04-24 12:20 p.m., Ingo Molnar wrote:
>
> * Luo Gengkun <luogengkun@huaweicloud.com> wrote:
>
>> Perf doesn't work at perf stat for hardware events:
>>
>> $perf stat -- sleep 1
>> Performance counter stats for 'sleep 1':
>> 16.44 msec task-clock # 0.016 CPUs utilized
>> 2 context-switches # 121.691 /sec
>> 0 cpu-migrations # 0.000 /sec
>> 54 page-faults # 3.286 K/sec
>> <not supported> cycles
>> <not supported> instructions
>> <not supported> branches
>> <not supported> branch-misses
>>
>> The reason is that the check in x86_pmu_hw_config for sampling event is
>> unexpectedly applied to the counting event.
>>
>> Fixes: 88ec7eedbbd2 ("perf/x86: Fix low freqency setting issue")
>> Signed-off-by: Luo Gengkun <luogengkun@huaweicloud.com>
>> ---
>> arch/x86/events/core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
>> index 6866cc5acb0b..3a4f031d2f44 100644
>> --- a/arch/x86/events/core.c
>> +++ b/arch/x86/events/core.c
>> @@ -629,7 +629,7 @@ int x86_pmu_hw_config(struct perf_event *event)
>> if (event->attr.type == event->pmu->type)
>> event->hw.config |= x86_pmu_get_event_config(event);
>>
>> - if (!event->attr.freq && x86_pmu.limit_period) {
>> + if (is_sampling_event(event) && !event->attr.freq && x86_pmu.limit_period) {
>
> Hm, so how come it works here, on an affected x86 system:
>
> $ perf stat -- sleep 1
>
> Performance counter stats for 'sleep 1':
>
> 0.64 msec task-clock:u # 0.001 CPUs utilized
> 0 context-switches:u # 0.000 /sec
> 0 cpu-migrations:u # 0.000 /sec
> 73 page-faults:u # 114.063 K/sec
> 325,849 instructions:u # 0.56 insn per cycle
> # 0.88 stalled cycles per insn
> 580,323 cycles:u # 0.907 GHz
> 286,348 stalled-cycles-frontend:u # 49.34% frontend cycles idle
> 72,623 branches:u # 113.474 M/sec
> 4,713 branch-misses:u # 6.49% of all branches
>
>
> ?
It doesn't affect all X86 platforms. It should only impact the platforms
with limit_period used for the non-pebs events. For Intel platforms, it
should only impact some older platforms, e.g., HSW, BDW and NHM.
For other platforms, the x86_pmu.limit_period is invoked. But the left
is not updated. So it still equals to event->attr.sample_period.
It doesn't error out.
if (!event->attr.freq && x86_pmu.limit_period) {
s64 left = event->attr.sample_period;
x86_pmu.limit_period(event, &left);
if (left > event->attr.sample_period)
return -EINVAL;
}
Thanks,
Kan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf/x86: Fix open counting event error
2025-04-24 6:52 ` Ravi Bangoria
@ 2025-04-24 17:15 ` Liang, Kan
2025-04-25 10:12 ` Ravi Bangoria
0 siblings, 1 reply; 10+ messages in thread
From: Liang, Kan @ 2025-04-24 17:15 UTC (permalink / raw)
To: Ravi Bangoria, Luo Gengkun, peterz@infradead.org
Cc: mingo@redhat.com, acme@kernel.org, namhyung@kernel.org,
mark.rutland@arm.com, alexander.shishkin@linux.intel.com,
jolsa@kernel.org, irogers@google.com, adrian.hunter@intel.com,
tglx@linutronix.de, bp@alien8.de, dave.hansen@linux.intel.com,
x86@kernel.org, hpa@zytor.com, linux-perf-users@vger.kernel.org,
linux-kernel@vger.kernel.org
On 2025-04-24 2:52 a.m., Ravi Bangoria wrote:
> On 23-Apr-25 12:17 PM, Luo Gengkun wrote:
>> Perf doesn't work at perf stat for hardware events:
>>
>> $perf stat -- sleep 1
>> Performance counter stats for 'sleep 1':
>> 16.44 msec task-clock # 0.016 CPUs utilized
>> 2 context-switches # 121.691 /sec
>> 0 cpu-migrations # 0.000 /sec
>> 54 page-faults # 3.286 K/sec
>> <not supported> cycles
>> <not supported> instructions
>> <not supported> branches
>> <not supported> branch-misses
>
> Wondering if it is worth to add this in perf test. Something like
> below?
>
> --- a/tools/perf/tests/shell/stat.sh
> +++ b/tools/perf/tests/shell/stat.sh
> @@ -16,6 +16,24 @@ test_default_stat() {
> echo "Basic stat command test [Success]"
> }
>
> +test_stat_count() {
> + echo "stat count test"
> +
> + if ! perf list | grep -q "cpu-cycles OR cycles"
> + then
> + echo "stat count test [Skipped cpu-cycles event missing]"
> + return
> + fi
> +
> + if perf stat -e cycles true 2>&1 | grep -E -q "<not supported>"
> + then
> + echo "stat count test [Failed]"
> + err=1
> + return
> + fi
> + echo "stat count test [Success]"
> +}
> +
> test_stat_record_report() {
> echo "stat record and report test"
> if ! perf stat record -o - true | perf stat report -i - 2>&1 | \
> @@ -201,6 +219,7 @@ test_hybrid() {
> }
>
> test_default_stat
> +test_stat_count
I think the perf stat default should always be supported, not just cycles.
Maybe we should add the check in test_default_stat?
Thanks,
Kan> test_stat_record_report
> test_stat_record_script
> test_stat_repeat_weak_groups
> ---
>
> Thanks,
> Ravi
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf/x86: Fix open counting event error
2025-04-24 17:08 ` Liang, Kan
@ 2025-04-24 18:14 ` Ingo Molnar
0 siblings, 0 replies; 10+ messages in thread
From: Ingo Molnar @ 2025-04-24 18:14 UTC (permalink / raw)
To: Liang, Kan
Cc: Luo Gengkun, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, tglx, bp,
dave.hansen, x86, hpa, ravi.bangoria, linux-perf-users,
linux-kernel
* Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
> On 2025-04-24 12:20 p.m., Ingo Molnar wrote:
> >
> > * Luo Gengkun <luogengkun@huaweicloud.com> wrote:
> >
> >> Perf doesn't work at perf stat for hardware events:
> >>
> >> $perf stat -- sleep 1
> >> Performance counter stats for 'sleep 1':
> >> 16.44 msec task-clock # 0.016 CPUs utilized
> >> 2 context-switches # 121.691 /sec
> >> 0 cpu-migrations # 0.000 /sec
> >> 54 page-faults # 3.286 K/sec
> >> <not supported> cycles
> >> <not supported> instructions
> >> <not supported> branches
> >> <not supported> branch-misses
> >>
> >> The reason is that the check in x86_pmu_hw_config for sampling event is
> >> unexpectedly applied to the counting event.
> >>
> >> Fixes: 88ec7eedbbd2 ("perf/x86: Fix low freqency setting issue")
> >> Signed-off-by: Luo Gengkun <luogengkun@huaweicloud.com>
> >> ---
> >> arch/x86/events/core.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> >> index 6866cc5acb0b..3a4f031d2f44 100644
> >> --- a/arch/x86/events/core.c
> >> +++ b/arch/x86/events/core.c
> >> @@ -629,7 +629,7 @@ int x86_pmu_hw_config(struct perf_event *event)
> >> if (event->attr.type == event->pmu->type)
> >> event->hw.config |= x86_pmu_get_event_config(event);
> >>
> >> - if (!event->attr.freq && x86_pmu.limit_period) {
> >> + if (is_sampling_event(event) && !event->attr.freq && x86_pmu.limit_period) {
> >
> > Hm, so how come it works here, on an affected x86 system:
> >
> > $ perf stat -- sleep 1
> >
> > Performance counter stats for 'sleep 1':
> >
> > 0.64 msec task-clock:u # 0.001 CPUs utilized
> > 0 context-switches:u # 0.000 /sec
> > 0 cpu-migrations:u # 0.000 /sec
> > 73 page-faults:u # 114.063 K/sec
> > 325,849 instructions:u # 0.56 insn per cycle
> > # 0.88 stalled cycles per insn
> > 580,323 cycles:u # 0.907 GHz
> > 286,348 stalled-cycles-frontend:u # 49.34% frontend cycles idle
> > 72,623 branches:u # 113.474 M/sec
> > 4,713 branch-misses:u # 6.49% of all branches
> >
> >
> > ?
>
> It doesn't affect all X86 platforms. It should only impact the platforms
> with limit_period used for the non-pebs events. For Intel platforms, it
> should only impact some older platforms, e.g., HSW, BDW and NHM.
>
> For other platforms, the x86_pmu.limit_period is invoked. But the left
> is not updated. So it still equals to event->attr.sample_period.
> It doesn't error out.
>
> if (!event->attr.freq && x86_pmu.limit_period) {
> s64 left = event->attr.sample_period;
> x86_pmu.limit_period(event, &left);
> if (left > event->attr.sample_period)
> return -EINVAL;
> }
Makes sense. I've added this paragraph to the changelog:
It should only impact x86 platforms with limit_period used for non-PEBS
events. For Intel platforms, it should only impact some older platforms,
e.g., HSW, BDW and NHM.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tip: perf/urgent] perf/x86: Fix non-sampling (counting) events on certain x86 platforms
2025-04-23 6:47 [PATCH] perf/x86: Fix open counting event error Luo Gengkun
` (2 preceding siblings ...)
2025-04-24 16:20 ` Ingo Molnar
@ 2025-04-24 18:22 ` tip-bot2 for Luo Gengkun
3 siblings, 0 replies; 10+ messages in thread
From: tip-bot2 for Luo Gengkun @ 2025-04-24 18:22 UTC (permalink / raw)
To: linux-tip-commits
Cc: Luo Gengkun, Ingo Molnar, Kan Liang, Alexander Shishkin,
Arnaldo Carvalho de Melo, Jiri Olsa, Mark Rutland, Namhyung Kim,
Peter Zijlstra, Ravi Bangoria, x86, linux-kernel
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: 1a97fea9db9e9b9c4839d4232dde9f505ff5b4cc
Gitweb: https://git.kernel.org/tip/1a97fea9db9e9b9c4839d4232dde9f505ff5b4cc
Author: Luo Gengkun <luogengkun@huaweicloud.com>
AuthorDate: Wed, 23 Apr 2025 06:47:24
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Thu, 24 Apr 2025 20:15:04 +02:00
perf/x86: Fix non-sampling (counting) events on certain x86 platforms
Perf doesn't work at perf stat for hardware events on certain x86 platforms:
$perf stat -- sleep 1
Performance counter stats for 'sleep 1':
16.44 msec task-clock # 0.016 CPUs utilized
2 context-switches # 121.691 /sec
0 cpu-migrations # 0.000 /sec
54 page-faults # 3.286 K/sec
<not supported> cycles
<not supported> instructions
<not supported> branches
<not supported> branch-misses
The reason is that the check in x86_pmu_hw_config() for sampling events is
unexpectedly applied to counting events as well.
It should only impact x86 platforms with limit_period used for non-PEBS
events. For Intel platforms, it should only impact some older platforms,
e.g., HSW, BDW and NHM.
Fixes: 88ec7eedbbd2 ("perf/x86: Fix low freqency setting issue")
Signed-off-by: Luo Gengkun <luogengkun@huaweicloud.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@amd.com>
Link: https://lore.kernel.org/r/20250423064724.3716211-1-luogengkun@huaweicloud.com
---
arch/x86/events/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 6866cc5..3a4f031 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -629,7 +629,7 @@ int x86_pmu_hw_config(struct perf_event *event)
if (event->attr.type == event->pmu->type)
event->hw.config |= x86_pmu_get_event_config(event);
- if (!event->attr.freq && x86_pmu.limit_period) {
+ if (is_sampling_event(event) && !event->attr.freq && x86_pmu.limit_period) {
s64 left = event->attr.sample_period;
x86_pmu.limit_period(event, &left);
if (left > event->attr.sample_period)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] perf/x86: Fix open counting event error
2025-04-24 17:15 ` Liang, Kan
@ 2025-04-25 10:12 ` Ravi Bangoria
2025-04-30 14:12 ` Liang, Kan
0 siblings, 1 reply; 10+ messages in thread
From: Ravi Bangoria @ 2025-04-25 10:12 UTC (permalink / raw)
To: Liang, Kan
Cc: Luo Gengkun, peterz@infradead.org, mingo@redhat.com,
acme@kernel.org, namhyung@kernel.org, mark.rutland@arm.com,
alexander.shishkin@linux.intel.com, jolsa@kernel.org,
irogers@google.com, adrian.hunter@intel.com, tglx@linutronix.de,
bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
hpa@zytor.com, linux-perf-users@vger.kernel.org,
linux-kernel@vger.kernel.org, Ravi Bangoria
Hi Kan,
>>> Perf doesn't work at perf stat for hardware events:
>>>
>>> $perf stat -- sleep 1
>>> Performance counter stats for 'sleep 1':
>>> 16.44 msec task-clock # 0.016 CPUs utilized
>>> 2 context-switches # 121.691 /sec
>>> 0 cpu-migrations # 0.000 /sec
>>> 54 page-faults # 3.286 K/sec
>>> <not supported> cycles
>>> <not supported> instructions
>>> <not supported> branches
>>> <not supported> branch-misses
>>
>> Wondering if it is worth to add this in perf test. Something like
>> below?
>>
>> --- a/tools/perf/tests/shell/stat.sh
>> +++ b/tools/perf/tests/shell/stat.sh
>> @@ -16,6 +16,24 @@ test_default_stat() {
>> echo "Basic stat command test [Success]"
>> }
>>
>> +test_stat_count() {
>> + echo "stat count test"
>> +
>> + if ! perf list | grep -q "cpu-cycles OR cycles"
>> + then
>> + echo "stat count test [Skipped cpu-cycles event missing]"
>> + return
>> + fi
>> +
>> + if perf stat -e cycles true 2>&1 | grep -E -q "<not supported>"
>> + then
>> + echo "stat count test [Failed]"
>> + err=1
>> + return
>> + fi
>> + echo "stat count test [Success]"
>> +}
>> +
>> test_stat_record_report() {
>> echo "stat record and report test"
>> if ! perf stat record -o - true | perf stat report -i - 2>&1 | \
>> @@ -201,6 +219,7 @@ test_hybrid() {
>> }
>>
>> test_default_stat
>> +test_stat_count
>
> I think the perf stat default should always be supported, not just cycles.
> Maybe we should add the check in test_default_stat?
Do you mean:
if perf stat true 2>&1 | grep -E -q "<not supported>"
err=1
Isn't this ambiguous? Also, this fails on machines where HW pmu
is not supported. For ex, on my qemu guest with `-cpu pmu=off`:
$ ./perf list | grep "cpu-cycles OR cycles"
<empty output>
$ ./perf stat true
Performance counter stats for 'true':
0.42 msec task-clock:u # 0.470 CPUs utilized
0 context-switches:u # 0.000 /sec
0 cpu-migrations:u # 0.000 /sec
48 page-faults:u # 113.874 K/sec
<not supported> cycles:u
Thanks,
Ravi
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf/x86: Fix open counting event error
2025-04-25 10:12 ` Ravi Bangoria
@ 2025-04-30 14:12 ` Liang, Kan
0 siblings, 0 replies; 10+ messages in thread
From: Liang, Kan @ 2025-04-30 14:12 UTC (permalink / raw)
To: Ravi Bangoria
Cc: Luo Gengkun, peterz@infradead.org, mingo@redhat.com,
acme@kernel.org, namhyung@kernel.org, mark.rutland@arm.com,
alexander.shishkin@linux.intel.com, jolsa@kernel.org,
irogers@google.com, adrian.hunter@intel.com, tglx@linutronix.de,
bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
hpa@zytor.com, linux-perf-users@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi Ravi,
Sorry for the late response. I was on vacation.
On 2025-04-25 6:12 a.m., Ravi Bangoria wrote:
> Hi Kan,
>
>>>> Perf doesn't work at perf stat for hardware events:
>>>>
>>>> $perf stat -- sleep 1
>>>> Performance counter stats for 'sleep 1':
>>>> 16.44 msec task-clock # 0.016 CPUs utilized
>>>> 2 context-switches # 121.691 /sec
>>>> 0 cpu-migrations # 0.000 /sec
>>>> 54 page-faults # 3.286 K/sec
>>>> <not supported> cycles
>>>> <not supported> instructions
>>>> <not supported> branches
>>>> <not supported> branch-misses
>>>
>>> Wondering if it is worth to add this in perf test. Something like
>>> below?
>>>
>>> --- a/tools/perf/tests/shell/stat.sh
>>> +++ b/tools/perf/tests/shell/stat.sh
>>> @@ -16,6 +16,24 @@ test_default_stat() {
>>> echo "Basic stat command test [Success]"
>>> }
>>>
>>> +test_stat_count() {
>>> + echo "stat count test"
>>> +
>>> + if ! perf list | grep -q "cpu-cycles OR cycles"
>>> + then
>>> + echo "stat count test [Skipped cpu-cycles event missing]"
>>> + return
>>> + fi
>>> +
>>> + if perf stat -e cycles true 2>&1 | grep -E -q "<not supported>"
>>> + then
>>> + echo "stat count test [Failed]"
>>> + err=1
>>> + return
>>> + fi
>>> + echo "stat count test [Success]"
>>> +}
>>> +
>>> test_stat_record_report() {
>>> echo "stat record and report test"
>>> if ! perf stat record -o - true | perf stat report -i - 2>&1 | \
>>> @@ -201,6 +219,7 @@ test_hybrid() {
>>> }
>>>
>>> test_default_stat
>>> +test_stat_count
>>
>> I think the perf stat default should always be supported, not just cycles.
>> Maybe we should add the check in test_default_stat?
>
> Do you mean:
>
> if perf stat true 2>&1 | grep -E -q "<not supported>"
> err=1
>
Yes, I assumed that all the events in the perf stat are always
available. But it seems the assumption is only true for bare metal.
> Isn't this ambiguous? Also, this fails on machines where HW pmu
> is not supported. For ex, on my qemu guest with `-cpu pmu=off`:
>
> $ ./perf list | grep "cpu-cycles OR cycles"
> <empty output>
>
> $ ./perf stat true
> Performance counter stats for 'true':
> 0.42 msec task-clock:u # 0.470 CPUs utilized
> 0 context-switches:u # 0.000 /sec
> 0 cpu-migrations:u # 0.000 /sec
> 48 page-faults:u # 113.874 K/sec
> <not supported> cycles:u
>
If taking the virtualization into account, the test_stat_count looks
good to me.
Thanks,
Kan
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-04-30 14:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-23 6:47 [PATCH] perf/x86: Fix open counting event error Luo Gengkun
2025-04-23 13:51 ` Liang, Kan
2025-04-24 6:52 ` Ravi Bangoria
2025-04-24 17:15 ` Liang, Kan
2025-04-25 10:12 ` Ravi Bangoria
2025-04-30 14:12 ` Liang, Kan
2025-04-24 16:20 ` Ingo Molnar
2025-04-24 17:08 ` Liang, Kan
2025-04-24 18:14 ` Ingo Molnar
2025-04-24 18:22 ` [tip: perf/urgent] perf/x86: Fix non-sampling (counting) events on certain x86 platforms tip-bot2 for Luo Gengkun
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.