Em Wed, Feb 01, 2023 at 05:18:29PM +0000, Alan Maguire escreveu:
> On 01/02/2023 17:01, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Feb 01, 2023 at 08:49:07AM -0800, Alexei Starovoitov escreveu:
> >> It feels fixed pahole should be done under some flag
> >> otherwise when people update the pahole the existing and older
> >> kernels might stop building with warns:
> >> WARN: resolve_btfids: unresolved symbol bpf_xdp_metadata_rx_hash
> >> WARN: resolve_btfids: unresolved symbol bpf_task_kptr_get
> >> ...
> Good point, something like
> --skip_inconsistent_proto Skip functions that have multiple inconsistent
> function prototypes sharing the same name, or
> have optimized-out parameters.
We have:
⬢[acme@toolbox pahole]$ grep '"skip_encoding.*' pahole.c
.name = "skip_encoding_btf_vars",
.name = "skip_encoding_btf_decl_tag",
.name = "skip_encoding_btf_type_tag",
.name = "skip_encoding_btf_enum64",
⬢[acme@toolbox pahole]$
Perhaps, even being long, we should be consistent and name it:
--skip_encoding_btf_inconsistent_proto
?
> ? Implementation needs a bit of thought though because we're
> not really doing the same thing that we were before. Previously we
> were adding the first instance of a function in the CU we came across.
> Probably safest to resurrect that behaviour for the legacy
> non-skip-inconsistent-proto case I think. The final patch handling
Consider getting what I have now in my next branch, that has the fixups
I made while reviewing, as discussed in this thread:
⬢[acme@toolbox pahole]$ git log --oneline -6
b1576cf15106efd7 (HEAD -> master) pahole: Sync with libbpf-1.1
e9db5622d97395b7 btf_encoder: Delay function addition to check for function prototype inconsistencies
74675488e8ed5718 btf_encoder: Represent "."-suffixed functions (".isra.0") in BTF
be470fa5757e5915 btf_encoder: Rework btf_encoders__*() API to allow traversal of encoders
d6e0778f6b5912da btf_encoder: Refactor function addition into dedicated btf_encoder__add_func
f77b5ae93844b5c4 dwarf_loader: Help spotting functions with optimized-out parameters
⬢[acme@toolbox pahole]$
And at the point where you change the behaviour you introduce the
option, so that we don't have to remove it and then ressurect.
- Arnaldo
> inconsistent function prototypes will need to be reworked a bit to
> support this, since we tossed this approach and used saving/merging
> multiple instances in the tree instead. Once I've built bpf trees I'll
> have a go at getting this working.
>
> >> Arnaldo, could you check what warns do you see with this fixed pahole
> >> in bpf tree ?
> >
> > Sure.
> >
>
> I can collect this for x86_64/aarch64 too; might take a few hours
> before I have the results.
>
> >> If there are only few warns then we can manually add __used noinline
> >> to these places, push to bpf tree and push to stable.
> >>
> >> Then in bpf-next we can clean up everything with __bpf_kfunc.
> >
--
- Arnaldo
