Em Thu, Dec 03, 2020 at 03:23:57PM -0800, Andrii Nakryiko escreveu: > On Thu, Dec 3, 2020 at 2:08 PM Jiri Olsa <jolsa@xxxxxxxxxx> wrote: > > > > Add support to detect kernel module ftrace addresses and use > > it as filter for detected functions. > > > > For kernel modules the ftrace addresses are stored in __mcount_loc > > section. Adding the code that detects this section and reads > > its data into array, which is then processed as filter by > > current code. > > > > There's one tricky point with kernel modules wrt Elf object, > > which we get from dwfl_module_getelf function. This function > > performs all possible relocations, including __mcount_loc > > section. > > > > So addrs array contains relocated values, which we need take > > into account when we compare them to functions values which > > are relative to their sections. > > > > With this change for example for xfs.ko module in my kernel > > config, I'm getting slightly bigger number of functions: > > > > before: 2373, after: 2601 > > > > The ftrace's available_filter_functions still shows 2701, but > > it includes functions like: > > > > suffix_kstrtoint.constprop.0 > > xchk_btree_check_minrecs.isra.0 > > xfs_ascii_ci_compname.part.0 > > > > which are not part of dwarf data, the rest matches BTF functions. > > > > Because of the malfunction DWARF's declaration tag, the 'before' > > functions contain also functions that are not part of the module. > > The 'after' functions contain only functions that are traceable > > and part of xfs.ko. > > > > Despite filtering out some declarations, this change also adds > > static functions, hence the total number of functions is bigger. > > > > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx> > > --- > > I've tested locally on bpf_testmod that I'm adding to selftests in > [0]. All worked well. I changed the test function from global to > non-inlined static, and BTF had it. And the tests passed. So LGTM. > > Tested-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx> Thanks, applied the three patches in this series, - Arnaldo > [0] https://patchwork.kernel.org/user/todo/netdevbpf/?series=395715&delegate=121173&state=* > > > btf_encoder.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++-- > > dutil.c | 10 +++++ > > dutil.h | 2 + > > 3 files changed, 109 insertions(+), 3 deletions(-) > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > index 398be0fbf7c7..570fce46b6d9 100644 > > --- a/btf_encoder.c > > +++ b/btf_encoder.c > > @@ -36,6 +36,7 @@ struct funcs_layout { > > struct elf_function { > > const char *name; > > unsigned long addr; > > + unsigned long sh_addr; > > bool generated; > > }; > > > > @@ -65,11 +66,12 @@ static void delete_functions(void) > > static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > { > > struct elf_function *new; > > + static GElf_Shdr sh; > > + static int last_idx; > > + int idx; > > > > if (elf_sym__type(sym) != STT_FUNC) > > return 0; > > - if (!elf_sym__value(sym)) > > - return 0; > > > > if (functions_cnt == functions_alloc) { > > functions_alloc = max(1000, functions_alloc * 3 / 2); > > @@ -84,8 +86,17 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > functions = new; > > } > > > > + idx = elf_sym__section(sym); > > + > > + if (idx != last_idx) { > > + if (!elf_section_by_idx(btfe->elf, &sh, idx)) > > + return 0; > > + last_idx = idx; > > + } > > + > > functions[functions_cnt].name = elf_sym__name(sym, btfe->symtab); > > functions[functions_cnt].addr = elf_sym__value(sym); > > + functions[functions_cnt].sh_addr = sh.sh_addr; > > functions[functions_cnt].generated = false; > > functions_cnt++; > > return 0; > > @@ -160,10 +171,74 @@ static int get_vmlinux_addrs(struct btf_elf *btfe, struct funcs_layout *fl, > > return 0; > > } > > > > +static int > > +get_kmod_addrs(struct btf_elf *btfe, __u64 **paddrs, __u64 *pcount) > > +{ > > + __u64 *addrs, count; > > + unsigned int addr_size, i; > > + GElf_Shdr shdr_mcount; > > + Elf_Data *data; > > + Elf_Scn *sec; > > + > > + /* Initialize for the sake of all error paths below. */ > > + *paddrs = NULL; > > + *pcount = 0; > > + > > + /* get __mcount_loc */ > > + sec = elf_section_by_name(btfe->elf, &btfe->ehdr, &shdr_mcount, > > + "__mcount_loc", NULL); > > + if (!sec) { > > + if (btf_elf__verbose) { > > + printf("%s: '%s' doesn't have __mcount_loc section\n", __func__, > > + btfe->filename); > > + } > > + return 0; > > + } > > + > > + data = elf_getdata(sec, NULL); > > + if (!data) { > > + fprintf(stderr, "Failed to data for __mcount_loc section.\n"); > > + return -1; > > + } > > + > > + /* Get address size from processed file's ELF class. */ > > + addr_size = gelf_getclass(btfe->elf) == ELFCLASS32 ? 4 : 8; > > + > > + count = data->d_size / addr_size; > > + > > + addrs = malloc(count * sizeof(addrs[0])); > > + if (!addrs) { > > + fprintf(stderr, "Failed to allocate memory for ftrace addresses.\n"); > > + return -1; > > + } > > + > > + if (addr_size == sizeof(__u64)) { > > + memcpy(addrs, data->d_buf, count * addr_size); > > + } else { > > + for (i = 0; i < count; i++) > > + addrs[i] = (__u64) *((__u32 *) (data->d_buf + i * addr_size)); > > + } > > + > > + /* > > + * We get Elf object from dwfl_module_getelf function, > > + * which performs all possible relocations, including > > + * __mcount_loc section. > > + * > > + * So addrs array now contains relocated values, which > > + * we need take into account when we compare them to > > + * functions values, see comment in setup_functions > > + * function. > > + */ > > + *paddrs = addrs; > > + *pcount = count; > > + return 0; > > +} > > + > > static int setup_functions(struct btf_elf *btfe, struct funcs_layout *fl) > > { > > __u64 *addrs, count, i; > > int functions_valid = 0; > > + bool kmod = false; > > > > /* > > * Check if we are processing vmlinux image and > > @@ -172,6 +247,16 @@ static int setup_functions(struct btf_elf *btfe, struct funcs_layout *fl) > > if (get_vmlinux_addrs(btfe, fl, &addrs, &count)) > > return -1; > > > > + /* > > + * Check if we are processing kernel module and > > + * get mcount data if it's detected. > > + */ > > + if (!addrs) { > > + if (get_kmod_addrs(btfe, &addrs, &count)) > > + return -1; > > + kmod = true; > > + } > > + > > if (!addrs) { > > if (btf_elf__verbose) > > printf("ftrace symbols not detected, falling back to DWARF data\n"); > > @@ -188,9 +273,18 @@ static int setup_functions(struct btf_elf *btfe, struct funcs_layout *fl) > > */ > > for (i = 0; i < functions_cnt; i++) { > > struct elf_function *func = &functions[i]; > > + /* > > + * For vmlinux image both addrs[x] and functions[x]::addr > > + * values are final address and are comparable. > > + * > > + * For kernel module addrs[x] is final address, but > > + * functions[x]::addr is relative address within section > > + * and needs to be relocated by adding sh_addr. > > + */ > > + __u64 addr = kmod ? func->addr + func->sh_addr : func->addr; > > > > /* Make sure function is within ftrace addresses. */ > > - if (bsearch(&func->addr, addrs, count, sizeof(addrs[0]), addrs_cmp)) { > > + if (bsearch(&addr, addrs, count, sizeof(addrs[0]), addrs_cmp)) { > > /* > > * We iterate over sorted array, so we can easily skip > > * not valid item and move following valid field into > > diff --git a/dutil.c b/dutil.c > > index f7b853f0660d..7b667647420f 100644 > > --- a/dutil.c > > +++ b/dutil.c > > @@ -196,6 +196,16 @@ Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep, > > return sec; > > } > > > > +Elf_Scn *elf_section_by_idx(Elf *elf, GElf_Shdr *shp, int idx) > > +{ > > + Elf_Scn *sec; > > + > > + sec = elf_getscn(elf, idx); > > + if (sec) > > + gelf_getshdr(sec, shp); > > + return sec; > > +} > > + > > char *strlwr(char *s) > > { > > int len = strlen(s), i; > > diff --git a/dutil.h b/dutil.h > > index 676770d5d5c9..0838dff2d679 100644 > > --- a/dutil.h > > +++ b/dutil.h > > @@ -324,6 +324,8 @@ void *zalloc(const size_t size); > > Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep, > > GElf_Shdr *shp, const char *name, size_t *index); > > > > +Elf_Scn *elf_section_by_idx(Elf *elf, GElf_Shdr *shp, int idx); > > + > > #ifndef SHT_GNU_ATTRIBUTES > > /* Just a way to check if we're using an old elfutils version */ > > static inline int elf_getshdrstrndx(Elf *elf, size_t *dst) > > -- > > 2.26.2 > > -- - Arnaldo
