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

[Namhyung Kim]

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.

> 
> 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!
Namhyung