On Thu, May 18, 2023 at 02:23:34PM +0100, 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. nice, I think it's better solution > > > > > - 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. I wonder now when the address will be stored as number (not string) we could somehow generate relocation records and have the module loader do the relocation automatically not sure how that works for vmlinux when it's loaded/relocated on boot jirka
