* 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
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ 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] 11+ 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; 11+ 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] 11+ 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
3 siblings, 0 replies; 11+ 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] 11+ 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
3 siblings, 0 replies; 11+ 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] 11+ 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
3 siblings, 1 reply; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ messages in thread