Re: [PATCH v10 07/29] perf lock: Only pass sample to handlers

Namhyung Kim
2026-04-12T20:16:57Z

On Sun, Apr 12, 2026 at 02:25:37AM +0000, sashiko-bot@kernel.org wrote:
> Sashiko AI review found 1 potential issue(s):
> - [Medium] Out-of-memory errors from get_callstack() are silently swallowed.
> --
>
> commit 4d8eb4c521b8cbbd581b94a51ae57655ebd7ab69
> Author: Ian Rogers <irogers@google.com>
> perf lock: Only pass sample to handlers
>
> This commit removes the unused evsel parameter from lock event handlers and
> adds a NULL check for sample->callchain in get_callstack().
>
> > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> > index e2b585e528aca..fcc33089b9455 100644
> > --- a/tools/perf/builtin-lock.c
> > +++ b/tools/perf/builtin-lock.c
> [ ... ]
> > @@ -948,9 +938,16 @@ static u64 *get_callstack(struct perf_sample *sample, int max_stack)
> > u64 i;
> > int c;
> >
> > + if (!sample->callchain) {
> > + pr_debug("Sample unexpectedly missing callchain\n");
> > + return NULL;
> > + }
> > +
> > callstack = calloc(max_stack, sizeof(*callstack));
> > - if (callstack == NULL)
> > + if (callstack == NULL) {
> > + pr_debug("Failed to allocate callstack\n");
> > return NULL;
> > + }
> >
> > for (i = 0, c = 0; i < sample->callchain->nr && c < max_stack; i++) {
> > u64 ip = sample->callchain->ips[i];
> [ ... ]
> > @@ -1070,7 +1066,7 @@ static int report_lock_contention_begin_event(struct evsel *evsel __maybe_unused
> > if (needs_callstack()) {
> > u64 *callstack = get_callstack(sample, max_stack_depth);
> > if (callstack == NULL)
> > - return -ENOMEM;
> > + return 0;
>
> Does this inadvertently swallow the memory allocation failure from
> get_callstack()?
>
> Since get_callstack() returns NULL both when the callchain is missing and
> when calloc() fails, changing this return from -ENOMEM to 0 means the tool
> will now silently drop events when memory is exhausted instead of propagating
> the error.

I've removed this part for now.

Thanks,
Namhyung