Re: [PATCH v4 perf,bpf 06/15] perf, bpf: save bpf_prog_info in a rbtree in perf_env

From: Song Liu
Date: Wed Feb 27 2019 - 12:39:27 EST




> On Feb 27, 2019, at 5:21 AM, Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> On Mon, Feb 25, 2019 at 04:20:10PM -0800, Song Liu wrote:
>> bpf_prog_info contains information necessary to annotate bpf programs.
>> This patch saves bpf_prog_info for bpf programs loaded in the system.
>>
>> Signed-off-by: Song Liu <songliubraving@xxxxxx>
>> ---
>> tools/perf/util/bpf-event.c | 32 +++++++++++++-
>> tools/perf/util/bpf-event.h | 7 ++-
>> tools/perf/util/env.c | 85 +++++++++++++++++++++++++++++++++++++
>> tools/perf/util/env.h | 17 ++++++++
>> 4 files changed, 139 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
>> index ff7ee149ec46..ce81b2c43a51 100644
>> --- a/tools/perf/util/bpf-event.c
>> +++ b/tools/perf/util/bpf-event.c
>> @@ -10,6 +10,7 @@
>> #include "debug.h"
>> #include "symbol.h"
>> #include "machine.h"
>> +#include "env.h"
>> #include "session.h"
>>
>> #define ptr_to_u64(ptr) ((__u64)(unsigned long)(ptr))
>> @@ -54,17 +55,28 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
>> struct bpf_event *bpf_event = &event->bpf_event;
>> struct bpf_prog_info_linear *info_linear;
>> struct perf_tool *tool = session->tool;
>> + struct bpf_prog_info_node *info_node;
>> struct bpf_prog_info *info;
>> struct btf *btf = NULL;
>> bool has_btf = false;
>> + struct perf_env *env;
>> u32 sub_prog_cnt, i;
>> int err = 0;
>> u64 arrays;
>>
>> + /*
>> + * for perf-record and perf-report use header.env;
>> + * otherwise, use global perf_env.
>> + */
>> + env = session->data ? &session->header.env : &perf_env;
>> +
>> arrays = 1UL << BPF_PROG_INFO_JITED_KSYMS;
>> arrays |= 1UL << BPF_PROG_INFO_JITED_FUNC_LENS;
>> arrays |= 1UL << BPF_PROG_INFO_FUNC_INFO;
>> arrays |= 1UL << BPF_PROG_INFO_PROG_TAGS;
>> + arrays |= 1UL << BPF_PROG_INFO_JITED_INSNS;
>> + arrays |= 1UL << BPF_PROG_INFO_LINE_INFO;
>> + arrays |= 1UL << BPF_PROG_INFO_JITED_LINE_INFO;
>>
>> info_linear = bpf_program__get_prog_info_linear(fd, arrays);
>> if (IS_ERR_OR_NULL(info_linear)) {
>> @@ -153,8 +165,8 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
>> machine, process);
>> }
>>
>> - /* Synthesize PERF_RECORD_BPF_EVENT */
>> if (opts->bpf_event) {
>> + /* Synthesize PERF_RECORD_BPF_EVENT */
>> *bpf_event = (struct bpf_event){
>> .header = {
>> .type = PERF_RECORD_BPF_EVENT,
>> @@ -167,6 +179,24 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
>> memcpy(bpf_event->tag, info->tag, BPF_TAG_SIZE);
>> memset((void *)event + event->header.size, 0, machine->id_hdr_size);
>> event->header.size += machine->id_hdr_size;
>> +
>> + /* save bpf_prog_info to env */
>
> why do we save this to perf_env in here? we just
> synthesize the same data as event, so report and
> top will read it and fill perf_env again, right?

The synthesized events are same as PERF_RECORD_BPF_EVENT
PROG_LOAD. We need to process them and fill perf_env as
soon as we get PROG_LOAD. Otherwise, the program might
unload before we process them.

>
> could you please explain the whole flow of the
> bpf events and its respective data in perf_env
> and put it into the changelog

I will add more information to the change log.

>
>> + info_node = malloc(sizeof(struct bpf_prog_info_node));
>> +
>> + /*
>> + * Do not bail out for !info_node, as we still want to
>> + * call perf_tool__process_synth_event()
>
> well, we are out of memory, so I dont think perf_tool__process_synth_event
> will get too far.. also the perf_env data would be inconsistent with what
> you store as event.. how can that work?

It is OK we have PERF_RECORD_BPF_EVENT but not related
bpf_prog_info and btf. The perf.data file will show
a BPF program was loaded, but we won't be able to do
annotation. Does this make sense?

Thanks,
Song

>
> thanks,
> jirka
>
>> + */
>> + if (info_node) {
>> + info_node->info_linear = info_linear;
>> + perf_env__insert_bpf_prog_info(env, info_node);
>> + info_linear = NULL;
>> + }
>> +
>
> SNIP