On Wed, Jul 10, 2024 at 2:46 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Tue, 2024-07-09 at 16:42 -0700, Andrii Nakryiko wrote:
>
> [...]
>
> > > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> > > index 2b54e25d2364..735ae0901b3d 100644
> > > --- a/include/linux/bpf_verifier.h
> > > +++ b/include/linux/bpf_verifier.h
> > > @@ -585,6 +585,15 @@ struct bpf_insn_aux_data {
> > > * accepts callback function as a parameter.
> > > */
> > > bool calls_callback;
> > > + /* true if STX or LDX instruction is a part of a spill/fill
> > > + * pattern for a no_caller_saved_registers call.
> > > + */
> > > + u8 nocsr_pattern:1;
> > > + /* for CALL instructions, a number of spill/fill pairs in the
> > > + * no_caller_saved_registers pattern.
> > > + */
> > > + u8 nocsr_spills_num:3;
> >
> > despite bitfields this will extend bpf_insn_aux_data by 8 bytes. there
> > are 2 bytes of padding after alu_state, let's put this there.
> >
> > And let's not add bitfields unless absolutely necessary (this can be
> > always done later).
>
> Unfortunately the bitfields are still necessary, here is pahole output
> after moving fields and removing bitfields:
yep, if it's needed it's needed. My slightly older kernel has
alu_state at offset 61, so seems like we already added something
meanwhile.
>
> struct bpf_insn_aux_data {
> ...
> u8 alu_state; /* 62 1 */
> u8 nocsr_pattern; /* 63 1 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> u8 nocsr_spills_num; /* 64 1 */
>
> /* XXX 3 bytes hole, try to pack */
>
> unsigned int orig_idx; /* 68 4 */
> ...
>
> /* size: 80, cachelines: 2, members: 20 */
> /* sum members: 73, holes: 1, sum holes: 3 */
> /* padding: 4 */
> /* last cacheline: 16 bytes */
> };
>
> While with bitfields:
>
> /* size: 72, cachelines: 2, members: 20 */
>
> [...]
