Re: [PATCHv2 bpf-next 09/11] bpf: Remove trace_in argument from __bpf_get_stack

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Jul 31, 2026 at 08:14:03AM -0700, Andrii Nakryiko wrote:
> On Fri, Jul 31, 2026 at 6:33 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Thu, Jul 30, 2026 at 04:06:14PM -0700, Andrii Nakryiko wrote:
> >
> > SNIP
> >
> > > >  BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
> > > >            void *, buf, u32, size, u64, flags)
> > > >  {
> > > > @@ -947,7 +970,7 @@ BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
> > > >         int err = -EINVAL;
> > > >
> > > >         if (!(event->attr.sample_type & PERF_SAMPLE_CALLCHAIN))
> > > > -               return __bpf_get_stack(regs, NULL, NULL, buf, size, flags, false /* !may_fault */);
> > > > +               return __bpf_get_stack(regs, NULL, buf, size, flags, false /* !may_fault */);
> > > >
> > > >         if (unlikely(flags & ~(BPF_F_SKIP_FIELD_MASK | BPF_F_USER_STACK |
> > > >                                BPF_F_USER_BUILD_ID)))
> > > > @@ -966,7 +989,7 @@ BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
> > > >
> > > >         if (kernel) {
> > > >                 trace->nr = nr_kernel;
> > >
> > > this whole count_kernel_ip() logic above, why do we have it? I don't
> > > think we can have combined user and kernel stack trace, so how can we
> > > end up with PERF_CONTEXT_USER "ip" at all? I might be missing
> >
> > the callchain comes from perf event's callchain and that can
> > have both kernel and user part
> 
> in general, but bpf_get_stack and bpf_get_stackid never request both
> kernel and user stack trace in one go and we set add_mark to false, so
> we can't have PERF_CONTEXT_USER, which is why I am asking what's the
> deal with this nr_kernel calculation.

it's the bpf_get_stack_pe helper for perf event and we store the
callchain before we run bpf program in bpf_overflow_handler:

  perf_prepare_sample(data, event, regs); 
   -> perf_sample_save_callchain
      -> perf_callchain
         -> get_perf_callchain(... add_mark=true ..)

  ret = bpf_prog_run(prog, &ctx);
   -> bpf_get_stack_pe
      -> trace = ctx->data->callchain

the perf event setup is independent of bpf_get_stack execution,
so you can have PERF_CONTEXT_USER in the callchain

jirka


> 
> If it's unnecessary, we should just drop that part and not have that
> extra unnecessary complication
> 
> >
> > > something subtle here, but can you please check again if this whole
> > > trace->nr calculation/override is even necessary.
> >
> > as I wrote in the other reply I don't think we need to change
> > trace->nr directly, just pass the needed nr/count as argument
> >
> > jirka
> >
> > >
> > > > -               err = __bpf_get_stack(regs, NULL, trace, buf, size, flags, false /* !may_fault */);
> > > > +               err = __bpf_get_stack_pe(trace, buf, size, flags);
> > > >
> > > >         } else { /* user */
> > > >                 u64 skip = flags & BPF_F_SKIP_FIELD_MASK;
> > > > @@ -974,9 +997,8 @@ BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
> > > >                 skip += nr_kernel;
> > > >                 if (skip > BPF_F_SKIP_FIELD_MASK)
> > > >                         goto clear;
> > > > -
> > > >                 flags = (flags & ~BPF_F_SKIP_FIELD_MASK) | skip;
> > > > -               err = __bpf_get_stack(regs, NULL, trace, buf, size, flags, false /* !may_fault */);
> > > > +               err = __bpf_get_stack_pe(trace, buf, size, flags);
> > > >         }
> > > >
> > > >         /* restore nr */
> > > > --
> > > > 2.54.0
> > > >