Message ID | 20200208154209.1797988-1-jolsa@kernel.org |
---|---|
Headers | show |
Series | bpf: Add trampoline and dispatcher to /proc/kallsyms | expand |
On Sat, 8 Feb 2020 at 16:42, Jiri Olsa <jolsa@kernel.org> wrote: > > hi, > this patchset adds trampoline and dispatcher objects > to be visible in /proc/kallsyms. The last patch also > adds sorting for all bpf objects in /proc/kallsyms. > Thanks for working on this! I'm probably missing something with my perf setup; I've applied your patches, and everything seem to work fine from an kallsyms perspective: # grep bpf_dispatcher_xdp /proc/kallsyms ... ffffffffc0511000 t bpf_dispatcher_xdp [bpf] However, when I run # perf top I still see the undecorated one: 0.90% [unknown] [k] 0xffffffffc0511037 Any ideas? Björn
On Mon, Feb 10, 2020 at 04:51:08PM +0100, Björn Töpel wrote: > On Sat, 8 Feb 2020 at 16:42, Jiri Olsa <jolsa@kernel.org> wrote: > > > > hi, > > this patchset adds trampoline and dispatcher objects > > to be visible in /proc/kallsyms. The last patch also > > adds sorting for all bpf objects in /proc/kallsyms. > > > > Thanks for working on this! > > I'm probably missing something with my perf setup; I've applied your > patches, and everything seem to work fine from an kallsyms > perspective: > > # grep bpf_dispatcher_xdp /proc/kallsyms > ... > ffffffffc0511000 t bpf_dispatcher_xdp [bpf] > > However, when I run > # perf top > > I still see the undecorated one: > 0.90% [unknown] [k] 0xffffffffc0511037 > > Any ideas? yea strange.. it should be picked up from /proc/kallsyms as fallback if there's no other source, I'll check on that (might be the problem with perf depending on address going only higher in /proc/kallsyms, while bpf symbols are at the end and start over from the lowest bpf address) anyway, in perf we enumerate bpf_progs via the perf events PERF_BPF_EVENT_PROG_LOAD,PERF_BPF_EVENT_PROG_UNLOAD interface together with PERF_RECORD_KSYMBOL_TYPE_BPF events we might need to add something like: PERF_RECORD_KSYMBOL_TYPE_BPF_TRAMPOLINE PERF_RECORD_KSYMBOL_TYPE_BPF_DISPATCHER to notify about the area, I'll check on that however the /proc/kallsyms fallback should work in any case.. thanks for report ;-) jirka
Em Sat, Feb 08, 2020 at 04:41:55PM +0100, Jiri Olsa escreveu: > hi, > this patchset adds trampoline and dispatcher objects > to be visible in /proc/kallsyms. The last patch also > adds sorting for all bpf objects in /proc/kallsyms. This will allow those to appear in profiles, right? That would be interesting to explicitely state, i.e. the _why_ of this patch, not just the _what_. Thanks, - Arnaldo > $ sudo cat /proc/kallsyms | tail -20 > ... > ffffffffa050f000 t bpf_prog_5a2b06eab81b8f51 [bpf] > ffffffffa0511000 t bpf_prog_6deef7357e7b4530 [bpf] > ffffffffa0542000 t bpf_trampoline_13832 [bpf] > ffffffffa0548000 t bpf_prog_96f1b5bf4e4cc6dc_mutex_lock [bpf] > ffffffffa0572000 t bpf_prog_d1c63e29ad82c4ab_bpf_prog1 [bpf] > ffffffffa0585000 t bpf_prog_e314084d332a5338__dissect [bpf] > ffffffffa0587000 t bpf_prog_59785a79eac7e5d2_mutex_unlock [bpf] > ffffffffa0589000 t bpf_prog_d0db6e0cac050163_mutex_lock [bpf] > ffffffffa058d000 t bpf_prog_d8f047721e4d8321_bpf_prog2 [bpf] > ffffffffa05df000 t bpf_trampoline_25637 [bpf] > ffffffffa05e3000 t bpf_prog_d8f047721e4d8321_bpf_prog2 [bpf] > ffffffffa05e5000 t bpf_prog_3b185187f1855c4c [bpf] > ffffffffa05e7000 t bpf_prog_d8f047721e4d8321_bpf_prog2 [bpf] > ffffffffa05eb000 t bpf_prog_93cebb259dd5c4b2_do_sys_open [bpf] > ffffffffa0677000 t bpf_dispatcher_xdp [bpf] > > thanks, > jirka > > > --- > Björn Töpel (1): > bpf: Add bpf_trampoline_ name prefix for DECLARE_BPF_DISPATCHER > > Jiri Olsa (13): > x86/mm: Rename is_kernel_text to __is_kernel_text > bpf: Add struct bpf_ksym > bpf: Add name to struct bpf_ksym > bpf: Add lnode list node to struct bpf_ksym > bpf: Add bpf_kallsyms_tree tree > bpf: Move bpf_tree add/del from bpf_prog_ksym_node_add/del > bpf: Separate kallsyms add/del functions > bpf: Add bpf_ksym_add/del functions > bpf: Re-initialize lnode in bpf_ksym_del > bpf: Rename bpf_tree to bpf_progs_tree > bpf: Add trampolines to kallsyms > bpf: Add dispatchers to kallsyms > bpf: Sort bpf kallsyms symbols > > arch/x86/mm/init_32.c | 14 ++++++---- > include/linux/bpf.h | 54 ++++++++++++++++++++++++++------------ > include/linux/filter.h | 13 +++------- > kernel/bpf/core.c | 182 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------- > kernel/bpf/dispatcher.c | 6 +++++ > kernel/bpf/trampoline.c | 23 ++++++++++++++++ > kernel/events/core.c | 4 +-- > net/core/filter.c | 5 ++-- > 8 files changed, 219 insertions(+), 82 deletions(-) >
Em Mon, Feb 10, 2020 at 05:17:51PM +0100, Jiri Olsa escreveu: > On Mon, Feb 10, 2020 at 04:51:08PM +0100, Björn Töpel wrote: > > On Sat, 8 Feb 2020 at 16:42, Jiri Olsa <jolsa@kernel.org> wrote: > > > this patchset adds trampoline and dispatcher objects > > > to be visible in /proc/kallsyms. The last patch also > > > adds sorting for all bpf objects in /proc/kallsyms. > > Thanks for working on this! > > I'm probably missing something with my perf setup; I've applied your > > patches, and everything seem to work fine from an kallsyms > > perspective: > > # grep bpf_dispatcher_xdp /proc/kallsyms > > ... > > ffffffffc0511000 t bpf_dispatcher_xdp [bpf] > > > > However, when I run > > # perf top > > > > I still see the undecorated one: > > 0.90% [unknown] [k] 0xffffffffc0511037 > > > > Any ideas? > yea strange.. it should be picked up from /proc/kallsyms as > fallback if there's no other source, I'll check on that > (might be the problem with perf depending on address going > only higher in /proc/kallsyms, while bpf symbols are at the > end and start over from the lowest bpf address) > > anyway, in perf we enumerate bpf_progs via the perf events > PERF_BPF_EVENT_PROG_LOAD,PERF_BPF_EVENT_PROG_UNLOAD interface > together with PERF_RECORD_KSYMBOL_TYPE_BPF events > > we might need to add something like: > PERF_RECORD_KSYMBOL_TYPE_BPF_TRAMPOLINE > PERF_RECORD_KSYMBOL_TYPE_BPF_DISPATCHER > > to notify about the area, I'll check on that > > however the /proc/kallsyms fallback should work in any > case.. thanks for report ;-) We should by now move kallsyms to be the preferred source of symbols, not vmlinux, right? Perhaps what is happening is: [root@quaco ~]# strace -f -e open,openat -o /tmp/bla perf top [root@quaco ~]# grep vmlinux /tmp/bla 11013 openat(AT_FDCWD, "vmlinux", O_RDONLY) = -1 ENOENT (No such file or directory) 11013 openat(AT_FDCWD, "/boot/vmlinux", O_RDONLY) = -1 ENOENT (No such file or directory) 11013 openat(AT_FDCWD, "/boot/vmlinux-5.5.0+", O_RDONLY) = -1 ENOENT (No such file or directory) 11013 openat(AT_FDCWD, "/usr/lib/debug/boot/vmlinux-5.5.0+", O_RDONLY) = -1 ENOENT (No such file or directory) 11013 openat(AT_FDCWD, "/lib/modules/5.5.0+/build/vmlinux", O_RDONLY) = 152 [root@quaco ~]# I.e. it is using vmlinux for resolving symbols and he should try with: [root@quaco ~]# strace -f -e open,openat -o /tmp/bla perf top --ignore-vmlinux [root@quaco ~]# perf top -h vmlinux Usage: perf top [<options>] -k, --vmlinux <file> vmlinux pathname --ignore-vmlinux don't load vmlinux even if found [root@quaco ~]# grep vmlinux /tmp/bla [root@quaco ~]# Historically vmlinux was preferred because it contains function sizes, but with all these out of the blue symbols, we need to prefer starting with /proc/kallsyms and, as we do now, continue getting updates via PERF_RECORD_KSYMBOL. Humm, but then trampolines don't generate that, right? Or does it? If it doesn't, then we will know about just the trampolines in place when the record/top session starts, reparsing /proc/kallsyms periodically seems excessive? - Arnaldo
On Tue, Feb 11, 2020 at 04:13:47PM -0300, Arnaldo Carvalho de Melo wrote: > Em Sat, Feb 08, 2020 at 04:41:55PM +0100, Jiri Olsa escreveu: > > hi, > > this patchset adds trampoline and dispatcher objects > > to be visible in /proc/kallsyms. The last patch also > > adds sorting for all bpf objects in /proc/kallsyms. > > This will allow those to appear in profiles, right? That would be yea, one would think so.. but as you saw in the other email there are still some issues ;-) > interesting to explicitely state, i.e. the _why_ of this patch, not just > the _what_. I guess another reason would be accountability of the kernel space, so that everything with the symbol would appear in /proc/kallsyms jirka > > Thanks, > > - Arnaldo > > > $ sudo cat /proc/kallsyms | tail -20 > > ... > > ffffffffa050f000 t bpf_prog_5a2b06eab81b8f51 [bpf] > > ffffffffa0511000 t bpf_prog_6deef7357e7b4530 [bpf] > > ffffffffa0542000 t bpf_trampoline_13832 [bpf] > > ffffffffa0548000 t bpf_prog_96f1b5bf4e4cc6dc_mutex_lock [bpf] > > ffffffffa0572000 t bpf_prog_d1c63e29ad82c4ab_bpf_prog1 [bpf] > > ffffffffa0585000 t bpf_prog_e314084d332a5338__dissect [bpf] > > ffffffffa0587000 t bpf_prog_59785a79eac7e5d2_mutex_unlock [bpf] > > ffffffffa0589000 t bpf_prog_d0db6e0cac050163_mutex_lock [bpf] > > ffffffffa058d000 t bpf_prog_d8f047721e4d8321_bpf_prog2 [bpf] > > ffffffffa05df000 t bpf_trampoline_25637 [bpf] > > ffffffffa05e3000 t bpf_prog_d8f047721e4d8321_bpf_prog2 [bpf] > > ffffffffa05e5000 t bpf_prog_3b185187f1855c4c [bpf] > > ffffffffa05e7000 t bpf_prog_d8f047721e4d8321_bpf_prog2 [bpf] > > ffffffffa05eb000 t bpf_prog_93cebb259dd5c4b2_do_sys_open [bpf] > > ffffffffa0677000 t bpf_dispatcher_xdp [bpf] > > > > thanks, > > jirka > > > > > > --- > > Björn Töpel (1): > > bpf: Add bpf_trampoline_ name prefix for DECLARE_BPF_DISPATCHER > > > > Jiri Olsa (13): > > x86/mm: Rename is_kernel_text to __is_kernel_text > > bpf: Add struct bpf_ksym > > bpf: Add name to struct bpf_ksym > > bpf: Add lnode list node to struct bpf_ksym > > bpf: Add bpf_kallsyms_tree tree > > bpf: Move bpf_tree add/del from bpf_prog_ksym_node_add/del > > bpf: Separate kallsyms add/del functions > > bpf: Add bpf_ksym_add/del functions > > bpf: Re-initialize lnode in bpf_ksym_del > > bpf: Rename bpf_tree to bpf_progs_tree > > bpf: Add trampolines to kallsyms > > bpf: Add dispatchers to kallsyms > > bpf: Sort bpf kallsyms symbols > > > > arch/x86/mm/init_32.c | 14 ++++++---- > > include/linux/bpf.h | 54 ++++++++++++++++++++++++++------------ > > include/linux/filter.h | 13 +++------- > > kernel/bpf/core.c | 182 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------- > > kernel/bpf/dispatcher.c | 6 +++++ > > kernel/bpf/trampoline.c | 23 ++++++++++++++++ > > kernel/events/core.c | 4 +-- > > net/core/filter.c | 5 ++-- > > 8 files changed, 219 insertions(+), 82 deletions(-) > > > > -- > > - Arnaldo >
On Tue, Feb 11, 2020 at 04:32:23PM -0300, Arnaldo Carvalho de Melo wrote: > Em Mon, Feb 10, 2020 at 05:17:51PM +0100, Jiri Olsa escreveu: > > On Mon, Feb 10, 2020 at 04:51:08PM +0100, Björn Töpel wrote: > > > On Sat, 8 Feb 2020 at 16:42, Jiri Olsa <jolsa@kernel.org> wrote: > > > > this patchset adds trampoline and dispatcher objects > > > > to be visible in /proc/kallsyms. The last patch also > > > > adds sorting for all bpf objects in /proc/kallsyms. > > > > Thanks for working on this! > > > > I'm probably missing something with my perf setup; I've applied your > > > patches, and everything seem to work fine from an kallsyms > > > perspective: > > > > # grep bpf_dispatcher_xdp /proc/kallsyms > > > ... > > > ffffffffc0511000 t bpf_dispatcher_xdp [bpf] > > > > > > However, when I run > > > # perf top > > > > > > I still see the undecorated one: > > > 0.90% [unknown] [k] 0xffffffffc0511037 > > > > > > Any ideas? > > > yea strange.. it should be picked up from /proc/kallsyms as > > fallback if there's no other source, I'll check on that > > (might be the problem with perf depending on address going > > only higher in /proc/kallsyms, while bpf symbols are at the > > end and start over from the lowest bpf address) > > > > anyway, in perf we enumerate bpf_progs via the perf events > > PERF_BPF_EVENT_PROG_LOAD,PERF_BPF_EVENT_PROG_UNLOAD interface > > together with PERF_RECORD_KSYMBOL_TYPE_BPF events > > > > we might need to add something like: > > PERF_RECORD_KSYMBOL_TYPE_BPF_TRAMPOLINE > > PERF_RECORD_KSYMBOL_TYPE_BPF_DISPATCHER > > > > to notify about the area, I'll check on that > > > > however the /proc/kallsyms fallback should work in any > > case.. thanks for report ;-) > > We should by now move kallsyms to be the preferred source of symbols, > not vmlinux, right? > > Perhaps what is happening is: > > [root@quaco ~]# strace -f -e open,openat -o /tmp/bla perf top > [root@quaco ~]# grep vmlinux /tmp/bla > 11013 openat(AT_FDCWD, "vmlinux", O_RDONLY) = -1 ENOENT (No such file or directory) > 11013 openat(AT_FDCWD, "/boot/vmlinux", O_RDONLY) = -1 ENOENT (No such file or directory) > 11013 openat(AT_FDCWD, "/boot/vmlinux-5.5.0+", O_RDONLY) = -1 ENOENT (No such file or directory) > 11013 openat(AT_FDCWD, "/usr/lib/debug/boot/vmlinux-5.5.0+", O_RDONLY) = -1 ENOENT (No such file or directory) > 11013 openat(AT_FDCWD, "/lib/modules/5.5.0+/build/vmlinux", O_RDONLY) = 152 > [root@quaco ~]# > > I.e. it is using vmlinux for resolving symbols and he should try with: > > [root@quaco ~]# strace -f -e open,openat -o /tmp/bla perf top --ignore-vmlinux > [root@quaco ~]# perf top -h vmlinux > > Usage: perf top [<options>] > > -k, --vmlinux <file> vmlinux pathname > --ignore-vmlinux don't load vmlinux even if found > > [root@quaco ~]# grep vmlinux /tmp/bla > [root@quaco ~]# > > Historically vmlinux was preferred because it contains function sizes, > but with all these out of the blue symbols, we need to prefer starting > with /proc/kallsyms and, as we do now, continue getting updates via > PERF_RECORD_KSYMBOL. > > Humm, but then trampolines don't generate that, right? Or does it? If it > doesn't, then we will know about just the trampolines in place when the > record/top session starts, reparsing /proc/kallsyms periodically seems > excessive? I plan to extend the KSYMBOL interface to contain trampolines/dispatcher data, plus we could do some inteligent fallback to /proc/kallsyms in case vmlinux won't have anything jirka
Em Wed, Feb 12, 2020 at 12:13:46PM +0100, Jiri Olsa escreveu: > On Tue, Feb 11, 2020 at 04:32:23PM -0300, Arnaldo Carvalho de Melo wrote: > > Historically vmlinux was preferred because it contains function sizes, > > but with all these out of the blue symbols, we need to prefer starting > > with /proc/kallsyms and, as we do now, continue getting updates via > > PERF_RECORD_KSYMBOL. > > Humm, but then trampolines don't generate that, right? Or does it? If it > > doesn't, then we will know about just the trampolines in place when the > > record/top session starts, reparsing /proc/kallsyms periodically seems > > excessive? > I plan to extend the KSYMBOL interface to contain trampolines/dispatcher > data, That seems like the sensible, without looking too much at all the details, to do, yes. > plus we could do some inteligent fallback to /proc/kallsyms in case > vmlinux won't have anything At this point what would be the good reason to prefer vmlinux instead of going straight to using /proc/kallsyms? We have support for taking a snapshot of it at 'perf top' start, i.e. right at the point we need to resolve a kernel symbol, then we get PERF_RECORD_KSYMBOL for things that gets in place after that. And as well we save it to the build-id cache so that later, at 'perf report/script' time we can resolve kernel symbols, etc. vmlinux is just what is in there right before boot, after that, for quite some time, _lots_ of stuff happens :-) - Arnaldo
On Wed, Feb 12, 2020 at 10:31:25AM -0300, Arnaldo Carvalho de Melo wrote: > Em Wed, Feb 12, 2020 at 12:13:46PM +0100, Jiri Olsa escreveu: > > On Tue, Feb 11, 2020 at 04:32:23PM -0300, Arnaldo Carvalho de Melo wrote: > > > Historically vmlinux was preferred because it contains function sizes, > > > but with all these out of the blue symbols, we need to prefer starting > > > with /proc/kallsyms and, as we do now, continue getting updates via > > > PERF_RECORD_KSYMBOL. > > > > Humm, but then trampolines don't generate that, right? Or does it? If it > > > doesn't, then we will know about just the trampolines in place when the > > > record/top session starts, reparsing /proc/kallsyms periodically seems > > > excessive? > > > I plan to extend the KSYMBOL interface to contain trampolines/dispatcher > > data, > > That seems like the sensible, without looking too much at all the > details, to do, yes. > > > plus we could do some inteligent fallback to /proc/kallsyms in case > > vmlinux won't have anything > > At this point what would be the good reason to prefer vmlinux instead of > going straight to using /proc/kallsyms? symbol (with sizes) and code for dwarf unwind, processor trace jirka > > We have support for taking a snapshot of it at 'perf top' start, i.e. > right at the point we need to resolve a kernel symbol, then we get > PERF_RECORD_KSYMBOL for things that gets in place after that. > > And as well we save it to the build-id cache so that later, at 'perf > report/script' time we can resolve kernel symbols, etc. > > vmlinux is just what is in there right before boot, after that, for > quite some time, _lots_ of stuff happens :-) > > - Arnaldo >
On Mon, Feb 10, 2020 at 04:51:08PM +0100, Björn Töpel wrote: > On Sat, 8 Feb 2020 at 16:42, Jiri Olsa <jolsa@kernel.org> wrote: > > > > hi, > > this patchset adds trampoline and dispatcher objects > > to be visible in /proc/kallsyms. The last patch also > > adds sorting for all bpf objects in /proc/kallsyms. > > > > Thanks for working on this! > > I'm probably missing something with my perf setup; I've applied your > patches, and everything seem to work fine from an kallsyms > perspective: > > # grep bpf_dispatcher_xdp /proc/kallsyms > ... > ffffffffc0511000 t bpf_dispatcher_xdp [bpf] > > However, when I run > # perf top > > I still see the undecorated one: > 0.90% [unknown] [k] 0xffffffffc0511037 > > Any ideas? heya, the code is little rusty and needs some fixing :-\ with the patch below on top of Arnaldo's perf/urgent branch, there's one workaround for now: # perf record --vmlinux /proc/kallsyms ^C[ perf record: Woken up 0 times to write data ] [ perf record: Captured and wrote 18.954 MB perf.data (348693 samples) ] # perf report --kallsyms /proc/kallsyms | grep bpf_trampoline_13795 0.01% sched-messaging kallsyms [k] bpf_trampoline_13795 0.00% perf kallsyms [k] bpf_trampoline_13795 0.00% :47547 kallsyms [k] bpf_trampoline_13795 0.00% :47546 kallsyms [k] bpf_trampoline_13795 0.00% :47544 kallsyms [k] bpf_trampoline_13795 with recent kcore/vmlinux changes we neglected kallsyms fallback, I'm preparing changes that will detect and use it automaticaly jirka --- diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index fb5c2cd44d30..463ada5117f8 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -742,6 +742,7 @@ static int machine__process_ksymbol_register(struct machine *machine, map->start = event->ksymbol.addr; map->end = map->start + event->ksymbol.len; maps__insert(&machine->kmaps, map); + dso__set_loaded(dso); } sym = symbol__new(map->map_ip(map, map->start),