Re: [PATCH v2 bpf-next 14/20] libbpf: Recognize __arena global varaibles.

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Thu, Feb 15, 2024 at 6:45 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Feb 15, 2024 at 3:22 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> >  {
> > @@ -2835,6 +2819,33 @@ static int
> > bpf_object__init_user_btf_maps(struct bpf_object *obj, bool strict,
> >              return err;
> >      }
> >
> > +    for (i = 0; i < obj->nr_maps; i++) {
> > +        struct bpf_map *map = &obj->maps[i];
> > +
> > +        if (map->def.type != BPF_MAP_TYPE_ARENA)
> > +            continue;
> > +
> > +        if (obj->arena_map) {
> > +            pr_warn("map '%s': only single ARENA map is supported
> > (map '%s' is also ARENA)\n",
> > +                map->name, obj->arena_map->name);
> > +            return -EINVAL;
> > +        }
> > +        obj->arena_map = map;
> > +
> > +        if (obj->efile.arena_data) {
> > +            err = init_arena_map_data(obj, map, ARENA_SEC,
> > obj->efile.arena_data_shndx,
> > +                          obj->efile.arena_data->d_buf,
> > +                          obj->efile.arena_data->d_size);
> > +            if (err)
> > +                return err;
> > +        }
> > +    }
> > +    if (obj->efile.arena_data && !obj->arena_map) {
> > +        pr_warn("elf: sec '%s': to use global __arena variables the
> > ARENA map should be explicitly declared in SEC(\".maps\")\n",
> > +            ARENA_SEC);
> > +        return -ENOENT;
> > +    }
> > +
> >      return 0;
> >  }
> >
> > @@ -3699,9 +3710,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> >                  obj->efile.st_ops_link_data = data;
> >                  obj->efile.st_ops_link_shndx = idx;
> >              } else if (strcmp(name, ARENA_SEC) == 0) {
> > -                sec_desc->sec_type = SEC_ARENA;
> > -                sec_desc->shdr = sh;
> > -                sec_desc->data = data;
> > +                obj->efile.arena_data = data;
> > +                obj->efile.arena_data_shndx = idx;
>
> I see. So these two are sort-of main tricks.
> Special case ARENA_SEC like ".maps" and then look for this
> obj level map in the right spots.

yep

> The special case around bpf_map__[set_]initial_value kind break
> the layering with:
> if (map->def.type == BPF_MAP_TYPE_ARENA)
>   actual_sz = map->obj->arena_data_sz;
> but no big deal.
>

true, and struct_ops will be another special case, so we might want to
think about generalizing that a bit, but that's a separate thing we
can handle later on

> How do you want me to squash the patches?
> Keep "rename is_internal_mmapable_map into is_mmapable_map" patch as-is

yep

> and then squash mine and your 2nd and 3rd?

I think `libbpf: move post-creation steps for ARENA map` should be
squashed into your `libbpf: Add support for bpf_arena.` which
introduces ARENA map by itself. And then `libbpf: Recognize __arena
global varaibles.` and `libbpf: remove fake __arena_internal map` can
be squashed together as well.