On 14/8/26 00:26, [email protected] wrote:
>> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
>> index a01d06d22d1a3..a50540ef6521c 100644
>> --- a/tools/bpf/bpftool/gen.c
>> +++ b/tools/bpf/bpftool/gen.c
>
> [ ... ]
>
>> @@ -254,7 +260,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 is_skel_data(const struct bpf_map *map, char *buf, size_t sz)
>> {
>> size_t tmp_sz;
>>
>> @@ -263,13 +269,24 @@ 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_internal(map))
>> return false;
>>
>> if (!get_map_ident(map, buf, sz))
>> return false;
>>
>> - return true;
>> + if (bpf_map__map_flags(map) & BPF_F_MMAPABLE)
>> + return true;
>> +
>> + if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY)
>> + return bpf_map__btf_value_type_id(map) != 0;
>> +
>> + return false;
>> +}
>> +
>> +static bool is_mmapable_map(const struct bpf_map *map, char *buf, size_t sz)
>> +{
>> + return is_skel_data(map, buf, sz) && bpf_map__type(map) !=
>> BPF_MAP_TYPE_PERCPU_ARRAY;
>> }
>>
>> static int codegen_datasecs(struct bpf_object *obj, const char *obj_name)
>> @@ -287,7 +304,7 @@ static int codegen_datasecs(struct bpf_object *obj,
>> const char *obj_name)
>>
>> bpf_object__for_each_map(map, obj) {
>> /* only generate definitions for memory-mapped internal maps */
>> - if (!is_mmapable_map(map, map_ident, sizeof(map_ident)))
>> + if (!is_skel_data(map, map_ident, sizeof(map_ident)))
>> continue;
>
> The comment here says "only generate definitions for memory-mapped
> internal maps," but is_skel_data() now also returns true for
> BPF_MAP_TYPE_PERCPU_ARRAY maps. Those percpu maps are never
> memory-mapped: map_is_mmapable() in tools/lib/bpf/libbpf.c returns false
> for LIBBPF_MAP_PERCPU, which is why this patch had to add mprotect() for
> the init buffer rather than relying on shared mmap.
>
> When you removed the identical comment from codegen_maps_skeleton()
> (line 988), should this one have been updated to say "skeleton data maps"
> or "internal maps exposed as skeleton data" to match what is_skel_data()
> actually checks?
>
> The same wording also appears at line 534 in codegen_asserts():
Will update the comments.
>
>> bpf_object__for_each_map(map, obj) {
>> - if (!is_mmapable_map(map, map_ident, sizeof(map_ident)))
>> + if (!is_skel_data(map, map_ident, sizeof(map_ident)))
>> continue;
>
> where the comment above (line 513) says "Emit type size asserts for all
> top-level fields in memory-mapped internal maps." That comment now covers
> percpu maps too.
Will update the comment to "Emit type size asserts for all top-level
fields in skel data maps.".
Thanks,
Leon
>
