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

On Mon, Aug 3, 2026 at 12:37=E2=80=AFAM Jiri Olsa <[email protected]> wrot=
e:
>
> On Fri, Jul 31, 2026 at 08:14:03AM -0700, Andrii Nakryiko wrote:
> > On Fri, Jul 31, 2026 at 6:33=E2=80=AFAM Jiri Olsa <[email protected]> =
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 =3D -EINVAL;
> > > > >
> > > > >         if (!(event->attr.sample_type & PERF_SAMPLE_CALLCHAIN))
> > > > > -               return __bpf_get_stack(regs, NULL, NULL, buf, siz=
e, flags, false /* !may_fault */);
> > > > > +               return __bpf_get_stack(regs, NULL, buf, size, fla=
gs, 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 =3D 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=3Dtrue ..)
>
>   ret =3D bpf_prog_run(prog, &ctx);
>    -> bpf_get_stack_pe
>       -> trace =3D ctx->data->callchain
>
> the perf event setup is independent of bpf_get_stack execution,
> so you can have PERF_CONTEXT_USER in the callchain
>

oh, I didn't realize that's what happens, thanks, never mind then!

> 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 =3D __bpf_get_stack(regs, NULL, trace, buf, s=
ize, flags, false /* !may_fault */);
> > > > > +               err =3D __bpf_get_stack_pe(trace, buf, size, flag=
s);
> > > > >
> > > > >         } else { /* user */
> > > > >                 u64 skip =3D 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 +=3D nr_kernel;
> > > > >                 if (skip > BPF_F_SKIP_FIELD_MASK)
> > > > >                         goto clear;
> > > > > -
> > > > >                 flags =3D (flags & ~BPF_F_SKIP_FIELD_MASK) | skip=
;
> > > > > -               err =3D __bpf_get_stack(regs, NULL, trace, buf, s=
ize, flags, false /* !may_fault */);
> > > > > +               err =3D __bpf_get_stack_pe(trace, buf, size, flag=
s);
> > > > >         }
> > > > >
> > > > >         /* restore nr */
> > > > > --
> > > > > 2.54.0
> > > > >