lkml.org 
[lkml]   [2019]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRe: [PATCH v9 perf,bpf 12/15] perf, bpf: enable annotation of bpf program
    Em Tue, Mar 19, 2019 at 06:05:30AM +0000, Song Liu escreveu:
    > Sorry for the late response.
    >
    > > On Mar 18, 2019, at 9:38 AM, Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
    > >
    > > Em Mon, Mar 11, 2019 at 10:30:48PM -0700, Song Liu escreveu:
    > >> This patch enables the annotation of bpf program.
    > >>
    > >> A new dso type DSO_BINARY_TYPE__BPF_PROG_INFO is introduced to for BPF
    > >> programs. In symbol__disassemble(), DSO_BINARY_TYPE__BPF_PROG_INFO dso
    > >> calls into a new function symbol__disassemble_bpf(), where annotation
    > >> line information is filled based bpf_prog_info and btf saved in given
    > >> perf_env.
    > >>
    > >> symbol__disassemble_bpf() uses libbfd to disassemble bpf programs.
    > >>
    > >> Signed-off-by: Song Liu <songliubraving@fb.com>
    > >> ---
    > >> tools/build/Makefile.feature | 6 +-
    > >> tools/perf/Makefile.config | 4 +
    > >
    > > I see the changes to these two files, but I can't see the feature test
    > > that it triggers, forgot to do a git-add? Or is it in an upcoming patch
    > > in this series?
    >
    > test-disassembler-four-args.c is an existing test used by bpftool.
    > It was added in fb982666e380c1632a74495b68b3c33a66e76430 .

    Ok, I'll add that to the commit log message, but see below

    > >
    > > After applying this test it "works":
    > >
    > > make: Entering directory '/home/acme/git/perf/tools/perf'
    > > BUILD: Doing 'make -j8' parallel build
    > >
    > > Auto-detecting system features:
    > > ... dwarf: [ on ]
    > > ... dwarf_getlocations: [ on ]
    > > <SNIP>
    > > ... bpf: [ on ]
    > > ... libaio: [ on ]
    > > ... disassembler-four-args: [ on ]
    > >
    > > Because you added it to FEATURE_TESTS_BASIC, and this means that if
    > > tools/build/feature/test-all.c builds, then what is in
    > > FEATURE_TESTS_BASIC is set to 'on', but you didn't add anything to
    > > tools/build/feature/test-all.c, so it works because all the other
    > > features are present.
    > >
    > > Take a look at:
    > >
    > > 2a07d814747b ("tools build feature: Check if libaio is available")
    > >
    > > To see what needs to be done.
    >
    > I used different versions of gcc to verify that current version of the
    > patch works for both test-succeed and test-fail cases. I guess something
    > like the following is needed? But the test does work without it.

    The following is definetely needed, otherwise if all the other tests in
    test-all.c works, this disassembler-four-args feature test will never be
    performed.

    - Arnaldo

    > diff --git i/tools/build/feature/test-all.c w/tools/build/feature/test-all.c
    > index e903b86b742f..7853e6d91090 100644
    > --- i/tools/build/feature/test-all.c
    > +++ w/tools/build/feature/test-all.c
    > @@ -178,6 +178,10 @@
    > # include "test-reallocarray.c"
    > #undef main
    >
    > +#define main main_test_disassembler_four_args
    > +# include "test-disassembler-four-args.c"
    > +#undef main
    > +
    > int main(int argc, char *argv[])
    > {
    > main_test_libpython();
    > @@ -219,6 +223,7 @@ int main(int argc, char *argv[])
    > main_test_setns();
    > main_test_libaio();
    > main_test_reallocarray();
    > + main_test_disassembler_four_args();
    >
    > return 0;
    > }
    >
    >
    > >
    > > Either way, its interesting to break this patch in two, one adding the
    > > feature test and another to use it, i.e. the second patch out of this
    > > should have the commit log message in this patch.
    > >
    > > I've applied the patches up to before this one and will continue
    > > processing other patches while you address this.
    > >
    > > Thanks,
    > >
    > > - Arnaldo
    > >
    > >> tools/perf/util/annotate.c | 150 ++++++++++++++++++++++++++++++++++-
    > >> tools/perf/util/dso.c | 1 +
    > >> tools/perf/util/dso.h | 32 +++++---
    > >> tools/perf/util/symbol.c | 1 +
    > >> 6 files changed, 180 insertions(+), 14 deletions(-)
    > >>
    > >> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
    > >> index 61e46d54a67c..8d3864b061f3 100644
    > >> --- a/tools/build/Makefile.feature
    > >> +++ b/tools/build/Makefile.feature
    > >> @@ -66,7 +66,8 @@ FEATURE_TESTS_BASIC := \
    > >> sched_getcpu \
    > >> sdt \
    > >> setns \
    > >> - libaio
    > >> + libaio \
    > >> + disassembler-four-args
    > >>
    > >> # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
    > >> # of all feature tests
    > >> @@ -118,7 +119,8 @@ FEATURE_DISPLAY ?= \
    > >> lzma \
    > >> get_cpuid \
    > >> bpf \
    > >> - libaio
    > >> + libaio \
    > >> + disassembler-four-args
    > >>
    > >> # Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
    > >> # If in the future we need per-feature checks/flags for features not
    > >> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
    > >> index df4ad45599ca..c51b59e43dcc 100644
    > >> --- a/tools/perf/Makefile.config
    > >> +++ b/tools/perf/Makefile.config
    > >> @@ -808,6 +808,10 @@ ifdef HAVE_KVM_STAT_SUPPORT
    > >> CFLAGS += -DHAVE_KVM_STAT_SUPPORT
    > >> endif
    > >>
    > >> +ifeq ($(feature-disassembler-four-args), 1)
    > >> + CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
    > >> +endif
    > >> +
    > >> ifeq (${IS_64_BIT}, 1)
    > >> ifndef NO_PERF_READ_VDSO32
    > >> $(call feature_check,compile-32)
    > >> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
    > >> index 5f6dbbf5d749..e492b19a157c 100644
    > >> --- a/tools/perf/util/annotate.c
    > >> +++ b/tools/perf/util/annotate.c
    > >> @@ -10,6 +10,10 @@
    > >> #include <errno.h>
    > >> #include <inttypes.h>
    > >> #include <libgen.h>
    > >> +#include <bpf/bpf.h>
    > >> +#include <bpf/btf.h>
    > >> +#include <bpf/libbpf.h>
    > >> +#include <linux/btf.h>
    > >> #include "util.h"
    > >> #include "ui/ui.h"
    > >> #include "sort.h"
    > >> @@ -24,6 +28,7 @@
    > >> #include "annotate.h"
    > >> #include "evsel.h"
    > >> #include "evlist.h"
    > >> +#include "bpf-event.h"
    > >> #include "block-range.h"
    > >> #include "string2.h"
    > >> #include "arch/common.h"
    > >> @@ -31,6 +36,9 @@
    > >> #include <pthread.h>
    > >> #include <linux/bitops.h>
    > >> #include <linux/kernel.h>
    > >> +#include <bfd.h>
    > >> +#include <dis-asm.h>
    > >> +#include <bpf/libbpf.h>
    > >>
    > >> /* FIXME: For the HE_COLORSET */
    > >> #include "ui/browser.h"
    > >> @@ -1674,6 +1682,144 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
    > >> return 0;
    > >> }
    > >>
    > >> +static int symbol__disassemble_bpf(struct symbol *sym,
    > >> + struct annotate_args *args)
    > >> +{
    > >> + struct annotation *notes = symbol__annotation(sym);
    > >> + struct annotation_options *opts = args->options;
    > >> + struct bpf_prog_info_linear *info_linear;
    > >> + struct bpf_prog_linfo *prog_linfo = NULL;
    > >> + struct bpf_prog_info_node *info_node;
    > >> + int len = sym->end - sym->start;
    > >> + disassembler_ftype disassemble;
    > >> + struct map *map = args->ms.map;
    > >> + struct disassemble_info info;
    > >> + struct dso *dso = map->dso;
    > >> + int pc = 0, count, sub_id;
    > >> + struct btf *btf = NULL;
    > >> + char tpath[PATH_MAX];
    > >> + size_t buf_size;
    > >> + int nr_skip = 0;
    > >> + int ret = -1;
    > >> + char *buf;
    > >> + bfd *bfdf;
    > >> + FILE *s;
    > >> +
    > >> + if (dso->binary_type != DSO_BINARY_TYPE__BPF_PROG_INFO)
    > >> + return -1;
    > >> +
    > >> + pr_debug("%s: handling sym %s addr %lx len %lx\n", __func__,
    > >> + sym->name, sym->start, sym->end - sym->start);
    > >> +
    > >> + memset(tpath, 0, sizeof(tpath));
    > >> + perf_exe(tpath, sizeof(tpath));
    > >> +
    > >> + bfdf = bfd_openr(tpath, NULL);
    > >> + assert(bfdf);
    > >> + assert(bfd_check_format(bfdf, bfd_object));
    > >> +
    > >> + s = open_memstream(&buf, &buf_size);
    > >> + if (!s)
    > >> + goto out;
    > >> + init_disassemble_info(&info, s,
    > >> + (fprintf_ftype) fprintf);
    > >> +
    > >> + info.arch = bfd_get_arch(bfdf);
    > >> + info.mach = bfd_get_mach(bfdf);
    > >> +
    > >> + info_node = perf_env__find_bpf_prog_info(dso->bpf_prog.env,
    > >> + dso->bpf_prog.id);
    > >> + if (!info_node)
    > >> + goto out;
    > >> + info_linear = info_node->info_linear;
    > >> + sub_id = dso->bpf_prog.sub_id;
    > >> +
    > >> + info.buffer = (void *)(info_linear->info.jited_prog_insns);
    > >> + info.buffer_length = info_linear->info.jited_prog_len;
    > >> +
    > >> + if (info_linear->info.nr_line_info)
    > >> + prog_linfo = bpf_prog_linfo__new(&info_linear->info);
    > >> +
    > >> + if (info_linear->info.btf_id) {
    > >> + struct btf_node *node;
    > >> +
    > >> + node = perf_env__find_btf(dso->bpf_prog.env,
    > >> + info_linear->info.btf_id);
    > >> + if (node)
    > >> + btf = btf__new((__u8 *)(node->data),
    > >> + node->data_size);
    > >> + }
    > >> +
    > >> + disassemble_init_for_target(&info);
    > >> +
    > >> +#ifdef DISASM_FOUR_ARGS_SIGNATURE
    > >> + disassemble = disassembler(info.arch,
    > >> + bfd_big_endian(bfdf),
    > >> + info.mach,
    > >> + bfdf);
    > >> +#else
    > >> + disassemble = disassembler(bfdf);
    > >> +#endif
    > >> + assert(disassemble);
    > >> +
    > >> + fflush(s);
    > >> + do {
    > >> + const struct bpf_line_info *linfo = NULL;
    > >> + struct disasm_line *dl;
    > >> + size_t prev_buf_size;
    > >> + const char *srcline;
    > >> + u64 addr;
    > >> +
    > >> + addr = pc + ((u64 *)(info_linear->info.jited_ksyms))[sub_id];
    > >> + count = disassemble(pc, &info);
    > >> +
    > >> + if (prog_linfo)
    > >> + linfo = bpf_prog_linfo__lfind_addr_func(prog_linfo,
    > >> + addr, sub_id,
    > >> + nr_skip);
    > >> +
    > >> + if (linfo && btf) {
    > >> + srcline = btf__name_by_offset(btf, linfo->line_off);
    > >> + nr_skip++;
    > >> + } else
    > >> + srcline = NULL;
    > >> +
    > >> + fprintf(s, "\n");
    > >> + prev_buf_size = buf_size;
    > >> + fflush(s);
    > >> +
    > >> + if (!opts->hide_src_code && srcline) {
    > >> + args->offset = -1;
    > >> + args->line = strdup(srcline);
    > >> + args->line_nr = 0;
    > >> + args->ms.sym = sym;
    > >> + dl = disasm_line__new(args);
    > >> + if (dl) {
    > >> + annotation_line__add(&dl->al,
    > >> + &notes->src->source);
    > >> + }
    > >> + }
    > >> +
    > >> + args->offset = pc;
    > >> + args->line = buf + prev_buf_size;
    > >> + args->line_nr = 0;
    > >> + args->ms.sym = sym;
    > >> + dl = disasm_line__new(args);
    > >> + if (dl)
    > >> + annotation_line__add(&dl->al, &notes->src->source);
    > >> +
    > >> + pc += count;
    > >> + } while (count > 0 && pc < len);
    > >> +
    > >> + ret = 0;
    > >> +out:
    > >> + free(prog_linfo);
    > >> + free(btf);
    > >> + fclose(s);
    > >> + bfd_close(bfdf);
    > >> + return ret;
    > >> +}
    > >> +
    > >> static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
    > >> {
    > >> struct annotation_options *opts = args->options;
    > >> @@ -1701,7 +1847,9 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
    > >> pr_debug("annotating [%p] %30s : [%p] %30s\n",
    > >> dso, dso->long_name, sym, sym->name);
    > >>
    > >> - if (dso__is_kcore(dso)) {
    > >> + if (dso->binary_type == DSO_BINARY_TYPE__BPF_PROG_INFO) {
    > >> + return symbol__disassemble_bpf(sym, args);
    > >> + } else if (dso__is_kcore(dso)) {
    > >> kce.kcore_filename = symfs_filename;
    > >> kce.addr = map__rip_2objdump(map, sym->start);
    > >> kce.offs = sym->start;
    > >> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
    > >> index ba58ba603b69..58547c468c65 100644
    > >> --- a/tools/perf/util/dso.c
    > >> +++ b/tools/perf/util/dso.c
    > >> @@ -184,6 +184,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
    > >> case DSO_BINARY_TYPE__KALLSYMS:
    > >> case DSO_BINARY_TYPE__GUEST_KALLSYMS:
    > >> case DSO_BINARY_TYPE__JAVA_JIT:
    > >> + case DSO_BINARY_TYPE__BPF_PROG_INFO:
    > >> case DSO_BINARY_TYPE__NOT_FOUND:
    > >> ret = -1;
    > >> break;
    > >> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
    > >> index bb417c54c25a..c274b5aa839d 100644
    > >> --- a/tools/perf/util/dso.h
    > >> +++ b/tools/perf/util/dso.h
    > >> @@ -14,6 +14,7 @@
    > >>
    > >> struct machine;
    > >> struct map;
    > >> +struct perf_env;
    > >>
    > >> enum dso_binary_type {
    > >> DSO_BINARY_TYPE__KALLSYMS = 0,
    > >> @@ -35,6 +36,7 @@ enum dso_binary_type {
    > >> DSO_BINARY_TYPE__KCORE,
    > >> DSO_BINARY_TYPE__GUEST_KCORE,
    > >> DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO,
    > >> + DSO_BINARY_TYPE__BPF_PROG_INFO,
    > >> DSO_BINARY_TYPE__NOT_FOUND,
    > >> };
    > >>
    > >> @@ -178,17 +180,25 @@ struct dso {
    > >> struct auxtrace_cache *auxtrace_cache;
    > >> int comp;
    > >>
    > >> - /* dso data file */
    > >> - struct {
    > >> - struct rb_root cache;
    > >> - int fd;
    > >> - int status;
    > >> - u32 status_seen;
    > >> - size_t file_size;
    > >> - struct list_head open_entry;
    > >> - u64 debug_frame_offset;
    > >> - u64 eh_frame_hdr_offset;
    > >> - } data;
    > >> + union {
    > >> + /* dso data file */
    > >> + struct {
    > >> + struct rb_root cache;
    > >> + int fd;
    > >> + int status;
    > >> + u32 status_seen;
    > >> + size_t file_size;
    > >> + struct list_head open_entry;
    > >> + u64 debug_frame_offset;
    > >> + u64 eh_frame_hdr_offset;
    > >> + } data;
    > >> + /* bpf prog information */
    > >> + struct {
    > >> + u32 id;
    > >> + u32 sub_id;
    > >> + struct perf_env *env;
    > >> + } bpf_prog;
    > >> + };
    > >>
    > >> union { /* Tool specific area */
    > >> void *priv;
    > >> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
    > >> index 758bf5f74e6e..4e2e304d4037 100644
    > >> --- a/tools/perf/util/symbol.c
    > >> +++ b/tools/perf/util/symbol.c
    > >> @@ -1451,6 +1451,7 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
    > >> case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO:
    > >> return true;
    > >>
    > >> + case DSO_BINARY_TYPE__BPF_PROG_INFO:
    > >> case DSO_BINARY_TYPE__NOT_FOUND:
    > >> default:
    > >> return false;
    > >> --
    > >> 2.17.1
    > >
    > > --
    > >
    > > - Arnaldo

    --

    - Arnaldo

    \
     
     \ /
      Last update: 2019-03-19 14:39    [W:4.279 / U:0.452 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site