Re: [PATCH] bpf: Mark kfuncs as __noclone

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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?

[...]





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux