Re: [PATCH bpf-next v11 05/10] libbpf: Add support for global percpu data

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Dead code in percpu global data feature check
--

--- Patch [5]: [PATCH bpf-next v11 05/10] libbpf: Add support for global percpu data ---
commit ee97f8a8e25b5e9337024df3dd6f3a1aae4f6cdc
Author: Leon Hwang <leon.hwang@linux.dev>

libbpf: Add support for global percpu data

This commit adds support for global percpu data in libbpf by introducing a 
new ".percpu" section. It updates the lightweight skeleton loader to use 
BPF_F_ALL_CPUS when updating the percpu_array map and marks the mmaped data 
as read-only after population.

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 514e4e9daa825..9f79a8e850fdb 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c

[ ... ]

> @@ -5293,18 +5326,26 @@ static int
>  bpf_object__populate_internal_map(struct bpf_object *obj, struct bpf_map *map)
>  {
>  	enum libbpf_map_type map_type = map->libbpf_type;
> +	bool is_percpu = map_type == LIBBPF_MAP_PERCPU;
> +	const __u64 update_flags = is_percpu ? BPF_F_ALL_CPUS : 0;
>  	int err, zero = 0;
>  	size_t mmap_sz;
>  
> +	if (is_percpu && !obj->gen_loader && !kernel_supports(obj, FEAT_PERCPU_DATA)) {
> +		pr_warn("map '%s': kernel does not support percpu data.\n",
> +			bpf_map__name(map));
> +		return -EOPNOTSUPP;
> +	}
> +

[Severity: Low]
Is this check reachable? 

Looking at bpf_object__create_maps(), if the kernel does not support
FEAT_PERCPU_DATA, map->autocreate is set to false and the map processing
loop continues early:

bpf_object__create_maps() {
    ...
    if (bpf_map__is_internal(map) && bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY &&
        !kernel_supports(obj, FEAT_PERCPU_DATA))
        map->autocreate = false;

    if (!map->autocreate) {
        pr_debug("map '%s': skipped auto-creating...\n", map->name);
        continue;
    }
    ...
}

Since bpf_object__populate_internal_map() is called later in this same
loop (after the continue), wouldn't the !kernel_supports() condition
already be filtered out, making this warning dead code?

>  	if (obj->gen_loader) {
>  		bpf_gen__map_update_elem(obj->gen_loader, map - obj->maps,
> -					 map->mmaped, map->def.value_size);
> +					 map->mmaped, map->def.value_size, update_flags);
>  		if (map_type == LIBBPF_MAP_RODATA || map_type == LIBBPF_MAP_KCONFIG)
>  			bpf_gen__map_freeze(obj->gen_loader, map - obj->maps);
>  		return 0;
>  	}
>  
> -	err = bpf_map_update_elem(map->fd, &zero, map->mmaped, 0);
> +	err = bpf_map_update_elem(map->fd, &zero, map->mmaped, update_flags);
>  	if (err) {
>  		err = -errno;
>  		pr_warn("map '%s': failed to set initial contents: %s\n",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806163125.11172-1-leon.hwang@linux.dev?part=5