| Message ID | 20190215000010.2590505-7-songliubraving@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | perf annotation of BPF programs | expand |
Em Thu, Feb 14, 2019 at 04:00:09PM -0800, Song Liu escreveu: > This patch enables perf-record to save btf information as headers to > perf.data A new header type HEADER_BTF is introduced for this data. Jiri, Wouldn't it be better for this HEADER_BTF to be introduced already as an user space event, Song, see: tools/perf/util/event.h and: tools/perf/util/event.c perf_event__synthesize_cpu_map() - Arnaldo > Signed-off-by: Song Liu <songliubraving@fb.com> > --- > tools/perf/util/header.c | 99 +++++++++++++++++++++++++++++++++++++++- > tools/perf/util/header.h | 1 + > 2 files changed, 99 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c > index 2ae76a9d06f6..3f1562afe8e5 100644 > --- a/tools/perf/util/header.c > +++ b/tools/perf/util/header.c > @@ -1125,6 +1125,45 @@ static int write_bpf_prog_info(struct feat_fd *ff, > return ret; > } > > +static int write_btf(struct feat_fd *ff, > + struct perf_evlist *evlist __maybe_unused) > +{ > + struct perf_env *env = &ff->ph->env; > + struct rb_root *root; > + struct rb_node *next; > + u32 count = 0; > + int ret; > + > + down_read(&env->bpf_info_lock); > + > + root = &env->btfs; > + next = rb_first(root); > + while (next) { > + ++count; > + next = rb_next(next); > + } > + > + ret = do_write(ff, &count, sizeof(count)); > + > + if (ret < 0) > + goto out; > + > + next = rb_first(root); > + while (next) { > + struct btf_node *node; > + > + node = rb_entry(next, struct btf_node, rb_node); > + next = rb_next(&node->rb_node); > + ret = do_write(ff, node, > + sizeof(struct btf_node) + node->data_size); > + if (ret < 0) > + goto out; > + } > +out: > + up_read(&env->bpf_info_lock); > + return ret; > +} > + > static int cpu_cache_level__sort(const void *a, const void *b) > { > struct cpu_cache_level *cache_a = (struct cpu_cache_level *)a; > @@ -1628,6 +1667,28 @@ static void print_bpf_prog_info(struct feat_fd *ff, FILE *fp) > up_read(&env->bpf_info_lock); > } > > +static void print_btf(struct feat_fd *ff, FILE *fp) > +{ > + struct perf_env *env = &ff->ph->env; > + struct rb_root *root; > + struct rb_node *next; > + > + down_read(&env->bpf_info_lock); > + > + root = &env->btfs; > + next = rb_first(root); > + > + while (next) { > + struct btf_node *node; > + > + node = rb_entry(next, struct btf_node, rb_node); > + next = rb_next(&node->rb_node); > + fprintf(fp, "# bpf_prog_info of id %u\n", node->id); > + } > + > + up_read(&env->bpf_info_lock); > +} > + > static void free_event_desc(struct perf_evsel *events) > { > struct perf_evsel *evsel; > @@ -2723,6 +2784,41 @@ static int process_bpf_prog_info(struct feat_fd *ff, > return err; > } > > +static int process_btf(struct feat_fd *ff, void *data __maybe_unused) > +{ > + struct perf_env *env = &ff->ph->env; > + u32 count, i; > + > + if (do_read_u32(ff, &count)) > + return -1; > + > + down_write(&env->bpf_info_lock); > + > + for (i = 0; i < count; ++i) { > + struct btf_node btf_node; > + struct btf_node *node; > + > + if (__do_read(ff, &btf_node, sizeof(struct btf_node))) > + return -1; > + > + node = malloc(sizeof(struct btf_node) + btf_node.data_size); > + if (!node) > + return -1; > + > + node->id = btf_node.id; > + node->data_size = btf_node.data_size; > + > + if (__do_read(ff, node->data, btf_node.data_size)) { > + free(node); > + return -1; > + } > + perf_env__insert_btf(env, node); > + } > + > + up_write(&env->bpf_info_lock); > + return 0; > +} > + > struct feature_ops { > int (*write)(struct feat_fd *ff, struct perf_evlist *evlist); > void (*print)(struct feat_fd *ff, FILE *fp); > @@ -2783,7 +2879,8 @@ static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = { > FEAT_OPR(SAMPLE_TIME, sample_time, false), > FEAT_OPR(MEM_TOPOLOGY, mem_topology, true), > FEAT_OPR(CLOCKID, clockid, false), > - FEAT_OPR(BPF_PROG_INFO, bpf_prog_info, false) > + FEAT_OPR(BPF_PROG_INFO, bpf_prog_info, false), > + FEAT_OPR(BTF, btf, false) > }; > > struct header_print_data { > diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h > index 0785c91b4c3a..ba51d8e43c53 100644 > --- a/tools/perf/util/header.h > +++ b/tools/perf/util/header.h > @@ -40,6 +40,7 @@ enum { > HEADER_MEM_TOPOLOGY, > HEADER_CLOCKID, > HEADER_BPF_PROG_INFO, > + HEADER_BTF, > HEADER_LAST_FEATURE, > HEADER_FEAT_BITS = 256, > }; > -- > 2.17.1
> On Feb 15, 2019, at 6:26 AM, Arnaldo Carvalho de Melo <acme@redhat.com> wrote: > > Em Thu, Feb 14, 2019 at 04:00:09PM -0800, Song Liu escreveu: >> This patch enables perf-record to save btf information as headers to >> perf.data A new header type HEADER_BTF is introduced for this data. > > Jiri, > > Wouldn't it be better for this HEADER_BTF to be introduced > already as an user space event, Song, see: > > tools/perf/util/event.h > > and: > > tools/perf/util/event.c > > perf_event__synthesize_cpu_map() > > - Arnaldo > BTF would be short living for short living BPF programs. I guess saving them as header is easier than merging them with samples. What's the benefit of saving them as user space events? Thanks, Song > >> Signed-off-by: Song Liu <songliubraving@fb.com> >> --- >> tools/perf/util/header.c | 99 +++++++++++++++++++++++++++++++++++++++- >> tools/perf/util/header.h | 1 + >> 2 files changed, 99 insertions(+), 1 deletion(-) >> >> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c >> index 2ae76a9d06f6..3f1562afe8e5 100644 >> --- a/tools/perf/util/header.c >> +++ b/tools/perf/util/header.c >> @@ -1125,6 +1125,45 @@ static int write_bpf_prog_info(struct feat_fd *ff, >> return ret; >> } >> >> +static int write_btf(struct feat_fd *ff, >> + struct perf_evlist *evlist __maybe_unused) >> +{ >> + struct perf_env *env = &ff->ph->env; >> + struct rb_root *root; >> + struct rb_node *next; >> + u32 count = 0; >> + int ret; >> + >> + down_read(&env->bpf_info_lock); >> + >> + root = &env->btfs; >> + next = rb_first(root); >> + while (next) { >> + ++count; >> + next = rb_next(next); >> + } >> + >> + ret = do_write(ff, &count, sizeof(count)); >> + >> + if (ret < 0) >> + goto out; >> + >> + next = rb_first(root); >> + while (next) { >> + struct btf_node *node; >> + >> + node = rb_entry(next, struct btf_node, rb_node); >> + next = rb_next(&node->rb_node); >> + ret = do_write(ff, node, >> + sizeof(struct btf_node) + node->data_size); >> + if (ret < 0) >> + goto out; >> + } >> +out: >> + up_read(&env->bpf_info_lock); >> + return ret; >> +} >> + >> static int cpu_cache_level__sort(const void *a, const void *b) >> { >> struct cpu_cache_level *cache_a = (struct cpu_cache_level *)a; >> @@ -1628,6 +1667,28 @@ static void print_bpf_prog_info(struct feat_fd *ff, FILE *fp) >> up_read(&env->bpf_info_lock); >> } >> >> +static void print_btf(struct feat_fd *ff, FILE *fp) >> +{ >> + struct perf_env *env = &ff->ph->env; >> + struct rb_root *root; >> + struct rb_node *next; >> + >> + down_read(&env->bpf_info_lock); >> + >> + root = &env->btfs; >> + next = rb_first(root); >> + >> + while (next) { >> + struct btf_node *node; >> + >> + node = rb_entry(next, struct btf_node, rb_node); >> + next = rb_next(&node->rb_node); >> + fprintf(fp, "# bpf_prog_info of id %u\n", node->id); >> + } >> + >> + up_read(&env->bpf_info_lock); >> +} >> + >> static void free_event_desc(struct perf_evsel *events) >> { >> struct perf_evsel *evsel; >> @@ -2723,6 +2784,41 @@ static int process_bpf_prog_info(struct feat_fd *ff, >> return err; >> } >> >> +static int process_btf(struct feat_fd *ff, void *data __maybe_unused) >> +{ >> + struct perf_env *env = &ff->ph->env; >> + u32 count, i; >> + >> + if (do_read_u32(ff, &count)) >> + return -1; >> + >> + down_write(&env->bpf_info_lock); >> + >> + for (i = 0; i < count; ++i) { >> + struct btf_node btf_node; >> + struct btf_node *node; >> + >> + if (__do_read(ff, &btf_node, sizeof(struct btf_node))) >> + return -1; >> + >> + node = malloc(sizeof(struct btf_node) + btf_node.data_size); >> + if (!node) >> + return -1; >> + >> + node->id = btf_node.id; >> + node->data_size = btf_node.data_size; >> + >> + if (__do_read(ff, node->data, btf_node.data_size)) { >> + free(node); >> + return -1; >> + } >> + perf_env__insert_btf(env, node); >> + } >> + >> + up_write(&env->bpf_info_lock); >> + return 0; >> +} >> + >> struct feature_ops { >> int (*write)(struct feat_fd *ff, struct perf_evlist *evlist); >> void (*print)(struct feat_fd *ff, FILE *fp); >> @@ -2783,7 +2879,8 @@ static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = { >> FEAT_OPR(SAMPLE_TIME, sample_time, false), >> FEAT_OPR(MEM_TOPOLOGY, mem_topology, true), >> FEAT_OPR(CLOCKID, clockid, false), >> - FEAT_OPR(BPF_PROG_INFO, bpf_prog_info, false) >> + FEAT_OPR(BPF_PROG_INFO, bpf_prog_info, false), >> + FEAT_OPR(BTF, btf, false) >> }; >> >> struct header_print_data { >> diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h >> index 0785c91b4c3a..ba51d8e43c53 100644 >> --- a/tools/perf/util/header.h >> +++ b/tools/perf/util/header.h >> @@ -40,6 +40,7 @@ enum { >> HEADER_MEM_TOPOLOGY, >> HEADER_CLOCKID, >> HEADER_BPF_PROG_INFO, >> + HEADER_BTF, >> HEADER_LAST_FEATURE, >> HEADER_FEAT_BITS = 256, >> }; >> -- >> 2.17.1
Em Fri, Feb 15, 2019 at 05:25:01PM +0000, Song Liu escreveu: > > On Feb 15, 2019, at 6:26 AM, Arnaldo Carvalho de Melo <acme@redhat.com> wrote: > > Em Thu, Feb 14, 2019 at 04:00:09PM -0800, Song Liu escreveu: > >> This patch enables perf-record to save btf information as headers to > >> perf.data A new header type HEADER_BTF is introduced for this data. > > Wouldn't it be better for this HEADER_BTF to be introduced > > already as an user space event, Song, see: > > tools/perf/util/event.h > > and: > > tools/perf/util/event.c > > perf_event__synthesize_cpu_map() > BTF would be short living for short living BPF programs. I guess > saving them as header is easier than merging them with samples. > What's the benefit of saving them as user space events? When we work with pipe mode, i.e.: perf record -o - | perf report -i - and other combinations (with 'perf script', 'perf inject', etc), we need a way to pass the headers to the other side, and the way was via user space events. This is something Stephane and Jiri have been discussing recently, probably they have more justifications, Stephane, Jiri? - Arnaldo
> On Feb 15, 2019, at 9:40 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Fri, Feb 15, 2019 at 05:25:01PM +0000, Song Liu escreveu: >>> On Feb 15, 2019, at 6:26 AM, Arnaldo Carvalho de Melo <acme@redhat.com> wrote: >>> Em Thu, Feb 14, 2019 at 04:00:09PM -0800, Song Liu escreveu: >>>> This patch enables perf-record to save btf information as headers to >>>> perf.data A new header type HEADER_BTF is introduced for this data. > >>> Wouldn't it be better for this HEADER_BTF to be introduced >>> already as an user space event, Song, see: > >>> tools/perf/util/event.h > >>> and: > >>> tools/perf/util/event.c > >>> perf_event__synthesize_cpu_map() > >> BTF would be short living for short living BPF programs. I guess >> saving them as header is easier than merging them with samples. > >> What's the benefit of saving them as user space events? > > When we work with pipe mode, i.e.: > > perf record -o - | perf report -i - > > and other combinations (with 'perf script', 'perf inject', etc), we need > a way to pass the headers to the other side, and the way was via user > space events. > > This is something Stephane and Jiri have been discussing recently, > probably they have more justifications, Stephane, Jiri? > > - Arnaldo I see. In this case, we will need some synchronization between main thread and the polling thread, as they are both writing to the same pipe. Thanks, Song
Em Fri, Feb 15, 2019 at 05:47:58PM +0000, Song Liu escreveu: > > > > On Feb 15, 2019, at 9:40 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > > Em Fri, Feb 15, 2019 at 05:25:01PM +0000, Song Liu escreveu: > >>> On Feb 15, 2019, at 6:26 AM, Arnaldo Carvalho de Melo <acme@redhat.com> wrote: > >>> Em Thu, Feb 14, 2019 at 04:00:09PM -0800, Song Liu escreveu: > >>>> This patch enables perf-record to save btf information as headers to > >>>> perf.data A new header type HEADER_BTF is introduced for this data. > > > >>> Wouldn't it be better for this HEADER_BTF to be introduced > >>> already as an user space event, Song, see: > > > >>> tools/perf/util/event.h > > > >>> and: > > > >>> tools/perf/util/event.c > > > >>> perf_event__synthesize_cpu_map() > > > >> BTF would be short living for short living BPF programs. I guess > >> saving them as header is easier than merging them with samples. > > > >> What's the benefit of saving them as user space events? > > > > When we work with pipe mode, i.e.: > > > > perf record -o - | perf report -i - > > > > and other combinations (with 'perf script', 'perf inject', etc), we need > > a way to pass the headers to the other side, and the way was via user > > space events. > > > > This is something Stephane and Jiri have been discussing recently, > > probably they have more justifications, Stephane, Jiri? > > > > - Arnaldo > > I see. In this case, we will need some synchronization between main > thread and the polling thread, as they are both writing to the same > pipe. So, the whole context is that we need to have 'perf record' to start a thread per CPU and then read the already per-cpu mmap buffers in the matching thread, with the right affinity, numa settings to have the record phase not cause contention, etc, so it ends up dumping one stream per CPU in a separate file in a 'perf.data' directory instead of a perf.data file. Jiri is working on that, so, if you dump one more stream into that directory, it would, at post processing time, be ordered together with the other stream, the per-cpu ones. - Arnaldo
> On Feb 15, 2019, at 10:20 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Fri, Feb 15, 2019 at 05:47:58PM +0000, Song Liu escreveu: >> >> >>> On Feb 15, 2019, at 9:40 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote: >>> >>> Em Fri, Feb 15, 2019 at 05:25:01PM +0000, Song Liu escreveu: >>>>> On Feb 15, 2019, at 6:26 AM, Arnaldo Carvalho de Melo <acme@redhat.com> wrote: >>>>> Em Thu, Feb 14, 2019 at 04:00:09PM -0800, Song Liu escreveu: >>>>>> This patch enables perf-record to save btf information as headers to >>>>>> perf.data A new header type HEADER_BTF is introduced for this data. >>> >>>>> Wouldn't it be better for this HEADER_BTF to be introduced >>>>> already as an user space event, Song, see: >>> >>>>> tools/perf/util/event.h >>> >>>>> and: >>> >>>>> tools/perf/util/event.c >>> >>>>> perf_event__synthesize_cpu_map() >>> >>>> BTF would be short living for short living BPF programs. I guess >>>> saving them as header is easier than merging them with samples. >>> >>>> What's the benefit of saving them as user space events? >>> >>> When we work with pipe mode, i.e.: >>> >>> perf record -o - | perf report -i - >>> >>> and other combinations (with 'perf script', 'perf inject', etc), we need >>> a way to pass the headers to the other side, and the way was via user >>> space events. >>> >>> This is something Stephane and Jiri have been discussing recently, >>> probably they have more justifications, Stephane, Jiri? >>> >>> - Arnaldo >> >> I see. In this case, we will need some synchronization between main >> thread and the polling thread, as they are both writing to the same >> pipe. > > So, the whole context is that we need to have 'perf record' to start a > thread per CPU and then read the already per-cpu mmap buffers in the > matching thread, with the right affinity, numa settings to have the > record phase not cause contention, etc, so it ends up dumping one stream > per CPU in a separate file in a 'perf.data' directory instead of a > perf.data file. > > Jiri is working on that, so, if you dump one more stream into that > directory, it would, at post processing time, be ordered together with > the other stream, the per-cpu ones. > > - Arnaldo I see. This solution looks great. For this set, how about I keep this part as-is (at least for v3)? In this case, it will goes to the header file after Jiri's change. Once Jiri's work is done, I will move them into per-cpu files. Thanks, Song
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index 2ae76a9d06f6..3f1562afe8e5 100644 --- a/tools/perf/util/header.c +++ b/tools/perf/util/header.c @@ -1125,6 +1125,45 @@ static int write_bpf_prog_info(struct feat_fd *ff, return ret; } +static int write_btf(struct feat_fd *ff, + struct perf_evlist *evlist __maybe_unused) +{ + struct perf_env *env = &ff->ph->env; + struct rb_root *root; + struct rb_node *next; + u32 count = 0; + int ret; + + down_read(&env->bpf_info_lock); + + root = &env->btfs; + next = rb_first(root); + while (next) { + ++count; + next = rb_next(next); + } + + ret = do_write(ff, &count, sizeof(count)); + + if (ret < 0) + goto out; + + next = rb_first(root); + while (next) { + struct btf_node *node; + + node = rb_entry(next, struct btf_node, rb_node); + next = rb_next(&node->rb_node); + ret = do_write(ff, node, + sizeof(struct btf_node) + node->data_size); + if (ret < 0) + goto out; + } +out: + up_read(&env->bpf_info_lock); + return ret; +} + static int cpu_cache_level__sort(const void *a, const void *b) { struct cpu_cache_level *cache_a = (struct cpu_cache_level *)a; @@ -1628,6 +1667,28 @@ static void print_bpf_prog_info(struct feat_fd *ff, FILE *fp) up_read(&env->bpf_info_lock); } +static void print_btf(struct feat_fd *ff, FILE *fp) +{ + struct perf_env *env = &ff->ph->env; + struct rb_root *root; + struct rb_node *next; + + down_read(&env->bpf_info_lock); + + root = &env->btfs; + next = rb_first(root); + + while (next) { + struct btf_node *node; + + node = rb_entry(next, struct btf_node, rb_node); + next = rb_next(&node->rb_node); + fprintf(fp, "# bpf_prog_info of id %u\n", node->id); + } + + up_read(&env->bpf_info_lock); +} + static void free_event_desc(struct perf_evsel *events) { struct perf_evsel *evsel; @@ -2723,6 +2784,41 @@ static int process_bpf_prog_info(struct feat_fd *ff, return err; } +static int process_btf(struct feat_fd *ff, void *data __maybe_unused) +{ + struct perf_env *env = &ff->ph->env; + u32 count, i; + + if (do_read_u32(ff, &count)) + return -1; + + down_write(&env->bpf_info_lock); + + for (i = 0; i < count; ++i) { + struct btf_node btf_node; + struct btf_node *node; + + if (__do_read(ff, &btf_node, sizeof(struct btf_node))) + return -1; + + node = malloc(sizeof(struct btf_node) + btf_node.data_size); + if (!node) + return -1; + + node->id = btf_node.id; + node->data_size = btf_node.data_size; + + if (__do_read(ff, node->data, btf_node.data_size)) { + free(node); + return -1; + } + perf_env__insert_btf(env, node); + } + + up_write(&env->bpf_info_lock); + return 0; +} + struct feature_ops { int (*write)(struct feat_fd *ff, struct perf_evlist *evlist); void (*print)(struct feat_fd *ff, FILE *fp); @@ -2783,7 +2879,8 @@ static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = { FEAT_OPR(SAMPLE_TIME, sample_time, false), FEAT_OPR(MEM_TOPOLOGY, mem_topology, true), FEAT_OPR(CLOCKID, clockid, false), - FEAT_OPR(BPF_PROG_INFO, bpf_prog_info, false) + FEAT_OPR(BPF_PROG_INFO, bpf_prog_info, false), + FEAT_OPR(BTF, btf, false) }; struct header_print_data { diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h index 0785c91b4c3a..ba51d8e43c53 100644 --- a/tools/perf/util/header.h +++ b/tools/perf/util/header.h @@ -40,6 +40,7 @@ enum { HEADER_MEM_TOPOLOGY, HEADER_CLOCKID, HEADER_BPF_PROG_INFO, + HEADER_BTF, HEADER_LAST_FEATURE, HEADER_FEAT_BITS = 256, };
This patch enables perf-record to save btf information as headers to perf.data A new header type HEADER_BTF is introduced for this data. Signed-off-by: Song Liu <songliubraving@fb.com> --- tools/perf/util/header.c | 99 +++++++++++++++++++++++++++++++++++++++- tools/perf/util/header.h | 1 + 2 files changed, 99 insertions(+), 1 deletion(-)