All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf: Fix 0 count issue of cpu-clock
@ 2025-11-12  8:05 Dapeng Mi
  2025-11-12 16:42 ` Ian Rogers
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Dapeng Mi @ 2025-11-12  8:05 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin,
	Andi Kleen, Eranian Stephane
  Cc: linux-kernel, linux-perf-users, Dapeng Mi, Zide Chen,
	Falcon Thomas, Xudong Hao, Dapeng Mi

Currently cpu-clock event always returns 0 count, e.g.,

perf stat -e cpu-clock -- sleep 1

 Performance counter stats for 'sleep 1':
                 0      cpu-clock                        #    0.000 CPUs utilized
       1.002308394 seconds time elapsed

The root cause is the commit 'bc4394e5e79c ("perf: Fix the throttle
 error of some clock events")' adds PERF_EF_UPDATE flag check before
calling cpu_clock_event_update() to update the count, however the
PERF_EF_UPDATE flag is never set when the cpu-clock event is stopped in
counting mode (pmu->dev() -> cpu_clock_event_del() ->
cpu_clock_event_stop()). This leads to the cpu-clock event count is
never updated.

To fix this issue, force to set PERF_EF_UPDATE flag for cpu-clock event
just like what task-clock does. Besides, or flags with PERF_EF_UPDATE
for task-clock although currently the flags argument would always be 0.

Fixes: bc4394e5e79c ("perf: Fix the throttle error of some clock events")
Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---

With this change, both cpu-clock and task-clock can do counting and
samping correctly.

1. perf stat -e cpu-clock,task-clock -- true

 Performance counter stats for 'true':
           240,636      cpu-clock                        #    0.358 CPUs utilized
           243,319      task-clock                       #    0.362 CPUs utilized

2. perf record -e cpu-clock -c 10000 -Iax,bx -- sleep 1
[ perf record: Woken up 2 times to write data ]
[ perf record: Captured and wrote 0.028 MB perf.data (36 samples) ]

3. perf record -e task-clock -c 10000 -Iax,bx -- sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.029 MB perf.data (41 samples) ]

 kernel/events/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index f6a08c73f783..77d3af5959c1 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11964,7 +11964,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags)
 
 static void cpu_clock_event_del(struct perf_event *event, int flags)
 {
-	cpu_clock_event_stop(event, flags);
+	cpu_clock_event_stop(event, flags | PERF_EF_UPDATE);
 }
 
 static void cpu_clock_event_read(struct perf_event *event)
@@ -12043,7 +12043,7 @@ static int task_clock_event_add(struct perf_event *event, int flags)
 
 static void task_clock_event_del(struct perf_event *event, int flags)
 {
-	task_clock_event_stop(event, PERF_EF_UPDATE);
+	task_clock_event_stop(event, flags | PERF_EF_UPDATE);
 }
 
 static void task_clock_event_read(struct perf_event *event)

base-commit: 2093d8cf80fa5552d1025a78a8f3a10bf3b6466e
prerequisite-patch-id: a15bcd62a8dcd219d17489eef88b66ea5488a2a0
prerequisite-patch-id: 2a0eefce67b21d1f30c272fd8115b0dc1aca3897
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf: Fix 0 count issue of cpu-clock
  2025-11-12  8:05 [PATCH] perf: Fix 0 count issue of cpu-clock Dapeng Mi
@ 2025-11-12 16:42 ` Ian Rogers
  2025-11-17 17:04   ` Ian Rogers
  2025-11-18  1:43 ` Namhyung Kim
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Ian Rogers @ 2025-11-12 16:42 UTC (permalink / raw)
  To: Dapeng Mi
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Adrian Hunter, Alexander Shishkin, Andi Kleen,
	Eranian Stephane, linux-kernel, linux-perf-users, Dapeng Mi,
	Zide Chen, Falcon Thomas, Xudong Hao

On Wed, Nov 12, 2025 at 12:07 AM Dapeng Mi <dapeng1.mi@linux.intel.com> wrote:
>
> Currently cpu-clock event always returns 0 count, e.g.,
>
> perf stat -e cpu-clock -- sleep 1
>
>  Performance counter stats for 'sleep 1':
>                  0      cpu-clock                        #    0.000 CPUs utilized
>        1.002308394 seconds time elapsed
>
> The root cause is the commit 'bc4394e5e79c ("perf: Fix the throttle
>  error of some clock events")' adds PERF_EF_UPDATE flag check before
> calling cpu_clock_event_update() to update the count, however the
> PERF_EF_UPDATE flag is never set when the cpu-clock event is stopped in
> counting mode (pmu->dev() -> cpu_clock_event_del() ->
> cpu_clock_event_stop()). This leads to the cpu-clock event count is
> never updated.
>
> To fix this issue, force to set PERF_EF_UPDATE flag for cpu-clock event
> just like what task-clock does. Besides, or flags with PERF_EF_UPDATE
> for task-clock although currently the flags argument would always be 0.
>
> Fixes: bc4394e5e79c ("perf: Fix the throttle error of some clock events")
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Thanks Dapeng! This is a fairly major regression and I'm surprised my
kernel picked it up so quickly. For those interested the relevant part
of the breaking change is requiring PERF_EF_UPDATE:

```
@@ -11829,7 +11834,8 @@ static void cpu_clock_event_start(struct
perf_event *event, int flags)
 static void cpu_clock_event_stop(struct perf_event *event, int flags)
 {
        perf_swevent_cancel_hrtimer(event);
-       cpu_clock_event_update(event);
+       if (flags & PERF_EF_UPDATE)
+               cpu_clock_event_update(event);
 }
 ```

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

Hopefully a maintainer can pick this up quickly. Thanks,
Ian

> ---
>
> With this change, both cpu-clock and task-clock can do counting and
> samping correctly.
>
> 1. perf stat -e cpu-clock,task-clock -- true
>
>  Performance counter stats for 'true':
>            240,636      cpu-clock                        #    0.358 CPUs utilized
>            243,319      task-clock                       #    0.362 CPUs utilized
>
> 2. perf record -e cpu-clock -c 10000 -Iax,bx -- sleep 1
> [ perf record: Woken up 2 times to write data ]
> [ perf record: Captured and wrote 0.028 MB perf.data (36 samples) ]
>
> 3. perf record -e task-clock -c 10000 -Iax,bx -- sleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.029 MB perf.data (41 samples) ]
>
>  kernel/events/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index f6a08c73f783..77d3af5959c1 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -11964,7 +11964,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags)
>
>  static void cpu_clock_event_del(struct perf_event *event, int flags)
>  {
> -       cpu_clock_event_stop(event, flags);
> +       cpu_clock_event_stop(event, flags | PERF_EF_UPDATE);
>  }
>
>  static void cpu_clock_event_read(struct perf_event *event)
> @@ -12043,7 +12043,7 @@ static int task_clock_event_add(struct perf_event *event, int flags)
>
>  static void task_clock_event_del(struct perf_event *event, int flags)
>  {
> -       task_clock_event_stop(event, PERF_EF_UPDATE);
> +       task_clock_event_stop(event, flags | PERF_EF_UPDATE);
>  }
>
>  static void task_clock_event_read(struct perf_event *event)
>
> base-commit: 2093d8cf80fa5552d1025a78a8f3a10bf3b6466e
> prerequisite-patch-id: a15bcd62a8dcd219d17489eef88b66ea5488a2a0
> prerequisite-patch-id: 2a0eefce67b21d1f30c272fd8115b0dc1aca3897
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf: Fix 0 count issue of cpu-clock
  2025-11-12 16:42 ` Ian Rogers
@ 2025-11-17 17:04   ` Ian Rogers
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2025-11-17 17:04 UTC (permalink / raw)
  To: Dapeng Mi, Ingo Molnar, Peter Zijlstra
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Adrian Hunter,
	Alexander Shishkin, Andi Kleen, Eranian Stephane, linux-kernel,
	linux-perf-users, Dapeng Mi, Zide Chen, Falcon Thomas, Xudong Hao

On Wed, Nov 12, 2025 at 8:42 AM Ian Rogers <irogers@google.com> wrote:
>
> On Wed, Nov 12, 2025 at 12:07 AM Dapeng Mi <dapeng1.mi@linux.intel.com> wrote:
> >
> > Currently cpu-clock event always returns 0 count, e.g.,
> >
> > perf stat -e cpu-clock -- sleep 1
> >
> >  Performance counter stats for 'sleep 1':
> >                  0      cpu-clock                        #    0.000 CPUs utilized
> >        1.002308394 seconds time elapsed
> >
> > The root cause is the commit 'bc4394e5e79c ("perf: Fix the throttle
> >  error of some clock events")' adds PERF_EF_UPDATE flag check before
> > calling cpu_clock_event_update() to update the count, however the
> > PERF_EF_UPDATE flag is never set when the cpu-clock event is stopped in
> > counting mode (pmu->dev() -> cpu_clock_event_del() ->
> > cpu_clock_event_stop()). This leads to the cpu-clock event count is
> > never updated.
> >
> > To fix this issue, force to set PERF_EF_UPDATE flag for cpu-clock event
> > just like what task-clock does. Besides, or flags with PERF_EF_UPDATE
> > for task-clock although currently the flags argument would always be 0.
> >
> > Fixes: bc4394e5e79c ("perf: Fix the throttle error of some clock events")
> > Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> Thanks Dapeng! This is a fairly major regression and I'm surprised my
> kernel picked it up so quickly. For those interested the relevant part
> of the breaking change is requiring PERF_EF_UPDATE:
>
> ```
> @@ -11829,7 +11834,8 @@ static void cpu_clock_event_start(struct
> perf_event *event, int flags)
>  static void cpu_clock_event_stop(struct perf_event *event, int flags)
>  {
>         perf_swevent_cancel_hrtimer(event);
> -       cpu_clock_event_update(event);
> +       if (flags & PERF_EF_UPDATE)
> +               cpu_clock_event_update(event);
>  }
>  ```
>
> Reviewed-by: Ian Rogers <irogers@google.com>
>
> Hopefully a maintainer can pick this up quickly. Thanks,
> Ian

Hi Peter and Ingo,

any update on picking this up? Having timer based software events
(task-clock, cpu-clock) broken is fairly major regression especially
in environments like hypervisors where PMU access is blocked.

Thanks,
Ian

> > ---
> >
> > With this change, both cpu-clock and task-clock can do counting and
> > samping correctly.
> >
> > 1. perf stat -e cpu-clock,task-clock -- true
> >
> >  Performance counter stats for 'true':
> >            240,636      cpu-clock                        #    0.358 CPUs utilized
> >            243,319      task-clock                       #    0.362 CPUs utilized
> >
> > 2. perf record -e cpu-clock -c 10000 -Iax,bx -- sleep 1
> > [ perf record: Woken up 2 times to write data ]
> > [ perf record: Captured and wrote 0.028 MB perf.data (36 samples) ]
> >
> > 3. perf record -e task-clock -c 10000 -Iax,bx -- sleep 1
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.029 MB perf.data (41 samples) ]
> >
> >  kernel/events/core.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index f6a08c73f783..77d3af5959c1 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -11964,7 +11964,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags)
> >
> >  static void cpu_clock_event_del(struct perf_event *event, int flags)
> >  {
> > -       cpu_clock_event_stop(event, flags);
> > +       cpu_clock_event_stop(event, flags | PERF_EF_UPDATE);
> >  }
> >
> >  static void cpu_clock_event_read(struct perf_event *event)
> > @@ -12043,7 +12043,7 @@ static int task_clock_event_add(struct perf_event *event, int flags)
> >
> >  static void task_clock_event_del(struct perf_event *event, int flags)
> >  {
> > -       task_clock_event_stop(event, PERF_EF_UPDATE);
> > +       task_clock_event_stop(event, flags | PERF_EF_UPDATE);
> >  }
> >
> >  static void task_clock_event_read(struct perf_event *event)
> >
> > base-commit: 2093d8cf80fa5552d1025a78a8f3a10bf3b6466e
> > prerequisite-patch-id: a15bcd62a8dcd219d17489eef88b66ea5488a2a0
> > prerequisite-patch-id: 2a0eefce67b21d1f30c272fd8115b0dc1aca3897
> > --
> > 2.34.1
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf: Fix 0 count issue of cpu-clock
  2025-11-12  8:05 [PATCH] perf: Fix 0 count issue of cpu-clock Dapeng Mi
  2025-11-12 16:42 ` Ian Rogers
@ 2025-11-18  1:43 ` Namhyung Kim
  2025-11-18 10:50 ` Peter Zijlstra
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2025-11-18  1:43 UTC (permalink / raw)
  To: Dapeng Mi
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Ian Rogers,
	Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane,
	linux-kernel, linux-perf-users, Dapeng Mi, Zide Chen,
	Falcon Thomas, Xudong Hao

Hello,

On Wed, Nov 12, 2025 at 04:05:26PM +0800, Dapeng Mi wrote:
> Currently cpu-clock event always returns 0 count, e.g.,
> 
> perf stat -e cpu-clock -- sleep 1
> 
>  Performance counter stats for 'sleep 1':
>                  0      cpu-clock                        #    0.000 CPUs utilized
>        1.002308394 seconds time elapsed
> 
> The root cause is the commit 'bc4394e5e79c ("perf: Fix the throttle
>  error of some clock events")' adds PERF_EF_UPDATE flag check before
> calling cpu_clock_event_update() to update the count, however the
> PERF_EF_UPDATE flag is never set when the cpu-clock event is stopped in
> counting mode (pmu->dev() -> cpu_clock_event_del() ->
> cpu_clock_event_stop()). This leads to the cpu-clock event count is
> never updated.
> 
> To fix this issue, force to set PERF_EF_UPDATE flag for cpu-clock event
> just like what task-clock does. Besides, or flags with PERF_EF_UPDATE
> for task-clock although currently the flags argument would always be 0.
> 
> Fixes: bc4394e5e79c ("perf: Fix the throttle error of some clock events")
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
> 
> With this change, both cpu-clock and task-clock can do counting and
> samping correctly.
> 
> 1. perf stat -e cpu-clock,task-clock -- true
> 
>  Performance counter stats for 'true':
>            240,636      cpu-clock                        #    0.358 CPUs utilized
>            243,319      task-clock                       #    0.362 CPUs utilized
> 
> 2. perf record -e cpu-clock -c 10000 -Iax,bx -- sleep 1
> [ perf record: Woken up 2 times to write data ]
> [ perf record: Captured and wrote 0.028 MB perf.data (36 samples) ]
> 
> 3. perf record -e task-clock -c 10000 -Iax,bx -- sleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.029 MB perf.data (41 samples) ]
> 
>  kernel/events/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index f6a08c73f783..77d3af5959c1 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -11964,7 +11964,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags)
>  
>  static void cpu_clock_event_del(struct perf_event *event, int flags)
>  {
> -	cpu_clock_event_stop(event, flags);
> +	cpu_clock_event_stop(event, flags | PERF_EF_UPDATE);
>  }
>  
>  static void cpu_clock_event_read(struct perf_event *event)
> @@ -12043,7 +12043,7 @@ static int task_clock_event_add(struct perf_event *event, int flags)
>  
>  static void task_clock_event_del(struct perf_event *event, int flags)
>  {
> -	task_clock_event_stop(event, PERF_EF_UPDATE);
> +	task_clock_event_stop(event, flags | PERF_EF_UPDATE);
>  }
>  
>  static void task_clock_event_read(struct perf_event *event)
> 
> base-commit: 2093d8cf80fa5552d1025a78a8f3a10bf3b6466e
> prerequisite-patch-id: a15bcd62a8dcd219d17489eef88b66ea5488a2a0
> prerequisite-patch-id: 2a0eefce67b21d1f30c272fd8115b0dc1aca3897
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf: Fix 0 count issue of cpu-clock
  2025-11-12  8:05 [PATCH] perf: Fix 0 count issue of cpu-clock Dapeng Mi
  2025-11-12 16:42 ` Ian Rogers
  2025-11-18  1:43 ` Namhyung Kim
@ 2025-11-18 10:50 ` Peter Zijlstra
  2025-11-18 11:03 ` Peter Zijlstra
  2025-11-20  9:47 ` [tip: perf/urgent] " tip-bot2 for Dapeng Mi
  4 siblings, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2025-11-18 10:50 UTC (permalink / raw)
  To: Dapeng Mi
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
	Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane,
	linux-kernel, linux-perf-users, Dapeng Mi, Zide Chen,
	Falcon Thomas, Xudong Hao

On Wed, Nov 12, 2025 at 04:05:26PM +0800, Dapeng Mi wrote:
> Currently cpu-clock event always returns 0 count, e.g.,
> 
> perf stat -e cpu-clock -- sleep 1
> 
>  Performance counter stats for 'sleep 1':
>                  0      cpu-clock                        #    0.000 CPUs utilized
>        1.002308394 seconds time elapsed
> 
> The root cause is the commit 'bc4394e5e79c ("perf: Fix the throttle
>  error of some clock events")' adds PERF_EF_UPDATE flag check before
> calling cpu_clock_event_update() to update the count, however the
> PERF_EF_UPDATE flag is never set when the cpu-clock event is stopped in
> counting mode (pmu->dev() -> cpu_clock_event_del() ->
> cpu_clock_event_stop()). This leads to the cpu-clock event count is
> never updated.
> 
> To fix this issue, force to set PERF_EF_UPDATE flag for cpu-clock event
> just like what task-clock does. Besides, or flags with PERF_EF_UPDATE
> for task-clock although currently the flags argument would always be 0.
> 
> Fixes: bc4394e5e79c ("perf: Fix the throttle error of some clock events")
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> ---

So I'm forever struggling to find what tree a particular commit is in,
but afaict the above fingered commit is already in Linus' tree and so
this should go to perf/urgent, right?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf: Fix 0 count issue of cpu-clock
  2025-11-12  8:05 [PATCH] perf: Fix 0 count issue of cpu-clock Dapeng Mi
                   ` (2 preceding siblings ...)
  2025-11-18 10:50 ` Peter Zijlstra
@ 2025-11-18 11:03 ` Peter Zijlstra
  2025-11-18 11:04   ` Peter Zijlstra
  2025-11-20  9:47 ` [tip: perf/urgent] " tip-bot2 for Dapeng Mi
  4 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2025-11-18 11:03 UTC (permalink / raw)
  To: Dapeng Mi
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
	Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane,
	linux-kernel, linux-perf-users, Dapeng Mi, Zide Chen,
	Falcon Thomas, Xudong Hao

On Wed, Nov 12, 2025 at 04:05:26PM +0800, Dapeng Mi wrote:

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index f6a08c73f783..77d3af5959c1 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -11964,7 +11964,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags)
>  
>  static void cpu_clock_event_del(struct perf_event *event, int flags)
>  {
> -	cpu_clock_event_stop(event, flags);
> +	cpu_clock_event_stop(event, flags | PERF_EF_UPDATE);
>  }
>  
>  static void cpu_clock_event_read(struct perf_event *event)
> @@ -12043,7 +12043,7 @@ static int task_clock_event_add(struct perf_event *event, int flags)
>  
>  static void task_clock_event_del(struct perf_event *event, int flags)
>  {
> -	task_clock_event_stop(event, PERF_EF_UPDATE);
> +	task_clock_event_stop(event, flags | PERF_EF_UPDATE);
>  }

I think it can both just be: PERF_EF_UPDATE. The only pmu::del() caller
hands in flags=0, but if there were to be flags added, we'd have to
audit all del methods anyway.

Also, the few comments we do have already note that ->del() must do
->stop(EF_UPDATE).

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf: Fix 0 count issue of cpu-clock
  2025-11-18 11:03 ` Peter Zijlstra
@ 2025-11-18 11:04   ` Peter Zijlstra
  2025-11-18 11:22     ` Mi, Dapeng
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2025-11-18 11:04 UTC (permalink / raw)
  To: Dapeng Mi
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
	Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane,
	linux-kernel, linux-perf-users, Dapeng Mi, Zide Chen,
	Falcon Thomas, Xudong Hao

On Tue, Nov 18, 2025 at 12:03:09PM +0100, Peter Zijlstra wrote:
> On Wed, Nov 12, 2025 at 04:05:26PM +0800, Dapeng Mi wrote:
> 
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index f6a08c73f783..77d3af5959c1 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -11964,7 +11964,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags)
> >  
> >  static void cpu_clock_event_del(struct perf_event *event, int flags)
> >  {
> > -	cpu_clock_event_stop(event, flags);
> > +	cpu_clock_event_stop(event, flags | PERF_EF_UPDATE);
> >  }
> >  
> >  static void cpu_clock_event_read(struct perf_event *event)
> > @@ -12043,7 +12043,7 @@ static int task_clock_event_add(struct perf_event *event, int flags)
> >  
> >  static void task_clock_event_del(struct perf_event *event, int flags)
> >  {
> > -	task_clock_event_stop(event, PERF_EF_UPDATE);
> > +	task_clock_event_stop(event, flags | PERF_EF_UPDATE);
> >  }
> 
> I think it can both just be: PERF_EF_UPDATE. The only pmu::del() caller
> hands in flags=0, but if there were to be flags added, we'd have to
> audit all del methods anyway.
> 
> Also, the few comments we do have already note that ->del() must do
> ->stop(EF_UPDATE).

Updated patch now sits in queue/perf/urgent.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf: Fix 0 count issue of cpu-clock
  2025-11-18 11:04   ` Peter Zijlstra
@ 2025-11-18 11:22     ` Mi, Dapeng
  2025-11-18 11:24       ` Peter Zijlstra
  0 siblings, 1 reply; 12+ messages in thread
From: Mi, Dapeng @ 2025-11-18 11:22 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
	Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane,
	linux-kernel, linux-perf-users, Dapeng Mi, Zide Chen,
	Falcon Thomas, Xudong Hao


On 11/18/2025 7:04 PM, Peter Zijlstra wrote:
> On Tue, Nov 18, 2025 at 12:03:09PM +0100, Peter Zijlstra wrote:
>> On Wed, Nov 12, 2025 at 04:05:26PM +0800, Dapeng Mi wrote:
>>
>>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>>> index f6a08c73f783..77d3af5959c1 100644
>>> --- a/kernel/events/core.c
>>> +++ b/kernel/events/core.c
>>> @@ -11964,7 +11964,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags)
>>>  
>>>  static void cpu_clock_event_del(struct perf_event *event, int flags)
>>>  {
>>> -	cpu_clock_event_stop(event, flags);
>>> +	cpu_clock_event_stop(event, flags | PERF_EF_UPDATE);
>>>  }
>>>  
>>>  static void cpu_clock_event_read(struct perf_event *event)
>>> @@ -12043,7 +12043,7 @@ static int task_clock_event_add(struct perf_event *event, int flags)
>>>  
>>>  static void task_clock_event_del(struct perf_event *event, int flags)
>>>  {
>>> -	task_clock_event_stop(event, PERF_EF_UPDATE);
>>> +	task_clock_event_stop(event, flags | PERF_EF_UPDATE);
>>>  }
>> I think it can both just be: PERF_EF_UPDATE. The only pmu::del() caller
>> hands in flags=0, but if there were to be flags added, we'd have to
>> audit all del methods anyway.
>>
>> Also, the few comments we do have already note that ->del() must do
>> ->stop(EF_UPDATE).
> Updated patch now sits in queue/perf/urgent.

Hi Peter,

Thanks for merging. If we decide not to or the "flags", then the last
sentence "Besides, or flags with PERF_EF_UPDATE for task-clock although
currently the flags argument would always be 0." in the commit message
should be dropped as well. :)

https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?h=perf/urgent&id=cfaecad27435b4eb6a22bb9d008fec4984e03a21

Thanks,

Dapeng Mi


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf: Fix 0 count issue of cpu-clock
  2025-11-18 11:22     ` Mi, Dapeng
@ 2025-11-18 11:24       ` Peter Zijlstra
  2025-12-05 23:44         ` Ian Rogers
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2025-11-18 11:24 UTC (permalink / raw)
  To: Mi, Dapeng
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
	Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane,
	linux-kernel, linux-perf-users, Dapeng Mi, Zide Chen,
	Falcon Thomas, Xudong Hao

On Tue, Nov 18, 2025 at 07:22:24PM +0800, Mi, Dapeng wrote:
> 
> On 11/18/2025 7:04 PM, Peter Zijlstra wrote:
> > On Tue, Nov 18, 2025 at 12:03:09PM +0100, Peter Zijlstra wrote:
> >> On Wed, Nov 12, 2025 at 04:05:26PM +0800, Dapeng Mi wrote:
> >>
> >>> diff --git a/kernel/events/core.c b/kernel/events/core.c
> >>> index f6a08c73f783..77d3af5959c1 100644
> >>> --- a/kernel/events/core.c
> >>> +++ b/kernel/events/core.c
> >>> @@ -11964,7 +11964,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags)
> >>>  
> >>>  static void cpu_clock_event_del(struct perf_event *event, int flags)
> >>>  {
> >>> -	cpu_clock_event_stop(event, flags);
> >>> +	cpu_clock_event_stop(event, flags | PERF_EF_UPDATE);
> >>>  }
> >>>  
> >>>  static void cpu_clock_event_read(struct perf_event *event)
> >>> @@ -12043,7 +12043,7 @@ static int task_clock_event_add(struct perf_event *event, int flags)
> >>>  
> >>>  static void task_clock_event_del(struct perf_event *event, int flags)
> >>>  {
> >>> -	task_clock_event_stop(event, PERF_EF_UPDATE);
> >>> +	task_clock_event_stop(event, flags | PERF_EF_UPDATE);
> >>>  }
> >> I think it can both just be: PERF_EF_UPDATE. The only pmu::del() caller
> >> hands in flags=0, but if there were to be flags added, we'd have to
> >> audit all del methods anyway.
> >>
> >> Also, the few comments we do have already note that ->del() must do
> >> ->stop(EF_UPDATE).
> > Updated patch now sits in queue/perf/urgent.
> 
> Hi Peter,
> 
> Thanks for merging. If we decide not to or the "flags", then the last
> sentence "Besides, or flags with PERF_EF_UPDATE for task-clock although
> currently the flags argument would always be 0." in the commit message
> should be dropped as well. :)

Should be fixed now. Thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip: perf/urgent] perf: Fix 0 count issue of cpu-clock
  2025-11-12  8:05 [PATCH] perf: Fix 0 count issue of cpu-clock Dapeng Mi
                   ` (3 preceding siblings ...)
  2025-11-18 11:03 ` Peter Zijlstra
@ 2025-11-20  9:47 ` tip-bot2 for Dapeng Mi
  4 siblings, 0 replies; 12+ messages in thread
From: tip-bot2 for Dapeng Mi @ 2025-11-20  9:47 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Dapeng Mi, Peter Zijlstra (Intel), Ian Rogers, Namhyung Kim, x86,
	linux-kernel

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     f1f96511b1c4c33e53f05909dd267878e0643a9a
Gitweb:        https://git.kernel.org/tip/f1f96511b1c4c33e53f05909dd267878e0643a9a
Author:        Dapeng Mi <dapeng1.mi@linux.intel.com>
AuthorDate:    Wed, 12 Nov 2025 16:05:26 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Thu, 20 Nov 2025 10:42:12 +01:00

perf: Fix 0 count issue of cpu-clock

Currently cpu-clock event always returns 0 count, e.g.,

perf stat -e cpu-clock -- sleep 1

 Performance counter stats for 'sleep 1':
                 0      cpu-clock                        #    0.000 CPUs utilized
       1.002308394 seconds time elapsed

The root cause is the commit 'bc4394e5e79c ("perf: Fix the throttle
 error of some clock events")' adds PERF_EF_UPDATE flag check before
calling cpu_clock_event_update() to update the count, however the
PERF_EF_UPDATE flag is never set when the cpu-clock event is stopped in
counting mode (pmu->dev() -> cpu_clock_event_del() ->
cpu_clock_event_stop()). This leads to the cpu-clock event count is
never updated.

To fix this issue, force to set PERF_EF_UPDATE flag for cpu-clock event
just like what task-clock does.

Fixes: bc4394e5e79c ("perf: Fix the throttle error of some clock events")
Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Ian Rogers <irogers@google.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Link: https://patch.msgid.link/20251112080526.3971392-1-dapeng1.mi@linux.intel.com
---
 kernel/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1fd347d..2c35acc 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11901,7 +11901,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags)
 
 static void cpu_clock_event_del(struct perf_event *event, int flags)
 {
-	cpu_clock_event_stop(event, flags);
+	cpu_clock_event_stop(event, PERF_EF_UPDATE);
 }
 
 static void cpu_clock_event_read(struct perf_event *event)

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf: Fix 0 count issue of cpu-clock
  2025-11-18 11:24       ` Peter Zijlstra
@ 2025-12-05 23:44         ` Ian Rogers
  2025-12-08  5:16           ` Mi, Dapeng
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Rogers @ 2025-12-05 23:44 UTC (permalink / raw)
  To: Peter Zijlstra, Mi, Dapeng, Ingo Molnar
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Adrian Hunter,
	Alexander Shishkin, Andi Kleen, Eranian Stephane, linux-kernel,
	linux-perf-users, Dapeng Mi, Zide Chen, Falcon Thomas, Xudong Hao

On Tue, Nov 18, 2025 at 3:24 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Tue, Nov 18, 2025 at 07:22:24PM +0800, Mi, Dapeng wrote:
> >
> > On 11/18/2025 7:04 PM, Peter Zijlstra wrote:
> > > On Tue, Nov 18, 2025 at 12:03:09PM +0100, Peter Zijlstra wrote:
> > >> Also, the few comments we do have already note that ->del() must do
> > >> ->stop(EF_UPDATE).

Hi Peter,

I was trying to do a quick sanity check that other callers of stop
were passing in EF_UPDATE. I wondered if the case of
perf_event_throttle_group with leader sampling has an issue? The
throttle should only happen on an event or group leader that is in
sampling mode, but the group members could be non-sampling with leader
sampling. Sorry for the noise if I'm wrong on this. Fwiw, the other
uses either pass in EF_UPDATE or I think for  __perf_event_aux_pause
and  __perf_event_overflow the problem isn't relevant.

Thanks,
Ian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf: Fix 0 count issue of cpu-clock
  2025-12-05 23:44         ` Ian Rogers
@ 2025-12-08  5:16           ` Mi, Dapeng
  0 siblings, 0 replies; 12+ messages in thread
From: Mi, Dapeng @ 2025-12-08  5:16 UTC (permalink / raw)
  To: Ian Rogers, Peter Zijlstra, Ingo Molnar
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Adrian Hunter,
	Alexander Shishkin, Andi Kleen, Eranian Stephane, linux-kernel,
	linux-perf-users, Dapeng Mi, Zide Chen, Falcon Thomas, Xudong Hao


On 12/6/2025 7:44 AM, Ian Rogers wrote:
> On Tue, Nov 18, 2025 at 3:24 AM Peter Zijlstra <peterz@infradead.org> wrote:
>> On Tue, Nov 18, 2025 at 07:22:24PM +0800, Mi, Dapeng wrote:
>>> On 11/18/2025 7:04 PM, Peter Zijlstra wrote:
>>>> On Tue, Nov 18, 2025 at 12:03:09PM +0100, Peter Zijlstra wrote:
>>>>> Also, the few comments we do have already note that ->del() must do
>>>>> ->stop(EF_UPDATE).
> Hi Peter,
>
> I was trying to do a quick sanity check that other callers of stop
> were passing in EF_UPDATE. I wondered if the case of
> perf_event_throttle_group with leader sampling has an issue? The
> throttle should only happen on an event or group leader that is in
> sampling mode, but the group members could be non-sampling with leader
> sampling. Sorry for the noise if I'm wrong on this. Fwiw, the other
> uses either pass in EF_UPDATE or I think for  __perf_event_aux_pause
> and  __perf_event_overflow the problem isn't relevant.

Base on the logic, it looks we need to add the flag EF_UPDATE in
perf_event_throttle_group(), but need Peter's confirm. 


>
> Thanks,
> Ian
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-12-08  5:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-12  8:05 [PATCH] perf: Fix 0 count issue of cpu-clock Dapeng Mi
2025-11-12 16:42 ` Ian Rogers
2025-11-17 17:04   ` Ian Rogers
2025-11-18  1:43 ` Namhyung Kim
2025-11-18 10:50 ` Peter Zijlstra
2025-11-18 11:03 ` Peter Zijlstra
2025-11-18 11:04   ` Peter Zijlstra
2025-11-18 11:22     ` Mi, Dapeng
2025-11-18 11:24       ` Peter Zijlstra
2025-12-05 23:44         ` Ian Rogers
2025-12-08  5:16           ` Mi, Dapeng
2025-11-20  9:47 ` [tip: perf/urgent] " tip-bot2 for Dapeng Mi

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.