On Fri, Jan 20, 2023 at 01:25:18PM -0600, David Vernet wrote:
> +
> +/**
> + * struct bpf_cpumask - refcounted BPF cpumask wrapper structure
> + * @cpumask: The actual cpumask embedded in the struct.
> + * @usage: Object reference counter. When the refcount goes to 0, the
> + * memory is released back to the BPF allocator, which provides
> + * RCU safety.
> + *
> + * Note that we explicitly embed a cpumask_t rather than a cpumask_var_t. This
> + * is done to avoid confusing the verifier due to the typedef of cpumask_var_t
> + * changing depending on whether CONFIG_CPUMASK_OFFSTACK is defined or not. See
> + * the details in <linux/cpumask.h>. The consequence is that this structure is
> + * likely a bit larger than it needs to be when CONFIG_CPUMASK_OFFSTACK is
> + * defined due to embedding the whole NR_CPUS-size bitmap, but the extra memory
> + * overhead is minimal. For the more typical case of CONFIG_CPUMASK_OFFSTACK
> + * not being defined, the structure is the same size regardless.
> + */
> +struct bpf_cpumask {
> + cpumask_t cpumask;
> + refcount_t usage;
> +};
> +
> +static struct bpf_mem_alloc bpf_cpumask_ma;
> +
> +static bool cpu_valid(u32 cpu)
> +{
> + return cpu < nr_cpu_ids;
> +}
> +
> +__diag_push();
> +__diag_ignore_all("-Wmissing-prototypes",
> + "Global kfuncs as their definitions will be in BTF");
> +
> +struct bpf_cpumask *bpf_cpumask_create(void)
> +{
> + struct bpf_cpumask *cpumask;
> +
> + cpumask = bpf_mem_alloc(&bpf_cpumask_ma, sizeof(*cpumask));
> + if (!cpumask)
> + return NULL;
> +
> + memset(cpumask, 0, sizeof(*cpumask));
> + refcount_set(&cpumask->usage, 1);
> +
> + return cpumask;
> +}
Applied patches 1 and 2. Patch 3 doesn't apply anymore. Pls rebase.
I'm fine with existing bpf_cpumask proposal, but can we do better?
This is so close to be a bitmap template.
Can we generalize it as
struct bpf_bitmap {
refcount_t refcnt;
int num_bits;
u64 bits[];
};
struct bpf_bitmap *bpf_bitmap_create(int bits)
{
bitmap = bpf_mem_alloc(&bpf_cpumask_ma, sizeof(*bitmap) + BITS_TO_LONGS(bits) * sizeof(u64));
bitmap->num_bits = bits;
}
and special case few custom kfuncs in the verifier that allow
type cast from bpf_bitmap with to 'struct cpumask *' ? Like
struct cpumask *bpf_bitmap_cast_to_cpumask(struct bpf_bitmap *bitmap)
{
if (bitmap->num_bits == nr_cpu_ids)
return bitmap->bits;
return NULL;
}
BTF_ID_FLAGS(func, bpf_bitmap_cast_to_cpumask, KF_TRUSTED_ARGS | KF_RET_NULL)
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.
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.
wdyt?
