Re: [PATCH] bpf: Mark kfuncs as __noclone

[Yonghong Song <yonghong.song@linux.dev>]

On 8/27/25 12:13 PM, Eduard Zingerman wrote:
> On Wed, 2025-08-27 at 10:00 -0700, Yonghong Song wrote:
>> On 8/26/25 10:02 PM, Eduard Zingerman wrote:
>>> On Tue, 2025-08-26 at 13:17 -0700, Yonghong Song wrote:
>>>
>>> [...]
>>>
>>>> I tried with gcc14 and can reproduced the issue described in the above.
>>>> I build the kernel like below with gcc14
>>>>      make KCFLAGS='-O3' -j
>>>> and get the following build error
>>>>      WARN: resolve_btfids: unresolved symbol bpf_strnchr
>>>>      make[2]: *** [/home/yhs/work/bpf-next/scripts/Makefile.vmlinux:91: vmlinux] Error 255
>>>>      make[2]: *** Deleting file 'vmlinux'
>>>> Checking the symbol table:
>>>>       22276: ffffffff81b15260   249 FUNC    LOCAL  DEFAULT    1 bpf_strnchr.cons[...]
>>>>      235128: ffffffff81b1f540   296 FUNC    GLOBAL DEFAULT    1 bpf_strnchr
>>>> and the disasm code:
>>>>      bpf_strnchr:
>>>>        ...
>>>>
>>>>      bpf_strchr:
>>>>        ...
>>>>        bpf_strnchr.constprop.0
>>>>        ...
>>>>
>>>> So in symbol table, we have both bpf_strnchr.constprop.0 and bpf_strnchr.
>>>> For such case, pahole will skip func bpf_strnchr hence the above resolve_btfids
>>>> failure.
>>>>
>>>> The solution in this patch can indeed resolve this issue.
>>> It looks like instead of adding __noclone there is an option to
>>> improve pahole's filtering of ambiguous functions.
>>> Abstractly, there is nothing wrong with having a clone of a global
>>> function that has undergone additional optimizations. As long as the
>>> original symbol exists, everything should be fine.
>> Right. The generated code itself is totally fine. The problem is
>> currently pahole will filter out bpf_strnchr since in the symbol table
>> having both bpf_strnchr and bpf_strnchr.constprop.0. It there is
>> no explicit dwarf-level signature in dwarf for bpf_strnchr.constprop.0.
>> (For this particular .constprop.0 case, it is possible to derive the
>>    signature. but it will be hard for other suffixes like .isra).
>> The current pahole will have strip out suffixes so the function
>> name is 'bpf_strnchr' which covers bpf_strnchr and bpf_strnchr.constprop.0.
>> Since two underlying signature is different, the 'bpf_strnchr'
>> will be filtered out.
> Yes, I understand the mechanics. My question is: is it really
> necessary for pahole to go through this process?
>
> It sees two functions: 'bpf_strnchr', 'bpf_strnchr.constprop.0',
> first global, second local, first with DWARF signature, second w/o
> DWARF signature. So, why conflating the two?

In this particular case, I think what you describe the correct.
For *Global* symbol 'bpf_strnchr', the signature should be in
the dwarf. But for *Local* symbol 'bpf_strnchr.constprop.0', the
signature is not clear. I suspect that pahole may not
distinguish between *Global* and *Local* symbols where they have
the same prefix.

The case like this patch to have a clone for a kfunc global
func should be very rare. That is another reason I think
__noclone should be good enough and it can reduce the
complexity in pahole. But I will be okay as well if the
consensus is to implement the support in pahole.

>
> For non-lto build the function being global guarantees signature
> correctness, and below you confirm that it is the case for lto builds
> as well. So, it looks like we are just loosing 'bpf_strnchr' for no
> good reason.
>
>> I am actually working to improve such cases in llvm to address
>> like foo() and foo.<...>() functions and they will have their
>> own respective functions. We will discuss with gcc folks
>> about how to implement similar approaches in gcc.
>>
>>> Since kfuncs are global, this should guarantee that the compiler does not
>>> change their signature, correct? Does this also hold for LTO builds?
>> Yes, the original signature will not changed. This holds for LTO build
>> and global variables/functions will not be renamed.
>>
>>> If so, when pahole sees a set of symbols like [foo, foo.1, foo.2, ...],
>> The compiler needs to emit the signature in dwarf for foo.1, foo.2, etc. and this
>> is something I am working on.
>>
>>> with 'foo' being global and the rest local, then there is no real need
>>> to filter out 'foo'.
>> I think the current __noclone approach is okay as the full implementation
>> for signature changes (foo, foo.1, ...) might takes a while for both llvm
>> and gcc.
>>
>>> Wdyt?
>>>
>>> [...]