mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] perf/core: Fix ITRACE start suppression for inherited events
@ 2026-09-03  9:35 Leo Yan
  2026-09-28  8:33 ` Leo Yan
  2026-09-28  9:38 ` James Clark
  0 siblings, 2 replies; 4+ messages in thread
From: Leo Yan @ 2026-09-03  9:35 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, James Clark
  Cc: Ingo Molnar, linux-perf-users, linux-kernel, Sashiko AI, Leo Yan

PMU drivers call perf_event_itrace_started() for the event that has
started tracing. This sets PERF_ATTACH_ITRACE in that event's
attach_state.

For inherited events, however, perf_log_itrace_start() replaces the
child event with its parent before checking PERF_ATTACH_ITRACE. The
setter and checker therefore operate on different events. If the
parent's flag is clear, the child continues to emit ITRACE_START
records on subsequent schedule-ins. If the parent has already started,
its flag can instead suppress the child's initial record.

Remove the parent substitution so that perf_log_itrace_start() checks
the same event that the PMU driver marks as started.

This is safe for tool consumers. Intel PT uses the ITRACE_START record
to set the current thread context. CoreSight ETM uses the record only
to find or create the corresponding thread. Neither decoder depends on
the parent event.

Reported-by: Sashiko AI <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-perf-users/20260901164705.042781F000E9@smtp.kernel.org/
Fixes: 9a6694cfa239 ("perf/x86/intel/pt: Do not force sync packets on every schedule-in")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 kernel/events/core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index a6c8e38a311042afab6b65814a84c67b87ba929b..991ae214d46ebe8b0d9de497255f25a4ad12397f 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10572,9 +10572,6 @@ static void perf_log_itrace_start(struct perf_event *event)
 	} rec;
 	int ret;
 
-	if (event->parent)
-		event = event->parent;
-
 	if (!(event->pmu->capabilities & PERF_PMU_CAP_ITRACE) ||
 	    event->attach_state & PERF_ATTACH_ITRACE)
 		return;

---
base-commit: 940de590b839f71d6dc846160534bf202401b8b7
change-id: 20260903-perf_core_itrace_start_fix_inherit_event-a8930d792899

Best regards,
-- 
Leo Yan <leo.yan@arm.com>


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

* Re: [PATCH] perf/core: Fix ITRACE start suppression for inherited events
  2026-09-03  9:35 [PATCH] perf/core: Fix ITRACE start suppression for inherited events Leo Yan
@ 2026-09-28  8:33 ` Leo Yan
  2026-09-28  9:38 ` James Clark
  1 sibling, 0 replies; 4+ messages in thread
From: Leo Yan @ 2026-09-28  8:33 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, James Clark
  Cc: Ingo Molnar, linux-perf-users, linux-kernel, Sashiko AI

On Thu, Sep 03, 2026 at 10:35:12AM +0100, Leo Yan wrote:
> PMU drivers call perf_event_itrace_started() for the event that has
> started tracing. This sets PERF_ATTACH_ITRACE in that event's
> attach_state.
> 
> For inherited events, however, perf_log_itrace_start() replaces the
> child event with its parent before checking PERF_ATTACH_ITRACE. The
> setter and checker therefore operate on different events. If the
> parent's flag is clear, the child continues to emit ITRACE_START
> records on subsequent schedule-ins. If the parent has already started,
> its flag can instead suppress the child's initial record.
> 
> Remove the parent substitution so that perf_log_itrace_start() checks
> the same event that the PMU driver marks as started.
> 
> This is safe for tool consumers. Intel PT uses the ITRACE_START record
> to set the current thread context. CoreSight ETM uses the record only
> to find or create the corresponding thread. Neither decoder depends on
> the parent event.
> 
> Reported-by: Sashiko AI <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/linux-perf-users/20260901164705.042781F000E9@smtp.kernel.org/
> Fixes: 9a6694cfa239 ("perf/x86/intel/pt: Do not force sync packets on every schedule-in")
> Signed-off-by: Leo Yan <leo.yan@arm.com>

Gentle ping ...

> ---
>  kernel/events/core.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a6c8e38a311042afab6b65814a84c67b87ba929b..991ae214d46ebe8b0d9de497255f25a4ad12397f 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -10572,9 +10572,6 @@ static void perf_log_itrace_start(struct perf_event *event)
>  	} rec;
>  	int ret;
>  
> -	if (event->parent)
> -		event = event->parent;
> -
>  	if (!(event->pmu->capabilities & PERF_PMU_CAP_ITRACE) ||
>  	    event->attach_state & PERF_ATTACH_ITRACE)
>  		return;
> 
> ---
> base-commit: 940de590b839f71d6dc846160534bf202401b8b7
> change-id: 20260903-perf_core_itrace_start_fix_inherit_event-a8930d792899
> 
> Best regards,
> -- 
> Leo Yan <leo.yan@arm.com>
> 

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

* Re: [PATCH] perf/core: Fix ITRACE start suppression for inherited events
  2026-09-03  9:35 [PATCH] perf/core: Fix ITRACE start suppression for inherited events Leo Yan
  2026-09-28  8:33 ` Leo Yan
@ 2026-09-28  9:38 ` James Clark
  2026-09-29 14:53   ` Leo Yan
  1 sibling, 1 reply; 4+ messages in thread
From: James Clark @ 2026-09-28  9:38 UTC (permalink / raw)
  To: Leo Yan, Alexander Shishkin
  Cc: Ingo Molnar, linux-perf-users, linux-kernel, Sashiko AI,
	Jiri Olsa, Ian Rogers, Adrian Hunter, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim,
	Mark Rutland



On 03/09/2026 10:35, Leo Yan wrote:
> PMU drivers call perf_event_itrace_started() for the event that has
> started tracing. This sets PERF_ATTACH_ITRACE in that event's
> attach_state.
> 
> For inherited events, however, perf_log_itrace_start() replaces the
> child event with its parent before checking PERF_ATTACH_ITRACE. The
> setter and checker therefore operate on different events. If the
> parent's flag is clear, the child continues to emit ITRACE_START
> records on subsequent schedule-ins. If the parent has already started,
> its flag can instead suppress the child's initial record.

This last part would need an earlier fixes: commit. The problem of 
suppressing child ITRACE_START records existed since the beginning on 
ec0d772 ("perf: Add ITRACE_START record to indicate that tracing has 
started").

Although the fixes: commit would be correct if the only problem was that 
setting and getting are on different events.

> 
> Remove the parent substitution so that perf_log_itrace_start() checks
> the same event that the PMU driver marks as started.
> 
> This is safe for tool consumers. Intel PT uses the ITRACE_START record
> to set the current thread context. CoreSight ETM uses the record only
> to find or create the corresponding thread. Neither decoder depends on
> the parent event.
> 

It's probably harmless to emit more ITRACE_STARTs, but it doesn't fit 
the original purpose of why it was added. It seems to be for when 
tracing first starts, which would be _after_ the corresponding sched 
event. Once tracing has started you can follow the subsequent sched 
events, so you don't need more ITRACE_START records for each child. 
Could we not fix it by changing the setter to follow the parent event to 
match, which would respect the original meaning?


> Reported-by: Sashiko AI <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/linux-perf-users/20260901164705.042781F000E9@smtp.kernel.org/
> Fixes: 9a6694cfa239 ("perf/x86/intel/pt: Do not force sync packets on every schedule-in")
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   kernel/events/core.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a6c8e38a311042afab6b65814a84c67b87ba929b..991ae214d46ebe8b0d9de497255f25a4ad12397f 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -10572,9 +10572,6 @@ static void perf_log_itrace_start(struct perf_event *event)
>   	} rec;
>   	int ret;
>   
> -	if (event->parent)
> -		event = event->parent;
> -
>   	if (!(event->pmu->capabilities & PERF_PMU_CAP_ITRACE) ||
>   	    event->attach_state & PERF_ATTACH_ITRACE)
>   		return;
> 
> ---
> base-commit: 940de590b839f71d6dc846160534bf202401b8b7
> change-id: 20260903-perf_core_itrace_start_fix_inherit_event-a8930d792899
> 
> Best regards,


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

* Re: [PATCH] perf/core: Fix ITRACE start suppression for inherited events
  2026-09-28  9:38 ` James Clark
@ 2026-09-29 14:53   ` Leo Yan
  0 siblings, 0 replies; 4+ messages in thread
From: Leo Yan @ 2026-09-29 14:53 UTC (permalink / raw)
  To: James Clark
  Cc: Alexander Shishkin, Ingo Molnar, linux-perf-users, linux-kernel,
	Sashiko AI, Jiri Olsa, Ian Rogers, Adrian Hunter, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim,
	Mark Rutland

On Mon, Sep 28, 2026 at 10:38:42AM +0100, James Clark wrote:
> 
> On 03/09/2026 10:35, Leo Yan wrote:
> > PMU drivers call perf_event_itrace_started() for the event that has
> > started tracing. This sets PERF_ATTACH_ITRACE in that event's
> > attach_state.
> > 
> > For inherited events, however, perf_log_itrace_start() replaces the
> > child event with its parent before checking PERF_ATTACH_ITRACE. The
> > setter and checker therefore operate on different events. If the
> > parent's flag is clear, the child continues to emit ITRACE_START
> > records on subsequent schedule-ins. If the parent has already started,
> > its flag can instead suppress the child's initial record.
> 
> This last part would need an earlier fixes: commit. The problem of
> suppressing child ITRACE_START records existed since the beginning on
> ec0d772 ("perf: Add ITRACE_START record to indicate that tracing has
> started").
> 
> Although the fixes: commit would be correct if the only problem was that
> setting and getting are on different events.

Makes sense. However, if follow your suggestion to change the setter to
fix the setter/checker mismatch, 9a6694cfa239 remains the appropriate
Fixes tag, since it introduced that mismatch; and it is a feasible
point for back port.

> > Remove the parent substitution so that perf_log_itrace_start() checks
> > the same event that the PMU driver marks as started.
> > 
> > This is safe for tool consumers. Intel PT uses the ITRACE_START record
> > to set the current thread context. CoreSight ETM uses the record only
> > to find or create the corresponding thread. Neither decoder depends on
> > the parent event.
> > 
> 
> It's probably harmless to emit more ITRACE_STARTs, but it doesn't fit the
> original purpose of why it was added. It seems to be for when tracing first
> starts, which would be _after_ the corresponding sched event. Once tracing
> has started you can follow the subsequent sched events, so you don't need
> more ITRACE_START records for each child. Could we not fix it by changing
> the setter to follow the parent event to match, which would respect the
> original meaning?

My concern was whether an inherited event and its parent could trace
concurrently on different CPUs.

Normal inherited AUX recording uses CPU restricted events (cpu != -1),
children inherit the parent event’s CPU restriction, a particular parent
event and its children cannot trace concurrently on different CPUs.

Since inherit with cpu == -1 prevents buffer mapping [1], my concern for
this case is also not valid.

I will update the patch to update setter. Thanks for suggestion.

[1] https://man7.org/linux/man-pages/man2/perf_event_open.2.html

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

end of thread, other threads:[~2026-09-29 14:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-03  9:35 [PATCH] perf/core: Fix ITRACE start suppression for inherited events Leo Yan
2026-09-28  8:33 ` Leo Yan
2026-09-28  9:38 ` James Clark
2026-09-29 14:53   ` Leo Yan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®