Re: [PATCH bpf-next v5 6/9] bpftool: Implement relocations recording for BTFGen

On Wed, Feb 2, 2022 at 5:55 PM Andrii Nakryiko
[off-list ref] wrote:

On Fri, Jan 28, 2022 at 2:33 PM Mauricio Vásquez [off-list ref] wrote:

quoted

This commit implements the logic to record the relocation information
for the different kind of relocations.

btfgen_record_field_relo() uses the target specification to save all the
types that are involved in a field-based CO-RE relocation. In this case
types resolved and added recursively (using btfgen_put_type()).
Only the struct and union members and their types) involved in the
relocation are added to optimize the size of the generated BTF file.

On the other hand, btfgen_record_type_relo() saves the types involved in
a type-based CO-RE relocation. In this case all the members for the
struct and union types are added. This is not strictly required since
libbpf doesn't use them while performing this kind of relocation,
however that logic could change on the future. Additionally, we expect
that the number of this kind of relocations in an BPF object to be very
low, hence the impact on the size of the generated BTF should be
negligible.

Finally, btfgen_record_enumval_relo() saves the whole enum type for
enum-based relocations.

Signed-off-by: Mauricio Vásquez <redacted>
Signed-off-by: Rafael David Tinoco <redacted>
Signed-off-by: Lorenzo Fontana <redacted>
Signed-off-by: Leonardo Di Donato <redacted>
---

I've been thinking about this in background. This proliferation of
hashmaps to store used types and their members really adds to
complexity (and no doubt to memory usage and CPU utilization, even
though I don't think either is too big for this use case).

What if instead of keeping track of used types and members separately,
we initialize the original struct btf and its btf_type, btf_member,
btf_enum, etc types. We can carve out one bit in them to mark whether
that specific entity was used. That way you don't need any extra
hashmap maintenance. You just set or check bit on each type or its
member to figure out if it has to be in the resulting BTF.

This can be highest bit of name_off or type fields, depending on
specific case. This will work well because type IDs never use highest
bit and string offset can never be as high as to needing full 32 bits.

You'll probably want to have two copies of target BTF for this, of
course, but I think simplicity of bookkeeping trumps this
inefficiency. WDYT?

It's a very nice idea indeed. I got a version working with this idea.
I keep two instances of the target BTF (as you suggested) one is only
for keeping track of the used types/members, the other one is used as
source when copying the BTF types and also to run the candidate search
algorithm and so on. Actually there is no need to use the highest bit,
I'm just setting the whole name_off to UINT32_MAX. It works fine
because that copy of the BTF isn't used anywhere else. I'm cleaning
this up and hope to send it early next week.

Thanks for all the feedback!