Re: [PATCHv3 bpf-next 08/12] bpf: Remove trace_in argument from __bpf_get_stack

[Jiri Olsa <olsajiri@gmail.com>]

On Mon, Aug 03, 2026 at 10:21:34PM +0000, bot+bpf-ci@kernel.org wrote:

SNIP

> > +	if (kernel && user_build_id)
> > +		return -EINVAL;
> > +
> > +	elem_size = user_build_id ? sizeof(struct bpf_stack_build_id) : sizeof(u64);
> > +	if (unlikely(size % elem_size))
> > +		return -EINVAL;
> > +
> > +	max_depth = stack_map_calculate_max_depth(size, elem_size, flags);
> > +	trace->nr = min_t(u32, trace->nr, max_depth);
> > +
> > +	if (trace->nr < skip)
> > +		return -EFAULT;
> > +
> > +	nr_trace = callchain_store(trace, buf, elem_size, flags);
> > +	return callchain_finalize(buf, size, nr_trace, elem_size, flags, false /* !may_fault */);
> > +}
> 
> This isn't a bug, but would naming this variable trace_nr match
> __bpf_get_stack() and the callchain_*() parameters it feeds?
> 
> Looking at __bpf_get_stack():
> 
> kernel/bpf/stackmap.c:__bpf_get_stack() {
> 	u32 trace_nr, elem_size, max_depth;
> 	...
> 	trace_nr = callchain_store(trace, buf, elem_size, flags);
> 	return callchain_finalize(buf, size, trace_nr, elem_size, flags, may_fault);
> }
> 
> And the shared helpers both expect trace_nr:
> 
> kernel/bpf/stackmap.c:callchain_store(struct perf_callchain_entry *trace, ...) {
> 	for (i = skip; i < trace_nr; i++) {
> 		...
> 	}
> 	return trace_nr - skip;
> }
> 
> kernel/bpf/stackmap.c:callchain_finalize(void *buf, u32 size, u32 trace_nr, ...) {
> 	u32 copy_len = trace_nr * elem_size;
> 	...
> }
> 
> The new helper uses nr_trace for the same role, making the two otherwise
> parallel functions read differently.

that rename actualy uncovered that later __bpf_get_stack_pe used both
trace_nr and nr_trace.. we're fine with just one trace_nr, will change

jirka