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

sashiko-bot
2026-04-12T02:25:38Z

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.

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260412020833.641177-1-irogers@google.com?part=7