Re: [PATCH bpf-next v2 4/9] bpf: Enable cpumasks to be queried and used as kptrs

Alexei Starovoitov
2023-01-25T05:43:26Z

On Tue, Jan 24, 2023 at 9:36 PM David Vernet <void@manifault.com> wrote:
>
> > The UX will be a bit worse, since bpf prog would need to do !=NULL check
> > but with future bpf_assert() we may get rid of !=NULL check.
> >
> > We can keep direct cpumask accessors as kfuncs:
> >
> > u32 bpf_cpumask_first(const struct cpumask *cpumask);
> > u32 bpf_cpumask_first_zero(const struct cpumask *cpumask);
> >
> > and add bpf_find_first_bit() and the rest of bit manipulations.
>
> Worth noting as well is that I think struct bpf_bitmap is going to be
> treated somewhat differently than struct bpf_cpumask and struct cpumask.
> There is no type-safety for bitmaps in the kernel. They're just
> represented as unsigned long *, so I don't we'll be able to allow
> programs to pass bitmaps allocated elsewhere in the kernel to read-only
> bitmap kfuncs like we do for struct cpumask *, as the verifier will just
> interpret them as pointers to statically sized scalars.

That's a good point. That's where run-time and verification-time
safety hurts UX too much.

> > Since all of the bpf_cpumask do run-time cpu_valid() check we're not
> > sacrificing performance.
> >
> > Feels more generic with wider applicability at the expense of little bit worse UX.
> > I haven't thought about acq/rel consequences.
>
> The TL;DR from me is that I agree that having bitmap kfuncs is a great
> idea, but I don't see the need to tie the two at the hip at the cost of
> a worse UX. I'd prefer to push the extra complexity into the BPF backend
> in favor of a simpler programming front-end for users.
>
> Thoughts?

Fair enough. Let's proceed with what you have.