On 5/18/23 6:23 AM, Alan Maguire wrote:
On 18/05/2023 09:39, Jiri Olsa wrote:
On Wed, May 17, 2023 at 05:16:47PM +0100, Alan Maguire wrote:
By making sorting function for our ELF function list match on
both name and function, we ensure that the set of ELF functions
includes multiple copies for functions which have multiple instances
across CUs. For example, cpumask_weight has 22 instances in
System.map/kallsyms:
ffffffff8103b530 t cpumask_weight
ffffffff8103e300 t cpumask_weight
ffffffff81040d30 t cpumask_weight
ffffffff8104fa00 t cpumask_weight
ffffffff81064300 t cpumask_weight
ffffffff81082ba0 t cpumask_weight
ffffffff81084f50 t cpumask_weight
ffffffff810a4ad0 t cpumask_weight
ffffffff810bb740 t cpumask_weight
ffffffff8110a6c0 t cpumask_weight
ffffffff81118ab0 t cpumask_weight
ffffffff81129b50 t cpumask_weight
ffffffff81137dc0 t cpumask_weight
ffffffff811aead0 t cpumask_weight
ffffffff811d6800 t cpumask_weight
ffffffff811e1370 t cpumask_weight
ffffffff812fae80 t cpumask_weight
ffffffff81375c50 t cpumask_weight
ffffffff81634b60 t cpumask_weight
ffffffff817ba540 t cpumask_weight
ffffffff819abf30 t cpumask_weight
ffffffff81a7cb60 t cpumask_weight
With ELF representations for each address, and DWARF info about
addresses (low_pc) we can match DWARF with ELF accurately.
The result for the BTF representation is that we end up with
a single de-duped function:
[9287] FUNC 'cpumask_weight' type_id=9286 linkage=static
...and 22 DECL_TAGs for each address that point at it:
9288] DECL_TAG 'address=0xffffffff8103b530' type_id=9287 component_idx=-1
[9623] DECL_TAG 'address=0xffffffff8103e300' type_id=9287 component_idx=-1
[9829] DECL_TAG 'address=0xffffffff81040d30' type_id=9287 component_idx=-1
[11609] DECL_TAG 'address=0xffffffff8104fa00' type_id=9287 component_idx=-1
[13299] DECL_TAG 'address=0xffffffff81064300' type_id=9287 component_idx=-1
[15704] DECL_TAG 'address=0xffffffff81082ba0' type_id=9287 component_idx=-1
[15731] DECL_TAG 'address=0xffffffff81084f50' type_id=9287 component_idx=-1
[18582] DECL_TAG 'address=0xffffffff810a4ad0' type_id=9287 component_idx=-1
[20234] DECL_TAG 'address=0xffffffff810bb740' type_id=9287 component_idx=-1
[25384] DECL_TAG 'address=0xffffffff8110a6c0' type_id=9287 component_idx=-1
[25798] DECL_TAG 'address=0xffffffff81118ab0' type_id=9287 component_idx=-1
[26285] DECL_TAG 'address=0xffffffff81129b50' type_id=9287 component_idx=-1
[27040] DECL_TAG 'address=0xffffffff81137dc0' type_id=9287 component_idx=-1
[32900] DECL_TAG 'address=0xffffffff811aead0' type_id=9287 component_idx=-1
[35059] DECL_TAG 'address=0xffffffff811d6800' type_id=9287 component_idx=-1
[35353] DECL_TAG 'address=0xffffffff811e1370' type_id=9287 component_idx=-1
[48934] DECL_TAG 'address=0xffffffff812fae80' type_id=9287 component_idx=-1
[54476] DECL_TAG 'address=0xffffffff81375c50' type_id=9287 component_idx=-1
[87772] DECL_TAG 'address=0xffffffff81634b60' type_id=9287 component_idx=-1
[108841] DECL_TAG 'address=0xffffffff817ba540' type_id=9287 component_idx=-1
[132557] DECL_TAG 'address=0xffffffff819abf30' type_id=9287 component_idx=-1
[143689] DECL_TAG 'address=0xffffffff81a7cb60' type_id=9287 component_idx=-1
right, Yonghong pointed this out in:
https://lore.kernel.org/bpf/49e4fee2-8be0-325f-3372-c79d96b686e9@xxxxxxxx/
it's problem, because we pass btf id as attach id during bpf program load,
and kernel does not have a way to figure out which address from the associated
DECL_TAGs to use
if we could change dedup algo to take the function address into account and
make it not de-duplicate equal functions with different addresses, then we
could:
- find btf id that properly and uniquely identifies the function we
want to trace
So maybe a more natural approach would be to extend BTF_KIND_FUNC
(I think Alexei suggested something this earlier but I could be
misremembering) as follows:
2.2.12 BTF_KIND_FUNC
~~~~~~~~~~~~~~~~~~~~
``struct btf_type`` encoding requirement:
* ``name_off``: offset to a valid C identifier
- * ``info.kind_flag``: 0
+ * ``info.kind_flag``: 0 or 1 if additional ``struct btf_func`` follows
* ``info.kind``: BTF_KIND_FUNC
* ``info.vlen``: linkage information (BTF_FUNC_STATIC, BTF_FUNC_GLOBAL
or BTF_FUNC_EXTERN)
* ``type``: a BTF_KIND_FUNC_PROTO type
- No additional type data follow ``btf_type``.
+ If ``info.kind_flag`` is specified, a ``struct btf_func`` follows.::
+
+ struct btf_func {
+ __u64 addr;
+ };
+ Otherwise no additional type data follows ``btf_type``.
With the above, dedup could be made to fail when functions have non-
identical associated addresses. Judging by the number of DECL_TAGs in
the RFC, we'd end up with ~1000 extra BTF_KIND_FUNCs, and the extra
space for struct btf_funcs would require roughly 400k. We'd still get
dedup of FUNC_PROTOs, so I suspect the extra size would be < 1MB.
Agree that this seems a better idea to save space and also not impacting
existing dedup algorithm. The only weird thing is previously we use
KIND + option vlen to decide overall size of the type, but now we need
KIND + kflag to decide FUNC type size. But this should be okay.
We need to add an option to pahole to enable this feature and
in kernel to enable this option only if the kernel supports new format.
Do we want to add addresses to all functions in which case we will have
FUNC types with both kflag 0 and kflag 1? Do you imagine whether
we potentially need to encode other additional information to FUNC type?
- store the vmlinux base address and treat stored function addresses as
offsets, so the verifier can get proper address even if the kernel
is relocated
yep; when we read kernel/module BTF in we could hopefully carry out
this recalculation and update the vmlinux/module BTF addresses
accordingly.
Thanks!
Alan
