MIME-Version: 1.0
References: <20230120192523.3650503-1-void@manifault.com> <20230120192523.3650503-5-void@manifault.com>
 <20230125043602.gmpi54ixerelmzzx@iphone-mikan.dhcp.thefacebook.com> <Y9C/7fxsIPXiG6mr@maniforge>
In-Reply-To: <Y9C/7fxsIPXiG6mr@maniforge>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Tue, 24 Jan 2023 21:43:08 -0800
Message-ID: <CAADnVQLi4ErtgFYvky9fh8Yecx_j+NSKBbC5NdfYOEowBAjr8A@mail.gmail.com>
Subject: Re: [PATCH bpf-next v2 4/9] bpf: Enable cpumasks to be queried and
 used as kptrs
To:     David Vernet <void@manifault.com>
Cc:     bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@meta.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Kernel Team <kernel-team@meta.com>, Tejun Heo <tj@kernel.org>,
        Kumar Kartikeya Dwivedi <memxor@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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.