On Thu, Feb 15, 2024 at 6:45 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Thu, Feb 15, 2024 at 3:22 PM Andrii Nakryiko
> <andrii.nakryiko@xxxxxxxxx> 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.
