Re: [RFC bpf-next v2 2/9] bpf: no_caller_saved_registers attribute for helper calls

[Eduard Zingerman <eddyz87@gmail.com>]

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:

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 */

[...]