On Thu, Sep 24, 2020 at 8:21 AM John Fastabend <john.fastabend@xxxxxxxxx> wrote:
>
> Andrii Nakryiko wrote:
> > Refactor internals of struct btf to remove assumptions that BTF header, type
> > data, and string data are layed out contiguously in a memory in a single
> > memory allocation. Now we have three separate pointers pointing to the start
> > of each respective are: header, types, strings. In the next patches, these
> > pointers will be re-assigned to point to independently allocated memory areas,
> > if BTF needs to be modified.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@xxxxxx>
> > ---
>
> [...]
>
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -27,18 +27,37 @@
> > static struct btf_type btf_void;
> >
> > struct btf {
> > - union {
> > - struct btf_header *hdr;
> > - void *data;
> > - };
> > + void *raw_data;
> > + __u32 raw_size;
> > +
> > + /*
> > + * When BTF is loaded from ELF or raw memory it is stored
> > + * in contiguous memory block, pointed to by raw_data pointer, and
> > + * hdr, types_data, and strs_data point inside that memory region to
> > + * respective parts of BTF representation:
>
> I find the above comment a bit confusing. The picture though is great. How
> about something like,
>
> When BTF is loaded from an ELF or raw memory it is stored
> in a continguous memory block. The hdr, type_data, and strs_data
> point inside that memory region to their respective parts of BTF
> representation
>
Had to do a mental diff to find the part you didn't like :) I'll
update the comment as you suggested.
> > + *
> > + * +--------------------------------+
> > + * | Header | Types | Strings |
> > + * +--------------------------------+
> > + * ^ ^ ^
> > + * | | |
> > + * hdr | |
> > + * types_data-+ |
> > + * strs_data------------+
> > + */
> > + struct btf_header *hdr;
> > + void *types_data;
> > + void *strs_data;
> > +
> > + /* type ID to `struct btf_type *` lookup index */
> > __u32 *type_offs;
> > __u32 type_offs_cap;
> > - const char *strings;
> > - void *nohdr_data;
> > - void *types_data;
> > __u32 nr_types;
> > - __u32 data_size;
> > +
> > + /* BTF object FD, if loaded into kernel */
> > int fd;
> > +
> > + /* Pointer size (in bytes) for a target architecture of this BTF */
> > int ptr_sz;
> > };
> >
>
> Thanks,
> John
