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

On Thu, Aug 13, 2026 at 9:26 AM <[email protected]> wrote:
>
> > 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,

ok, wow, being shamed by AI... I did respond on v10, and yes, I think
passing type and basing decisions on that is better going forward, no
need for name-based checks.

> 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

Is it done for .rodata, really?... As far as I can see, .rodata are
still non-const because they are expected to be set before load. Eh,
AI...

> 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