Re: [PATCH bpf-next v8 5/9] bpftool: Generate skeleton for global percpu data

[Quentin Monnet <qmo@kernel.org>]

2026-07-02 14:24 UTC+0800 ~ Leon Hwang <leon.hwang@linux.dev>
> On 2/7/26 03:32, Andrii Nakryiko wrote:
>> On Wed, Jul 1, 2026 at 9:49 AM Quentin Monnet <qmo@kernel.org> wrote:
>>>
>>> 2026-06-29 23:24 UTC+0800 ~ Leon Hwang <leon.hwang@linux.dev>
> [...]
>>>> @@ -254,6 +254,20 @@ static const struct btf_type *find_type_for_map(struct btf *btf, const char *map
>>>>       return NULL;
>>>>  }
>>>>
>>>> +static bool bpf_map_is_skel_data(const struct bpf_map *map)
>>>> +{
>>>> +     if (!bpf_map__is_internal(map))
>>>> +             return false;
>>>> +
>>>> +     if (bpf_map__map_flags(map) & BPF_F_MMAPABLE)
>>>> +             return true;
>>>> +
>>>> +     if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY)
>>>> +             return true;
>>>> +
>>>> +     return false;
>>>> +}
>>>> +
>>>>  static bool is_mmapable_map(const struct bpf_map *map, char *buf, size_t sz)
>>>>  {
>>>>       size_t tmp_sz;
>>>> @@ -263,7 +277,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))
>>>
>>>
>>> Thanks! The bpftool patch looks good, with one reservation: after this
>>> patch, I believe "is_mmapable_map(map, ...)" will return true if map is
>>> a percpu map, although percpu maps aren't mmap-able, so we should
>>> probably update the name of that function to avoid any confusion?
>>>
>>
>> Great observation, Quentin!
>>
>> bpf_map_is_skel_data() I think was supposed to be exactly that generic
>> name. But it seems like Leon went half-way through with unification.
>> Unless there are some subtle situations where per-cpu array shouldn't
>> be handled where is_mmapable_map() is handled, we should rename
>> is_mmapable_map() and add BPF_MAP_TYPE_PERCPU_ARRAY check (assuming
>> it's internal map, of course) there.
>>
>> Leon, can you please check?
>>
> Aha, my bad.
> 
> Try to rename is_mmapable_map() to bpf_map_is_skel_data(). See below patch.
> 
> Thanks,
> Leon
> 
> ---
> 
> From 654306cd9091a175883dd61c7b251996c42e7cae Mon Sep 17 00:00:00 2001
> From: Leon Hwang <leon.hwang@linux.dev>
> Date: Mon, 29 Jun 2026 23:24:02 +0800
> Subject: [PATCH bpf-next v9 5/9] bpftool: Generate skeleton for global
> percpu
>  data
> 

[...]

> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index 6ae7262ebe0c..798a34366e08 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c

[...]

> @@ -254,7 +254,7 @@ static const struct btf_type
> *find_type_for_map(struct btf *btf, const char *map
>  	return NULL;
>  }
> 
> -static bool is_mmapable_map(const struct bpf_map *map, char *buf,
> size_t sz)
> +static bool bpf_map_is_skel_data(const struct bpf_map *map, char *buf,
> size_t sz)
>  {


Yes, I think it addresses the issue, thanks! I'd maybe drop the
"bpf_map_" prefix in the function name ("is_skel_data_map()" instead?)
to remain closer to is_mmapable_map(), and to avoid creating confusion
with libbpf functions names, although I don't feel strongly about it.
You can add my ACK to this v9 for the bpftool patch when you repost the
series.

Thanks,
Quentin