[DTrace-devel] BPF_F_CURRENT_CPU

[Kris Van Hees]

On Wed, May 13, 2020 at 07:24:16AM -0700, Eugene Loh wrote:
> On 05/13/2020 12:01 AM, Kris Van Hees wrote:
> 
> > On Wed, May 13, 2020 at 01:40:19AM -0400, Kris Van Hees wrote:
> >> Cc: to the dtrace-devel list because it is an important discussion.
> >>
> >> On Tue, May 12, 2020 at 06:53:31PM -0700, Eugene Loh wrote:
> >>> In our BPF function prologue, we call a BPF helper function to get the
> >>> current CPU, then convert to 32-bit, then store to the stack.  In the
> >>> epilogue, we retrieve that value to pass to bpf_perf_event_output().
> >>>
> >>> Wouldn't it be simpler just to pass BPF_F_CURRENT_CPU to
> >>> bpf_perf_event_output()?  Or will we want the current CPU on the stack
> >>> anyhow for other reasons (e.g., built-in variable)?
> >> Yes, you are right.  When I wrote that code originally, I had more uses for
> >> the current cpu index (e.g. obtaining scratch memory to construct the output
> >> record) and indeed inlined resolving of built-in variables.  And since I had
> >> it anyway I never even looked to see if there was a mode for
> >> bpf_perf_event_output() to output to the buffer for the current cpu without
> >> needing to explicitly specify it.
> >>
> >> Since you found this convenient way to not needing to know the cpu index and
> >> since we actually do not have any other need for it anymore (the builtin
> >> variable need to be handled different anyway because it provides a struct of
> >> data rather than just the cpu index), it would be a good idea to rework that
> >> code to no longer retrieve and store the cpu index.  That also frees up a
> >> little bit of space in the dt_bpf_context.
> > Correcting myself here...  the cpu index is not stored in dt_bpf_context,
> > but is (as you mention) stored on the stack.  It is part of the dt_program
> > stack frame layout, so it can get dropped from there.
> >
> >> Do you want to tackle that reworking?
> Sure.  I'm happy to.

Actually, I had to make some other changes in this code (well, more like a
pretty significant rewrite) so I merged this change into it already.  See my
upcoming patch concerning using builtin variables in a predicate yielding the
program getting rejected by the BPF verifier.

	Kris