On 6/2/25 08:09, Andrii Nakryiko wrote:
> On Mon, Jan 27, 2025 at 8:22 AM Leon Hwang <leon.hwang@xxxxxxxxx> wrote:
>>
>> This patch introduces global percpu data, inspired by commit
>> 6316f78306c1 ("Merge branch 'support-global-data'"). It enables the
>> definition of global percpu variables in BPF, similar to the
>> DEFINE_PER_CPU() macro in the kernel[0].
>>
>> For example, in BPF, it is able to define a global percpu variable like
>> this:
>>
>> int percpu_data SEC(".percpu");
>>
>> With this patch, tools like retsnoop[1] and bpflbr[2] can simplify their
>> BPF code for handling LBRs. The code can be updated from
>>
>> static struct perf_branch_entry lbrs[1][MAX_LBR_ENTRIES] SEC(".data.lbrs");
>>
>> to
>>
>> static struct perf_branch_entry lbrs[MAX_LBR_ENTRIES] SEC(".percpu.lbrs");
>>
>> This eliminates the need to retrieve the CPU ID using the
>> bpf_get_smp_processor_id() helper.
>>
>> Additionally, by reusing global percpu data map, sharing information
>> between tail callers and callees or freplace callers and callees becomes
>> simpler compared to reusing percpu_array maps.
>>
>> Links:
>> [0] https://github.com/torvalds/linux/blob/fbfd64d25c7af3b8695201ebc85efe90be28c5a3/include/linux/percpu-defs.h#L114
>> [1] https://github.com/anakryiko/retsnoop
>> [2] https://github.com/Asphaltt/bpflbr
>>
>> Signed-off-by: Leon Hwang <leon.hwang@xxxxxxxxx>
>> ---
>> kernel/bpf/arraymap.c | 39 ++++++++++++++++++++++++++++++++++++-
>> kernel/bpf/verifier.c | 45 +++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 83 insertions(+), 1 deletion(-)
>>
>
> [...]
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 9971c03adfd5d..9d99497c2b94c 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -6810,6 +6810,8 @@ static int bpf_map_direct_read(struct bpf_map *map, int off, int size, u64 *val,
>> u64 addr;
>> int err;
>>
>> + if (map->map_type != BPF_MAP_TYPE_ARRAY)
>> + return -EINVAL;
>> err = map->ops->map_direct_value_addr(map, &addr, off);
>> if (err)
>> return err;
>> @@ -7322,6 +7324,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
>> /* if map is read-only, track its contents as scalars */
>> if (tnum_is_const(reg->var_off) &&
>> bpf_map_is_rdonly(map) &&
>> + map->map_type != BPF_MAP_TYPE_PERCPU_ARRAY &&
>
> shouldn't this rather be a safer `map->map_type == BPF_MAP_TYPE_ARRAY` check?
>
Ack. I will update it to `map->map_type == BPF_MAP_TYPE_ARRAY`.
Thanks,
Leon
>> map->ops->map_direct_value_addr) {
>> int map_off = off + reg->var_off.value;
>> u64 val = 0;
>
> [...]
