On Fri, 2024-11-22 at 00:12 +0100, Kumar Kartikeya Dwivedi wrote: > On Fri, 22 Nov 2024 at 00:08, Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > > > 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. > > In the kernel or userspace, yes, but I'm assuming unsigned long will > always be 64-bit for target=BPF. > Would that be incorrect? This pattern will only happen within BPF programs. Discussed off-list. Kumar is right, and there is no problem, as on BPF side 'unsigned long' is always 8 bytes. [...]
