All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Alexei Starovoitov <ast@kernel.org>
Cc: "David S . Miller" <davem@davemloft.net>,
	daniel@iogearbox.net, peterz@infradead.org,
	netdev@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH perf 3/3] tools/perf: recognize and process RECORD_MMAP events for bpf progs
Date: Thu, 20 Sep 2018 11:05:06 -0300	[thread overview]
Message-ID: <20180920140506.GD19861@kernel.org> (raw)
In-Reply-To: <20180920133617.GB19861@kernel.org>

Em Thu, Sep 20, 2018 at 10:36:17AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Sep 19, 2018 at 03:39:35PM -0700, Alexei Starovoitov escreveu:
> > Recognize JITed bpf prog load/unload events.
> > Add/remove kernel symbols accordingly.
> > 
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> >  tools/perf/util/machine.c | 27 +++++++++++++++++++++++++++
> >  tools/perf/util/symbol.c  | 13 +++++++++++++
> >  tools/perf/util/symbol.h  |  1 +
> >  3 files changed, 41 insertions(+)
> > 
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index c4acd2001db0..ae4f8a0fdc7e 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -25,6 +25,7 @@
> >  #include "sane_ctype.h"
> >  #include <symbol/kallsyms.h>
> >  #include <linux/mman.h>
> > +#include <linux/magic.h>
> >  
> >  static void __machine__remove_thread(struct machine *machine, struct thread *th, bool lock);
> >  
> > @@ -1460,6 +1461,32 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> >  	enum dso_kernel_type kernel_type;
> >  	bool is_kernel_mmap;
> >  
> > +	/* process JITed bpf programs load/unload events */
> > +	if (event->mmap.pid == ~0u && event->mmap.tid == BPF_FS_MAGIC) {
> 
> 
> So, this would be in machine__process_kernel_munmap-event(machine), etc,
> no check for BPF_FS_MAGIC would be needed with a PERF_RECORD_MUNMAP.
> 
> > +		struct symbol *sym;
> > +		u64 ip;
> > +
> > +		map = map_groups__find(&machine->kmaps, event->mmap.start);
> > +		if (!map) {
> > +			pr_err("No kernel map for IP %lx\n", event->mmap.start);
> > +			goto out_problem;
> > +		}
> > +		ip = event->mmap.start - map->start + map->pgoff;
> > +		if (event->mmap.filename[0]) {
> > +			sym = symbol__new(ip, event->mmap.len, 0, 0,
> > +					  event->mmap.filename);
> 
> Humm, so the bpf program would be just one symbol... bpf-to-bpf calls
> will be to a different bpf program, right? 
> 
> /me goes to read https://lwn.net/Articles/741773/
>                  "[PATCH bpf-next 00/13] bpf: introduce function calls"

After reading it, yeah, I think we need some way to access a symbol
table for a BPF program, and also its binary so that we can do
annotation, static (perf annotate) and live (perf top), was this already
considered? I think one can get the binary for a program giving
sufficient perms somehow, right? One other thing I need to catch up 8-)

- Arnaldo
 
> > +			dso__insert_symbol(map->dso, sym);
> > +		} else {
> > +			if (symbols__erase(&map->dso->symbols, ip)) {
> > +				pr_err("No bpf prog at IP %lx/%lx\n",
> > +				       event->mmap.start, ip);
> > +				goto out_problem;
> > +			}
> > +			dso__reset_find_symbol_cache(map->dso);
> > +		}
> > +		return 0;
> > +	}
> > +
> >  	/* If we have maps from kcore then we do not need or want any others */
> >  	if (machine__uses_kcore(machine))
> >  		return 0;
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index d188b7588152..0653f313661d 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -353,6 +353,19 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip)
> >  	return NULL;
> >  }
> >  
> > +int symbols__erase(struct rb_root *symbols, u64 ip)
> > +{
> > +	struct symbol *s;
> > +
> > +	s = symbols__find(symbols, ip);
> > +	if (!s)
> > +		return -ENOENT;
> > +
> > +	rb_erase(&s->rb_node, symbols);
> > +	symbol__delete(s);
> > +	return 0;
> > +}
> > +
> >  static struct symbol *symbols__first(struct rb_root *symbols)
> >  {
> >  	struct rb_node *n = rb_first(symbols);
> > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> > index f25fae4b5743..92ef31953d9a 100644
> > --- a/tools/perf/util/symbol.h
> > +++ b/tools/perf/util/symbol.h
> > @@ -310,6 +310,7 @@ char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name);
> >  
> >  void __symbols__insert(struct rb_root *symbols, struct symbol *sym, bool kernel);
> >  void symbols__insert(struct rb_root *symbols, struct symbol *sym);
> > +int symbols__erase(struct rb_root *symbols, u64 ip);
> >  void symbols__fixup_duplicate(struct rb_root *symbols);
> >  void symbols__fixup_end(struct rb_root *symbols);
> >  void map_groups__fixup_end(struct map_groups *mg);
> > -- 
> > 2.17.1

  reply	other threads:[~2018-09-20 19:48 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-19 22:39 [PATCH bpf-next 0/3] perf, bpf: reveal invisible bpf programs Alexei Starovoitov
2018-09-19 22:39 ` [PATCH bpf-next 1/3] perf/core: introduce perf_event_mmap_bpf_prog Alexei Starovoitov
2018-09-19 23:30   ` Song Liu
2018-09-20  0:53     ` Alexei Starovoitov
2018-09-19 22:39 ` [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload Alexei Starovoitov
2018-09-19 23:44   ` Song Liu
2018-09-20  0:59     ` Alexei Starovoitov
2018-09-20  5:48       ` Song Liu
2018-09-20  8:44   ` Peter Zijlstra
2018-09-20 13:25     ` Arnaldo Carvalho de Melo
2018-09-20 13:56       ` Peter Zijlstra
2018-09-21  3:14         ` Alexei Starovoitov
2018-09-21 12:25           ` Arnaldo Carvalho de Melo
2018-09-21 13:55             ` Peter Zijlstra
2018-09-21 13:59             ` Peter Zijlstra
2018-09-21 22:13             ` Alexei Starovoitov
2018-10-15 23:33               ` Song Liu
2018-10-16 23:43                 ` David Ahern
2018-10-17  6:43                   ` Song Liu
2018-10-17 12:11                     ` Arnaldo Carvalho de Melo
2018-10-17 12:50                       ` Arnaldo Carvalho de Melo
2018-10-17 16:06                         ` Song Liu
2018-11-02  1:08                       ` Song Liu
2018-10-17 15:09                     ` David Ahern
2018-10-17 16:18                       ` Song Liu
2018-10-17 16:36                       ` Alexei Starovoitov
2018-10-17 18:53                         ` Arnaldo Carvalho de Melo
2018-10-17 19:08                           ` Alexei Starovoitov
2018-10-17 21:31                             ` Arnaldo Carvalho de Melo
2018-10-17 22:01                               ` Alexei Starovoitov
2018-09-20 13:36     ` Peter Zijlstra
2018-09-19 22:39 ` [PATCH perf 3/3] tools/perf: recognize and process RECORD_MMAP events for bpf progs Alexei Starovoitov
2018-09-20 13:36   ` Arnaldo Carvalho de Melo
2018-09-20 14:05     ` Arnaldo Carvalho de Melo [this message]
2018-09-21 22:57   ` Song Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180920140506.GD19861@kernel.org \
    --to=acme@kernel.org \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.