> diff --git a/tools/lib/bpf/bpf_gen_internal.h
> b/tools/lib/bpf/bpf_gen_internal.h
> index 0425691877521..6c5ad6c55e8a6 100644
> --- a/tools/lib/bpf/bpf_gen_internal.h
> +++ b/tools/lib/bpf/bpf_gen_internal.h
[ ... ]
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 514e4e9daa825..e574870fb7169 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
[ ... ]
> @@ -1839,6 +1842,8 @@ static size_t bpf_map_mmap_sz(const struct bpf_map *map)
> switch (map->def.type) {
> case BPF_MAP_TYPE_ARRAY:
> return array_map_mmap_sz(map->def.value_size,
> map->def.max_entries);
> + case BPF_MAP_TYPE_PERCPU_ARRAY:
> + return map->def.value_size;
> case BPF_MAP_TYPE_ARENA:
> return page_sz * map->def.max_entries;
This isn't a bug, but bpf_map_mmap_sz() is documented as returning the
length to pass to mmap()/munmap()/mprotect(), and both other arms return
page-granular lengths (array_map_mmap_sz() rounds up to page_sz, ARENA
multiplies by page_sz).
The new PERCPU_ARRAY arm returns the raw, unrounded value_size. This
creates an asymmetry visible in bpf_map__set_value_size(), which compares
mmap_old_sz = bpf_map_mmap_sz(map) (unrounded for percpu) against
mmap_new_sz = array_map_mmap_sz(size, max_entries) (page-rounded), so even
a no-op resize performs a redundant mmap() + memcpy() + munmap() cycle.
Would using array_map_mmap_sz(map->def.value_size, map->def.max_entries)
for the PERCPU_ARRAY case be more consistent with the ARRAY case?
[ ... ]
> @@ -1866,7 +1871,8 @@ static int bpf_map_mmap_resize(struct bpf_map *map,
> size_t old_sz, size_t new_sz
> return 0;
> }
>
> -static char *internal_map_name(struct bpf_object *obj, const char *real_name)
> +static char *internal_map_name(struct bpf_object *obj, const char *real_name,
> + enum libbpf_map_type type)
> {
> char map_name[BPF_OBJ_NAME_LEN], *p;
> int pfx_len, sfx_len = max((size_t)7, strlen(real_name));
> @@ -1907,8 +1913,11 @@ static char *internal_map_name(struct bpf_object *obj,
> const char *real_name)
> if (sfx_len >= BPF_OBJ_NAME_LEN)
> sfx_len = BPF_OBJ_NAME_LEN - 1;
>
> - /* if there are two or more dots in map name, it's a custom dot map */
> - if (strchr(real_name + 1, '.') != NULL)
> + /*
> + * Don't prefix the bpf_object name if this is a custom dot map
> + * (containing two or more dots) or a percpu data map.
> + */
> + if (strchr(real_name + 1, '.') != NULL || type == LIBBPF_MAP_PERCPU)
> pfx_len = 0;
> else
> pfx_len = min((size_t)BPF_OBJ_NAME_LEN - sfx_len - 1,
> strlen(obj->name));
Emil Tsalapatis raised a design question in v10: Why does .percpu get
special treatment in internal_map_name() instead of following the same
prefixing logic as .data and .rodata?
He noted "it seems like we're singling out the .percpu section for highly
nonbvious reasons" and asked "@Andrii Wdyt?" You responded that you prefer
excluding PERCPU "to avoid the legacy naming convention for new internal
maps" but said "Let us wait for Andrii's comment."
Andrii didn't publicly respond to this design question in the lore threads,
and v11/v12 proceeded with the same special treatment. The related
map_uses_real_name() function also treats PERCPU differently (always
returns true for PERCPU, while DATA/RODATA check against their base section
names).
Should this naming convention uniformity concern be addressed, or is there
a technical reason for .percpu's special handling that could be documented
in the comment?
[ ... ]
> @@ -1941,6 +1950,13 @@ static bool map_is_mmapable(struct bpf_object *obj,
> struct bpf_map *map)
> if (!map->btf_value_type_id)
> return false;
>
> + /*
> + * The internal PERCPU maps are not mmapble because the underlying
> + * percpu_array maps do not have mmap support.
> + */
> + if (map->libbpf_type == LIBBPF_MAP_PERCPU)
> + return false;
> +
This isn't a bug, but there's a typo: "mmapble" should be "mmappable".
[ ... ]
> @@ -5293,18 +5326,20 @@ 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 (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",
[ ... ]
> @@ -5349,6 +5384,13 @@ bpf_object__populate_internal_map(struct bpf_object
> *obj, struct bpf_map *map)
> return err;
> }
> map->mmaped = mmaped;
> + } else if (is_percpu) {
> + if (mprotect(map->mmaped, mmap_sz, PROT_READ)) {
> + err = -errno;
> + pr_warn("map '%s': failed to mprotect() contents: %s\n",
> + bpf_map__name(map), errstr(err));
> + return err;
> + }
> + } else if (map->mmaped) {
> munmap(map->mmaped, mmap_sz);
> map->mmaped = NULL;
This isn't a bug, but unlike every other internal map, the percpu
initialization image is deliberately kept mapped and turned read-only with
mprotect() instead of being munmap()ed.
The skeleton pointer that bpftool generates for this map is a plain,
non-const 'struct <obj>__percpu *', so a perfectly ordinary post-load
assignment such as 'skel->percpu->var = 1;' now faults with SIGSEGV in the
user process rather than failing in a diagnosable way.
The behaviour is intentional and described in the commit message, but would
a const-qualified generated pointer (as is done for .rodata) express the
restriction at compile time instead of at runtime?
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31716447858
