On Tue, Jun 23, 2026 at 9:15 PM Leon Hwang <[email protected]> wrote:
>
> On 24/6/26 06:46, Andrii Nakryiko wrote:
> > On Mon, Jun 22, 2026 at 7:37 AM Leon Hwang <[email protected]> wrote:
> [...]
> >> @@ -263,13 +268,12 @@ static bool is_mmapable_map(const struct bpf_map
> >> *map, char *buf, size_t sz)
> >> return true;
> >> }
> >>
> >> - if (!bpf_map__is_internal(map) || !(bpf_map__map_flags(map) &
> >> BPF_F_MMAPABLE))
> >> - return false;
> >> -
> >> - if (!get_map_ident(map, buf, sz))
> >> - return false;
> >> + if (bpf_map__is_internal(map) &&
> >> + ((bpf_map__map_flags(map) & BPF_F_MMAPABLE) ||
> >> bpf_map_is_percpu_data(map)) &&
> >> + get_map_ident(map, buf, sz))
> >> + return true;
> >>
> >
> > just add `if (bpf_map_is_percpu_data(map) return true;`? maybe also
> > move get_map_ident check a bit earlier. I think that will be a bit
> > cleaner, this condition is quite hard to follow
> >
>
> Makes sense.
>
> With adding the helper bpf_map_is_skel_data(), will this change looks
> more readable?
>
>
> +static bool bpf_map_is_skel_data(const struct bpf_map *map)
> +{
> + return bpf_map__is_internal(map) &&
> + ((bpf_map__map_flags(map) & BPF_F_MMAPABLE) ||
> + bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY);
if (!bpf_map__is_internal(map))
return false;
return (bpf_map__map_flags(map) & BPF_F_MMAPABLE) ||
bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY;
or just split that last return into two if (...) return true);
both work for me
> +}
> +
> static bool is_mmapable_map(const struct bpf_map *map, char *buf,
> size_t sz)
> {
> size_t tmp_sz;
> @@ -263,7 +274,7 @@ static bool is_mmapable_map(const struct bpf_map
> *map, char *buf, size_t sz)
> return true;
> }
>
> - if (!bpf_map__is_internal(map) || !(bpf_map__map_flags(map) &
> BPF_F_MMAPABLE))
> + if (!bpf_map_is_skel_data(map))
> return false;
>
> if (!get_map_ident(map, buf, sz))
>
>
> >> - return true;
> >> + return false;
> >> }
> >>
> >> static int codegen_datasecs(struct bpf_object *obj, const char *obj_name)
> >> @@ -343,6 +347,9 @@ static int codegen_subskel_datasecs(struct bpf_object
> >> *obj, const char *obj_name
> >> if (!is_mmapable_map(map, map_ident, sizeof(map_ident)))
> >> continue;
> >>
> >> + if (bpf_map_is_percpu_data(map))
> >> + continue;
> >> +
> >> sec = find_type_for_map(btf, map_ident);
> >> if (!sec)
> >> continue;
> >> @@ -669,7 +676,7 @@ static void codegen_destroy(struct bpf_object *obj,
> >> const char *obj_name)
> >> if (!get_map_ident(map, ident, sizeof(ident)))
> >> continue;
> >> if (bpf_map__is_internal(map) &&
> >> - (bpf_map__map_flags(map) & BPF_F_MMAPABLE))
> >> + ((bpf_map__map_flags(map) & BPF_F_MMAPABLE) ||
> >> bpf_map_is_percpu_data(map)))
> >
> > also we can add a helper to check if it's an internal map with a
> > dedicated data section in the skeleton (too lazy to think of a good
> > name right now.. ;)
> >
>
> How about bpf_map_is_skel_data()?
>
> Then, this 'if' will be updated as 'if (bpf_map_is_skel_data(map))'.
yeah, I like the name
>
> Thanks,
> Leon
>
> >> printf("\tskel_free_map_data(skel->%1$s,
> >> skel->maps.%1$s.initial_value, %2$zu);\n",
> >> ident, bpf_map_mmap_sz(map));
> >> codegen("\
> >
> > [...]
>
