Re: [PATCH v11 perf, bpf-next 7/9] perf tools: synthesize PERF_RECORD_* for loaded BPF programs

On Jan 22, 2019, at 11:15 AM, Jiri Olsa [off-list ref] wrote:

On Tue, Jan 22, 2019 at 06:38:56PM +0000, Song Liu wrote:

SNIP

quoted

quoted

quoted

in perf_session__process_event, this happens right when processing
buildids in 'perf record', and also in 'perf report', so that is
something badly synthesized that hits perf.data for PERF_RECORD_KSYMBOL.

it's reproducible with simple:
perf record -e cycles,instructions ls

as you said on irc, it's the machine->id_hdr_size size missing
there's one more glitch, attached patch fixes that for me
you can't use sizeof(struct ksymbol_event), because it includes
the name as well.. which screws the size

but I don't know that code that much.. might be still something
missing

jirka

Hi Arnaldo and Jiri,

Thanks for catching and fixing the bug. 

I guess the following is OK?

       *bpf_event = (struct bpf_event){
               .header = {
                       .type = PERF_RECORD_BPF_EVENT,
                       .size = sizeof(struct bpf_event),
               },
               .type = PERF_BPF_EVENT_PROG_LOAD,
               .flags = 0,
               .id = info.id,
       };

as struct bpf_event doesn't have variable length name:

       struct bpf_event {
               struct perf_event_header header;
               u16 type;
               u16 flags;
               u32 id;

               /* for bpf_prog types */
               u8 tag[BPF_TAG_SIZE];  // prog tag
       };

Or we need similar fix? 

yep, looks good.. also don't forget to add the 'machine->id_hdr_size'

jirka

So we still need something like?

diff --git i/tools/perf/util/bpf-event.c w/tools/perf/util/bpf-event.c
index 01e1dc1bb7fb..d7bf45485820 100644
--- i/tools/perf/util/bpf-event.c
+++ w/tools/perf/util/bpf-event.c

@@ -4,6 +4,7 @@
 #include <bpf/bpf.h>
 #include <bpf/btf.h>
 #include <linux/btf.h>
+#include "machine.h"
 #include "bpf-event.h"
 #include "debug.h"
 #include "symbol.h"

@@ -187,7 +188,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
                *bpf_event = (struct bpf_event){
                        .header = {
                                .type = PERF_RECORD_BPF_EVENT,
-                               .size = sizeof(struct bpf_event),
+                               .size = sizeof(struct bpf_event) + machine->id_hdr_size,
                        },
                        .type = PERF_BPF_EVENT_PROG_LOAD,
                        .flags = 0,

Would you send the official patch? Or would you prefer me sending it?

Thanks,
Song