Re: [PATCH perf-tools-next v12 05/10] perf ftrace: Support context switch tracking in function graph tracer

[Aaron Tomlin]

On Tue, Nov 03, 2026 at 11:42:53PM -0800, Namhyung Kim wrote:
> On Sun, Nov 01, 2026 at 07:04:17PM -0500, Aaron Tomlin wrote:
> > On Sat, Oct 31, 2026 at 03:01:51PM -0700, Namhyung Kim wrote:
> > > On Sun, Oct 25, 2026 at 10:57:30PM -0400, Aaron Tomlin wrote:
> [SNIP]
> > > > +
> > > > +		/*
> > > > +		 * Prune frames on existing and ts that exited during subsequent
> > > > +		 * execution recorded on the other state.
> > > > +		 */
> > > > +		ftrace_task_state__prune_stack(existing, ts);
> > > > +		ftrace_task_state__prune_stack(ts, existing);
> > > 
> > > I'm not sure about this.  In this context, it knows the 'ts' is uptodate
> > > and the 'existing' contains the old state.  Does it need to prune the ts
> > > using the existing?
> > > 
> > > IIUC this code assumes either ts or existing may have uptodate state
> > > like in general cases but it would make the code unnecessarily longer.
> > 
> > The reason this handled both directions is due to how trace_pipe merges
> > per-CPU buffers when a task starts on CPU 0 (unidentified, tracked in ts
> > under FTRACE_UNKNOWN_PID(0)) and migrates to CPU 1:
> > 
> >     1.  Task X starts on CPU 0 before any context switch banner is seen on
> >         CPU 0, so its initial frames are recorded in ts.
> > 
> >     2.  Task X migrates to CPU 1. If CPU 1's incoming banner ("1) idle =>
> >         X") and subsequent function exits are read from trace_pipe before
> >         CPU 0's outgoing banner ("0) X => idle") is flushed, existing is
> >         created on CPU 1 first and actually holds the newer state, while
> >         ts holds the older pre-migration frames from CPU 0.
> > 
> >     3.  Conversely, if Task X was already known on CPU 1 (existing) and
> >         migrated to an unbannered CPU 0 (ts), then ts holds the newer state
> >         and existing holds the older state.
> 
> IIRC we use the global trace pipe so I expect its output in time order.
> How can it see CPU 1's sched switch before the CPU 0's?  Maybe some time
> drifts on each CPU?  Anyway, if that happens in practice, yeah then we
> can check in both directions.

Hi Namhyung,

Yes, global trace_pipe is in timestamp order.

The reason is that function_graph does not emit context-switch banners at
the time of sched_switch; instead, verif_pid() in
kernel/trace/trace_functions_graph.c emits "CPU) prev => next" lazily only
when the next traced function entry/return event is formatted on that CPU.

So if Task X runs traced functions on CPU 0 (i.e. where prev_pid was -1, no
banner was printed yet) and then schedules out on CPU 0 to an untraced task
(e.g. under -T|-G filters or idle) before migrating to CPU 1:

    - CPU 0 emits no fgraph events while the untraced task runs, so CPU 0's
      "0) X => ..." banner is not emitted yet

    - Task X immediately hits traced functions on CPU 1, which triggers
      verif_pid() on CPU 1 and emits "1) ... => X" (and subsequent exits)
      at an earlier timestamp

    - Only later, when a task on CPU 0 next hits a traced function, does
      verif_pid() on CPU 0 emit "0) X => ..."


> > That said, I agree that maintaining a dynamic per-task watermarks[] array
> > for arbitrary multi-hop migrations before the first banner is superfluous.
> > Replacing the dynamic watermarks array with a single scalar minimum-indent
> > watermark on struct ftrace_task_state makes this much simpler and removes
> > ftrace_task_state__record_indent(), ftrace_task_state__min_indent_after(),
> > and the watermark merge loop in ftrace_task_state__rekey().
> 
> Sounds good, thanks!

Acknowledged!

Please ignore my v13. Sashiko found additional valid concerns which I need
to address first.


Thank you,
-- 
Aaron Tomlin