On 30/06/2025 14:51, Jiri Olsa wrote:
> On Mon, Jun 30, 2025 at 11:01:19AM +0100, Alan Maguire wrote:
>> On 24/06/2025 17:14, Alan Maguire wrote:
>>> On 22/05/2025 07:37, Tony Ambardar wrote:
>>>> I encountered an issue building BTF kernels for 32-bit armhf, where many
>>>> functions are missing in BTF data:
>>>>
>>>> LD vmlinux
>>>> BTFIDS vmlinux
>>>> WARN: resolve_btfids: unresolved symbol vfs_truncate
>>>> WARN: resolve_btfids: unresolved symbol vfs_fallocate
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_select_cpu_dfl
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu_node
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu_node
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_kick_cpu
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_exit_bstr
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_nr_queued
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_vtime
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_to_local
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert_vtime
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime_from_dsq
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_vtime
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_slice
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_destroy_dsq
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_create_dsq
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_consume
>>>> WARN: resolve_btfids: unresolved symbol bpf_throw
>>>> WARN: resolve_btfids: unresolved symbol bpf_sock_ops_enable_tx_tstamp
>>>> WARN: resolve_btfids: unresolved symbol bpf_percpu_obj_new_impl
>>>> WARN: resolve_btfids: unresolved symbol bpf_obj_new_impl
>>>> WARN: resolve_btfids: unresolved symbol bpf_lookup_user_key
>>>> WARN: resolve_btfids: unresolved symbol bpf_lookup_system_key
>>>> WARN: resolve_btfids: unresolved symbol bpf_iter_task_vma_new
>>>> WARN: resolve_btfids: unresolved symbol bpf_iter_scx_dsq_new
>>>> WARN: resolve_btfids: unresolved symbol bpf_get_kmem_cache
>>>> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_xdp
>>>> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_skb
>>>> WARN: resolve_btfids: unresolved symbol bpf_cgroup_from_id
>>>> NM System.map
>>>>
>>>> After further debugging this can be reproduced more simply:
>>>>
>>>> $ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf
>>>> btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF
>>>> btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl'
>>>>
>>>> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
>>>> <nothing>
>>>>
>>>> $ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
>>>> s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
>>>>
>>>> $ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf
>>>>
>>>> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
>>>> bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
>>>>
>>>> The key things to note are the pahole 'consistent_func' feature and the u64
>>>> 'wake_flags' parameter vs. arm 32-bit registers. These point to existing
>>>> code handling arguments larger than register-size, allowing them to be
>>>> BTF encoded but only if structs.
>>>>
>>>> Generalize the code for any argument type larger than register size (i.e.
>>>> size > cu->addr_size). This should work for integral or aggregate types,
>>>> and also avoids a bug in the current code where a register-sized struct
>>>> could be mistaken for larger. Note that zero-sized arguments will still
>>>> be marked as inconsistent and not encoded.
>>>>
>>>> Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
>>>> Tested-by: Alexis Lothoré <alexis.lothore@xxxxxxxxxxx>
>>>> Tested-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
>>>> Signed-off-by: Tony Ambardar <tony.ambardar@xxxxxxxxx>
>>>
>>> hi Tony,
>>>
>>> I'm planning on landing this shortly unless anyone objects; and on that
>>> topic if anyone has the cycles to test with this patch that would be
>>> great! I ran it through the work-in-progress BTF comparison in github CI
>>> and all looks good; see the "Compare functions generated" step in [1].
>>>
>>> Thanks!
>>>
>>
>> In fact I spoke too soon; there was a bug in the function comparison.
>> After that was fixed, I reran with this patch; see [1].
>>
>> It shows that - as expected - functions with 0-sized params are left
>> out, specifically
>>
>> < int __io_run_local_work(struct io_ring_ctx * ctx, io_tw_token_t tw,
>> int min_events, int max_events);
>> < int __io_run_local_work_loop(struct llist_node * * node, io_tw_token_t
>> tw, int events);
>>
>> We expect this since io_tw_token_t is 0-sized. However on x86_64 it did
>> show one _extra_ function that I didn't expect:
>>
>>> int __vxlan_fdb_delete(struct vxlan_dev * vxlan, const unsigned char
>> * addr, union vxlan_addr ip, __be16 port, __be32 src_vni, __be32 vni,
>> u32 ifindex, bool swdev_notify);
>>
>> It's not clear to me why that function was added with this change - I
>> would have expected it either with or without the change. Any idea why
>> that might be?
>
> hi,
> I can see that as well, IIUC the 'ip' argument is:
>
> union vxlan_addr {
> struct sockaddr_in sin;
> struct sockaddr_in6 sin6;
> struct sockaddr sa;
> };
>
> so we have struct as 4th argument, which sets the has_wide_param condition
> and won't set the fn->proto.unexpected_reg for the function, because of:
>
> if (!has_wide_param)
> fn->proto.unexpected_reg = 1;
>
> I'm not sure it's correct.. if the ip struct is big enough that it's passed
> on stack, why are the rest of the arguments marked with unexpected_reg
> (in parameter__new) I think I'm missing something
>
Ah you've nailed it, that's the reason! Before we only checked for
struct/typedef or const struct/typedef. However in this case it is a
union, so moving to a size-based rather than an (incomplete) type-based
tests means we previously excluded this function but don't now.
Alan
