On Thu, 2024-11-21 at 23:06 +0100, Kumar Kartikeya Dwivedi wrote:
[...]
> > > +/* Keep unsinged long in prototype so that kfunc is usable when emitted to
> > > + * vmlinux.h in BPF programs directly, but since unsigned long may potentially
> > > + * be 4 byte, always cast to u64 when reading/writing from this pointer as it
> > > + * always points to an 8-byte memory region in BPF stack.
> > > + */
> > > +__bpf_kfunc void bpf_local_irq_save(unsigned long *flags__irq_flag)
> >
> > Nit: 'unsigned long long' is guaranteed to be at-least 64 bit.
> > What would go wrong if 'u64' is used here?
>
> It goes like this:
> If I make this unsigned long long * or u64 *, the kfunc emitted to
> vmlinux.h expects a pointer of that type.
> Typically, kernel code is always passing unsigned long flags to these
> functions, and that's what people are used to.
> Given for --target=bpf unsigned long * is always a 8-byte value, I
> just did this, so that in kernels that are 32-bit,
> we don't end up relying on unsigned long still being 8 when
> fetching/storing flags on BPF stack.
So, the goal is to enable the following pattern:
unsigned long flags;
bpf_local_irq_save(&flags);
Right?
For a 32-bit system 'flags' would be 4 bytes long.
Consider the following example:
unsigned long flags; // assume 'flags' and 'foo'
int foo; // are allocated sequentially.
bpf_local_irq_save(&flags);
I think that in such case '*ptr = flags;' would overwrite foo.
[...]
> > > +{
> > > + u64 *ptr = (u64 *)flags__irq_flag;
> > > + unsigned long flags;
> > > +
> > > + local_irq_save(flags);
> > > + *ptr = flags;
> > > +}
