[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fU33sEARn0tc1hSMahBVCvs0cy+Cu-J6+BG0Cm-nuwKnA@mail.gmail.com>
Date: Fri, 14 Nov 2025 10:09:26 -0800
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, James Clark <james.clark@...aro.org>,
Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>,
LKML <linux-kernel@...r.kernel.org>, linux-perf-users@...r.kernel.org,
Steven Rostedt <rostedt@...dmis.org>, Josh Poimboeuf <jpoimboe@...nel.org>,
Indu Bhagat <indu.bhagat@...cle.com>, Jens Remus <jremus@...ux.ibm.com>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, linux-trace-kernel@...r.kernel.org,
bpf@...r.kernel.org
Subject: Re: [PATCH v3 3/5] perf record: Enable defer_callchain for user callchains
On Fri, Nov 14, 2025 at 9:59 AM Ian Rogers <irogers@...gle.com> wrote:
>
> On Thu, Nov 13, 2025 at 11:01 PM Namhyung Kim <namhyung@...nel.org> wrote:
> >
> > And add the missing feature detection logic to clear the flag on old
> > kernels.
> >
> > $ perf record -g -vv true
> > ...
> > ------------------------------------------------------------
> > perf_event_attr:
> > type 0 (PERF_TYPE_HARDWARE)
> > size 136
> > config 0 (PERF_COUNT_HW_CPU_CYCLES)
> > { sample_period, sample_freq } 4000
> > sample_type IP|TID|TIME|CALLCHAIN|PERIOD
> > read_format ID|LOST
> > disabled 1
> > inherit 1
> > mmap 1
> > comm 1
> > freq 1
> > enable_on_exec 1
> > task 1
> > sample_id_all 1
> > mmap2 1
> > comm_exec 1
> > ksymbol 1
> > bpf_event 1
> > defer_callchain 1
> > defer_output 1
> > ------------------------------------------------------------
> > sys_perf_event_open: pid 162755 cpu 0 group_fd -1 flags 0x8
> > sys_perf_event_open failed, error -22
> > switching off deferred callchain support
> >
> > Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> > ---
> > tools/perf/util/evsel.c | 24 ++++++++++++++++++++++++
> > tools/perf/util/evsel.h | 1 +
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index 244b3e44d090d413..f5652d00b457d096 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -1061,6 +1061,14 @@ static void __evsel__config_callchain(struct evsel *evsel, struct record_opts *o
> > }
> > }
> >
> > + if (param->record_mode == CALLCHAIN_FP && !attr->exclude_callchain_user) {
> > + /*
> > + * Enable deferred callchains optimistically. It'll be switched
> > + * off later if the kernel doesn't support it.
> > + */
> > + attr->defer_callchain = 1;
> > + }
>
> If a user has requested frame pointer call chains why would they want
> deferred call chains? The point of deferral to my understanding is to
> allow the paging in of debug data, but frame pointers don't need that
> as the stack should be in the page cache.
>
> Is this being done for code coverage reasons so that deferral is known
> to work for later addition of SFrames? In which case this should be an
> opt-in not default behavior. When there is a record_mode of
> CALLCHAIN_SFRAME then making deferral the default for that mode makes
> sense, but not for frame pointers IMO.
Just to be clear. I don't think the behavior of using frame pointers
should change. Deferral has downsides, for example:
$ perf record -g -a sleep 1
Without deferral kernel stack traces will contain both kernel and user
traces. With deferral the user stack trace is only generated when the
system call returns and so there is a chance for kernel stack traces
to be missing their user part. An obvious behavioral change. I think
for what you are doing here we can have an option something like:
$ perf record --call-graph fp-deferred -a sleep 1
Which would need a man page update, etc. What is happening with the
other call-graph modes and deferral? Could the option be something
like `--call-graph fp,deferred` so that the option is a common one and
say stack snapshots for dwarf be somehow improved?
Thanks,
Ian
> Thanks,
> Ian
>
> > +
> > if (function) {
> > pr_info("Disabling user space callchains for function trace event.\n");
> > attr->exclude_callchain_user = 1;
> > @@ -1511,6 +1519,7 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
> > attr->mmap2 = track && !perf_missing_features.mmap2;
> > attr->comm = track;
> > attr->build_id = track && opts->build_id;
> > + attr->defer_output = track;
> >
> > /*
> > * ksymbol is tracked separately with text poke because it needs to be
> > @@ -2199,6 +2208,10 @@ static int __evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
> >
> > static void evsel__disable_missing_features(struct evsel *evsel)
> > {
> > + if (perf_missing_features.defer_callchain && evsel->core.attr.defer_callchain)
> > + evsel->core.attr.defer_callchain = 0;
> > + if (perf_missing_features.defer_callchain && evsel->core.attr.defer_output)
> > + evsel->core.attr.defer_output = 0;
> > if (perf_missing_features.inherit_sample_read && evsel->core.attr.inherit &&
> > (evsel->core.attr.sample_type & PERF_SAMPLE_READ))
> > evsel->core.attr.inherit = 0;
> > @@ -2473,6 +2486,13 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu
> >
> > /* Please add new feature detection here. */
> >
> > + attr.defer_callchain = true;
> > + if (has_attr_feature(&attr, /*flags=*/0))
> > + goto found;
> > + perf_missing_features.defer_callchain = true;
> > + pr_debug2("switching off deferred callchain support\n");
> > + attr.defer_callchain = false;
> > +
> > attr.inherit = true;
> > attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID;
> > if (has_attr_feature(&attr, /*flags=*/0))
> > @@ -2584,6 +2604,10 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu
> > errno = old_errno;
> >
> > check:
> > + if ((evsel->core.attr.defer_callchain || evsel->core.attr.defer_output) &&
> > + perf_missing_features.defer_callchain)
> > + return true;
> > +
> > if (evsel->core.attr.inherit &&
> > (evsel->core.attr.sample_type & PERF_SAMPLE_READ) &&
> > perf_missing_features.inherit_sample_read)
> > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> > index 3ae4ac8f9a37e009..a08130ff2e47a887 100644
> > --- a/tools/perf/util/evsel.h
> > +++ b/tools/perf/util/evsel.h
> > @@ -221,6 +221,7 @@ struct perf_missing_features {
> > bool branch_counters;
> > bool aux_action;
> > bool inherit_sample_read;
> > + bool defer_callchain;
> > };
> >
> > extern struct perf_missing_features perf_missing_features;
> > --
> > 2.52.0.rc1.455.g30608eb744-goog
> >
> >
Powered by blists - more mailing lists