Re: [PATCH dwarves 2/2] btf_encoder: fix skipping per-CPU variables at offset 0

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

 



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
>



[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