Re: [PATCH v3 2/5] perf tools: Minimal DEFERRED_CALLCHAIN support

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Nov 14, 2025 at 09:52:41AM -0800, Ian Rogers wrote:
> On Thu, Nov 13, 2025 at 11:00 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Add a new event type for deferred callchains and a new callback for the
> > struct perf_tool.  For now it doesn't actually handle the deferred
> > callchains but it just marks the sample if it has the PERF_CONTEXT_
> > USER_DEFFERED in the callchain array.
> >
> > At least, perf report can dump the raw data with this change.  Actually
> > this requires the next commit to enable attr.defer_callchain, but if you
> > already have a data file, it'll show the following result.
> >
> >   $ perf report -D
> >   ...
> >   0x2158@perf.data [0x40]: event: 22
> >   .
> >   . ... raw event: size 64 bytes
> >   .  0000:  16 00 00 00 02 00 40 00 06 00 00 00 0b 00 00 00  ......@.........
> >   .  0010:  03 00 00 00 00 00 00 00 a7 7f 33 fe 18 7f 00 00  ..........3.....
> >   .  0020:  0f 0e 33 fe 18 7f 00 00 48 14 33 fe 18 7f 00 00  ..3.....H.3.....
> >   .  0030:  08 09 00 00 08 09 00 00 e6 7a e7 35 1c 00 00 00  .........z.5....
> >
> >   121163447014 0x2158 [0x40]: PERF_RECORD_CALLCHAIN_DEFERRED(IP, 0x2): 2312/2312: 0xb00000006
> >   ... FP chain: nr:3
> >   .....  0: 00007f18fe337fa7
> >   .....  1: 00007f18fe330e0f
> >   .....  2: 00007f18fe331448
> >   : unhandled!
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/lib/perf/include/perf/event.h       |  8 ++++++++
> >  tools/perf/util/event.c                   |  1 +
> >  tools/perf/util/evsel.c                   | 19 +++++++++++++++++++
> >  tools/perf/util/machine.c                 |  1 +
> >  tools/perf/util/perf_event_attr_fprintf.c |  2 ++
> >  tools/perf/util/sample.h                  |  2 ++
> >  tools/perf/util/session.c                 | 20 ++++++++++++++++++++
> >  tools/perf/util/tool.c                    |  1 +
> >  tools/perf/util/tool.h                    |  3 ++-
> >  9 files changed, 56 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h
> > index aa1e91c97a226e1a..769bc48ca85c0eb8 100644
> > --- a/tools/lib/perf/include/perf/event.h
> > +++ b/tools/lib/perf/include/perf/event.h
> > @@ -151,6 +151,13 @@ struct perf_record_switch {
> >         __u32                    next_prev_tid;
> >  };
> >
> > +struct perf_record_callchain_deferred {
> > +       struct perf_event_header header;
> > +       __u64                    cookie;
> 
> Could we add a comment that this value is used to match user and
> kernel stack traces together? I don't believe that intent is
> immediately obvious from the word "cookie".

Sounds good, will add.

> 
> > +       __u64                    nr;
> > +       __u64                    ips[];
> > +};
> > +
> >  struct perf_record_header_attr {
> >         struct perf_event_header header;
> >         struct perf_event_attr   attr;
> > @@ -523,6 +530,7 @@ union perf_event {
> >         struct perf_record_read                 read;
> >         struct perf_record_throttle             throttle;
> >         struct perf_record_sample               sample;
> > +       struct perf_record_callchain_deferred   callchain_deferred;
> >         struct perf_record_bpf_event            bpf;
> >         struct perf_record_ksymbol              ksymbol;
> >         struct perf_record_text_poke_event      text_poke;
> > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> > index fcf44149feb20c35..4c92cc1a952c1d9f 100644
> > --- a/tools/perf/util/event.c
> > +++ b/tools/perf/util/event.c
> > @@ -61,6 +61,7 @@ static const char *perf_event__names[] = {
> >         [PERF_RECORD_CGROUP]                    = "CGROUP",
> >         [PERF_RECORD_TEXT_POKE]                 = "TEXT_POKE",
> >         [PERF_RECORD_AUX_OUTPUT_HW_ID]          = "AUX_OUTPUT_HW_ID",
> > +       [PERF_RECORD_CALLCHAIN_DEFERRED]        = "CALLCHAIN_DEFERRED",
> >         [PERF_RECORD_HEADER_ATTR]               = "ATTR",
> >         [PERF_RECORD_HEADER_EVENT_TYPE]         = "EVENT_TYPE",
> >         [PERF_RECORD_HEADER_TRACING_DATA]       = "TRACING_DATA",
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index 989c56d4a23f74f4..244b3e44d090d413 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -3089,6 +3089,20 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
> >         data->data_src = PERF_MEM_DATA_SRC_NONE;
> >         data->vcpu = -1;
> >
> > +       if (event->header.type == PERF_RECORD_CALLCHAIN_DEFERRED) {
> > +               const u64 max_callchain_nr = UINT64_MAX / sizeof(u64);
> > +
> > +               data->callchain = (struct ip_callchain *)&event->callchain_deferred.nr;
> > +               if (data->callchain->nr > max_callchain_nr)
> > +                       return -EFAULT;
> > +
> > +               data->deferred_cookie = event->callchain_deferred.cookie;
> > +
> > +               if (evsel->core.attr.sample_id_all)
> > +                       perf_evsel__parse_id_sample(evsel, event, data);
> > +               return 0;
> > +       }
> > +
> >         if (event->header.type != PERF_RECORD_SAMPLE) {
> >                 if (!evsel->core.attr.sample_id_all)
> >                         return 0;
> > @@ -3219,6 +3233,11 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
> >                 if (data->callchain->nr > max_callchain_nr)
> >                         return -EFAULT;
> >                 sz = data->callchain->nr * sizeof(u64);
> > +               if (evsel->core.attr.defer_callchain && data->callchain->nr >= 2 &&
> > +                   data->callchain->ips[data->callchain->nr - 2] == PERF_CONTEXT_USER_DEFERRED) {
> > +                       data->deferred_cookie = data->callchain->ips[data->callchain->nr - 1];
> > +                       data->deferred_callchain = true;
> > +               }
> 
> It'd be nice to have a comment saying what is going on here. I can see
> that if there are 2 stack slots and the 2nd is a magic value then the
> first should be read as the "cookie". At a first look this code is
> difficult to parse so a comment would add value.

Will add the comment.

Thanks,
Namhyung

 
> >                 OVERFLOW_CHECK(array, sz, max_size);
> >                 array = (void *)array + sz;
> >         }