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

On Sun, Apr 12, 2026 at 02:25:37AM +0000, [email protected] 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 <[email protected]>
> 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