Andrii, sorry for the late reply. This change looks good. It makes
much more sense than before.
Thanks,
Hao
On Thu, Dec 10, 2020 at 8:11 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote:
>
> Adjust pahole logic of skipping any per-CPU symbol with offset 0, which is
> especially bad for kernel modules, because it most certainly skips the very
> first per-CPU variable.
>
> Instead, do collect per-CPU ELF symbol with 0 offset, but do extra check for
> non-kernel module case by verifying that ELF symbol name and DWARF variable
> name match. Due to the bug of DWARF name of variable sometimes being NULL,
> this is necessarily too pessimistic check (e.g., on my vmlinux image,
> fixed_percpu_data variable is still not emitted due to missing DWARF variable
> name), it allows to emit data for all module per-CPU variables.
>
> Fixes: f3d9054ba8ff ("btf_encoder: Teach pahole to store percpu variables in vmlinux BTF.")
> Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> ---
> btf_encoder.c | 40 ++++++++++++++++++++++++----------------
> 1 file changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index a7d484765ce2..1d7817078f89 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -412,21 +412,6 @@ static int collect_percpu_var(struct btf_elf *btfe, GElf_Sym *sym)
> return 0;
>
> addr = elf_sym__value(sym);
> - /*
> - * Store only those symbols that have allocated space in the percpu section.
> - * This excludes the following three types of symbols:
> - *
> - * 1. __ADDRESSABLE(sym), which are forcely emitted as symbols.
> - * 2. __UNIQUE_ID(prefix), which are introduced to generate unique ids.
> - * 3. __exitcall(fn), functions which are labeled as exit calls.
> - *
> - * In addition, the variables defined using DEFINE_PERCPU_FIRST are
> - * also not included, which currently includes:
> - *
> - * 1. fixed_percpu_data
> - */
> - if (!addr)
> - return 0;
>
> size = elf_sym__size(sym);
> if (!size)
> @@ -652,7 +637,7 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
>
> cu__for_each_variable(cu, core_id, pos) {
> uint32_t size, type, linkage;
> - const char *name;
> + const char *name, *dwarf_name;
> uint64_t addr;
> int id;
>
> @@ -680,6 +665,29 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
> if (!percpu_var_exists(addr, &size, &name))
> continue; /* not a per-CPU variable */
>
> + /* A lot of "special" DWARF variables (e.g, __UNIQUE_ID___xxx)
> + * have addr == 0, which is the same as, say, valid
> + * fixed_percpu_data per-CPU variable. To distinguish between
> + * them, additionally compare DWARF and ELF symbol names. If
> + * DWARF doesn't provide proper name, pessimistically assume
> + * bad variable.
> + *
> + * Examples of such special variables are:
> + *
> + * 1. __ADDRESSABLE(sym), which are forcely emitted as symbols.
> + * 2. __UNIQUE_ID(prefix), which are introduced to generate unique ids.
> + * 3. __exitcall(fn), functions which are labeled as exit calls.
> + *
> + * This is relevant only for vmlinux image, as for kernel
> + * modules per-CPU data section has non-zero offset so all
> + * per-CPU symbols have non-zero values.
> + */
> + if (var->ip.addr == 0) {
> + dwarf_name = variable__name(var, cu);
> + if (!dwarf_name || strcmp(dwarf_name, name))
> + continue;
> + }
> +
> if (var->spec)
> var = var->spec;
>
> --
> 2.24.1
>
