| Message ID | 20210721153808.6902-4-quentin@isovalent.com |
|---|---|
| State | New |
| Headers | show |
| Series | libbpf: rename btf__get_from_id() and btf__load() APIs, support split BTF | expand |
On Wed, Jul 21, 2021 at 8:38 AM Quentin Monnet <quentin@isovalent.com> wrote: > > Replace the calls to deprecated function btf__get_from_id() with calls > to btf__load_from_kernel_by_id() in tools/ (bpftool, perf, selftests). > Update the surrounding code accordingly (instead of passing a pointer to > the btf struct, get it as a return value from the function). Also make > sure that btf__free() is called on the pointer after use. > > v2: > - Given that btf__load_from_kernel_by_id() has changed since v1, adapt > the code accordingly instead of just renaming the function. Also add a > few calls to btf__free() when necessary. > > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > Acked-by: John Fastabend <john.fastabend@gmail.com> > --- > tools/bpf/bpftool/btf.c | 8 ++---- > tools/bpf/bpftool/btf_dumper.c | 6 ++-- > tools/bpf/bpftool/map.c | 16 +++++------ > tools/bpf/bpftool/prog.c | 29 ++++++++++++++------ > tools/perf/util/bpf-event.c | 11 ++++---- > tools/perf/util/bpf_counter.c | 12 ++++++-- > tools/testing/selftests/bpf/prog_tests/btf.c | 4 ++- > 7 files changed, 51 insertions(+), 35 deletions(-) > [...] > diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c > index 09ae0381205b..12787758ce03 100644 > --- a/tools/bpf/bpftool/map.c > +++ b/tools/bpf/bpftool/map.c > @@ -805,12 +805,11 @@ static struct btf *get_map_kv_btf(const struct bpf_map_info *info) > } > return btf_vmlinux; > } else if (info->btf_value_type_id) { > - int err; > - > - err = btf__get_from_id(info->btf_id, &btf); > - if (err || !btf) { > + btf = btf__load_from_kernel_by_id(info->btf_id); > + if (libbpf_get_error(btf)) { > p_err("failed to get btf"); > - btf = err ? ERR_PTR(err) : ERR_PTR(-ESRCH); > + if (!btf) > + btf = ERR_PTR(-ESRCH); why not do a simpler (less conditionals) err = libbpf_get_error(btf); if (err) { btf = ERR_PTR(err); } ? > } > } > > @@ -1039,11 +1038,10 @@ static void print_key_value(struct bpf_map_info *info, void *key, > void *value) > { > json_writer_t *btf_wtr; > - struct btf *btf = NULL; > - int err; > + struct btf *btf; > > - err = btf__get_from_id(info->btf_id, &btf); > - if (err) { > + btf = btf__load_from_kernel_by_id(info->btf_id); > + if (libbpf_get_error(btf)) { > p_err("failed to get btf"); > return; > } [...] > > func_info = u64_to_ptr(info->func_info); > @@ -781,6 +784,8 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, > kernel_syms_destroy(&dd); > } > > + btf__free(btf); > + warrants a Fixes: tag? > return 0; > } > > @@ -2002,8 +2007,8 @@ static char *profile_target_name(int tgt_fd) > struct bpf_prog_info_linear *info_linear; > struct bpf_func_info *func_info; > const struct btf_type *t; > + struct btf *btf = NULL; > char *name = NULL; > - struct btf *btf; > > info_linear = bpf_program__get_prog_info_linear( > tgt_fd, 1UL << BPF_PROG_INFO_FUNC_INFO); > @@ -2012,12 +2017,17 @@ static char *profile_target_name(int tgt_fd) > return NULL; > } > > - if (info_linear->info.btf_id == 0 || > - btf__get_from_id(info_linear->info.btf_id, &btf)) { > + if (info_linear->info.btf_id == 0) { > p_err("prog FD %d doesn't have valid btf", tgt_fd); > goto out; > } > > + btf = btf__load_from_kernel_by_id(info_linear->info.btf_id); > + if (libbpf_get_error(btf)) { > + p_err("failed to load btf for prog FD %d", tgt_fd); > + goto out; > + } > + > func_info = u64_to_ptr(info_linear->info.func_info); > t = btf__type_by_id(btf, func_info[0].type_id); > if (!t) { > @@ -2027,6 +2037,7 @@ static char *profile_target_name(int tgt_fd) > } > name = strdup(btf__name_by_offset(btf, t->name_off)); > out: > + btf__free(btf); and another Fixes? :) and two more below > free(info_linear); > return name; > } [...]
2021-07-22 17:48 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com> > On Wed, Jul 21, 2021 at 8:38 AM Quentin Monnet <quentin@isovalent.com> wrote: >> >> Replace the calls to deprecated function btf__get_from_id() with calls >> to btf__load_from_kernel_by_id() in tools/ (bpftool, perf, selftests). >> Update the surrounding code accordingly (instead of passing a pointer to >> the btf struct, get it as a return value from the function). Also make >> sure that btf__free() is called on the pointer after use. >> >> v2: >> - Given that btf__load_from_kernel_by_id() has changed since v1, adapt >> the code accordingly instead of just renaming the function. Also add a >> few calls to btf__free() when necessary. >> >> Signed-off-by: Quentin Monnet <quentin@isovalent.com> >> Acked-by: John Fastabend <john.fastabend@gmail.com> >> --- >> tools/bpf/bpftool/btf.c | 8 ++---- >> tools/bpf/bpftool/btf_dumper.c | 6 ++-- >> tools/bpf/bpftool/map.c | 16 +++++------ >> tools/bpf/bpftool/prog.c | 29 ++++++++++++++------ >> tools/perf/util/bpf-event.c | 11 ++++---- >> tools/perf/util/bpf_counter.c | 12 ++++++-- >> tools/testing/selftests/bpf/prog_tests/btf.c | 4 ++- >> 7 files changed, 51 insertions(+), 35 deletions(-) >> > > [...] > >> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c >> index 09ae0381205b..12787758ce03 100644 >> --- a/tools/bpf/bpftool/map.c >> +++ b/tools/bpf/bpftool/map.c >> @@ -805,12 +805,11 @@ static struct btf *get_map_kv_btf(const struct bpf_map_info *info) >> } >> return btf_vmlinux; >> } else if (info->btf_value_type_id) { >> - int err; >> - >> - err = btf__get_from_id(info->btf_id, &btf); >> - if (err || !btf) { >> + btf = btf__load_from_kernel_by_id(info->btf_id); >> + if (libbpf_get_error(btf)) { >> p_err("failed to get btf"); >> - btf = err ? ERR_PTR(err) : ERR_PTR(-ESRCH); >> + if (!btf) >> + btf = ERR_PTR(-ESRCH); > > why not do a simpler (less conditionals) > > err = libbpf_get_error(btf); > if (err) { > btf = ERR_PTR(err); > } > > ? Because if btf is NULL at this stage, this would change the return value from -ESRCH to NULL. This would be problematic in mapdump(), since we check this value ("if (IS_ERR(btf))") to detect a failure in get_map_kv_btf(). I could change that check in mapdump() to use libbpf_get_error() instead, but in that case it would similarly change the return value for mapdump() (and errno), which I think would be propagated up to main() and would return 0 instead of -ESRCH. This does not seem suitable and would play badly with batch mode, among other things. So I'm considering keeping the one additional if. > >> } >> } >> >> @@ -1039,11 +1038,10 @@ static void print_key_value(struct bpf_map_info *info, void *key, >> void *value) >> { >> json_writer_t *btf_wtr; >> - struct btf *btf = NULL; >> - int err; >> + struct btf *btf; >> >> - err = btf__get_from_id(info->btf_id, &btf); >> - if (err) { >> + btf = btf__load_from_kernel_by_id(info->btf_id); >> + if (libbpf_get_error(btf)) { >> p_err("failed to get btf"); >> return; >> } > > [...] > >> >> func_info = u64_to_ptr(info->func_info); >> @@ -781,6 +784,8 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, >> kernel_syms_destroy(&dd); >> } >> >> + btf__free(btf); >> + > > warrants a Fixes: tag? I don't mind adding the tags, but do they have any advantage here? My understanding is that they tend to be neon signs for backports to stable branches, but this patch depends on btf__load_from_kernel_by_id(), meaning more patches to pull. I'll see if I can move the btf__free() fixes to a separate commit, maybe.
On Fri, Jul 23, 2021 at 2:52 AM Quentin Monnet <quentin@isovalent.com> wrote: > > 2021-07-22 17:48 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com> > > On Wed, Jul 21, 2021 at 8:38 AM Quentin Monnet <quentin@isovalent.com> wrote: > >> > >> Replace the calls to deprecated function btf__get_from_id() with calls > >> to btf__load_from_kernel_by_id() in tools/ (bpftool, perf, selftests). > >> Update the surrounding code accordingly (instead of passing a pointer to > >> the btf struct, get it as a return value from the function). Also make > >> sure that btf__free() is called on the pointer after use. > >> > >> v2: > >> - Given that btf__load_from_kernel_by_id() has changed since v1, adapt > >> the code accordingly instead of just renaming the function. Also add a > >> few calls to btf__free() when necessary. > >> > >> Signed-off-by: Quentin Monnet <quentin@isovalent.com> > >> Acked-by: John Fastabend <john.fastabend@gmail.com> > >> --- > >> tools/bpf/bpftool/btf.c | 8 ++---- > >> tools/bpf/bpftool/btf_dumper.c | 6 ++-- > >> tools/bpf/bpftool/map.c | 16 +++++------ > >> tools/bpf/bpftool/prog.c | 29 ++++++++++++++------ > >> tools/perf/util/bpf-event.c | 11 ++++---- > >> tools/perf/util/bpf_counter.c | 12 ++++++-- > >> tools/testing/selftests/bpf/prog_tests/btf.c | 4 ++- > >> 7 files changed, 51 insertions(+), 35 deletions(-) > >> > > > > [...] > > > >> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c > >> index 09ae0381205b..12787758ce03 100644 > >> --- a/tools/bpf/bpftool/map.c > >> +++ b/tools/bpf/bpftool/map.c > >> @@ -805,12 +805,11 @@ static struct btf *get_map_kv_btf(const struct bpf_map_info *info) > >> } > >> return btf_vmlinux; > >> } else if (info->btf_value_type_id) { > >> - int err; > >> - > >> - err = btf__get_from_id(info->btf_id, &btf); > >> - if (err || !btf) { > >> + btf = btf__load_from_kernel_by_id(info->btf_id); > >> + if (libbpf_get_error(btf)) { > >> p_err("failed to get btf"); > >> - btf = err ? ERR_PTR(err) : ERR_PTR(-ESRCH); > >> + if (!btf) > >> + btf = ERR_PTR(-ESRCH); > > > > why not do a simpler (less conditionals) > > > > err = libbpf_get_error(btf); > > if (err) { > > btf = ERR_PTR(err); > > } > > > > ? > > Because if btf is NULL at this stage, this would change the return value > from -ESRCH to NULL. This would be problematic in mapdump(), since we > check this value ("if (IS_ERR(btf))") to detect a failure in > get_map_kv_btf(). see my reply on previous patch. libbpf_get_error() handles this transparently regardless of CLEAN_PTRS mode, as long as it is called right after API call. So the above sample will work as you'd expect, preserving errors. > > I could change that check in mapdump() to use libbpf_get_error() > instead, but in that case it would similarly change the return value for > mapdump() (and errno), which I think would be propagated up to main() > and would return 0 instead of -ESRCH. This does not seem suitable and > would play badly with batch mode, among other things. > > So I'm considering keeping the one additional if. > > > > >> } > >> } > >> > >> @@ -1039,11 +1038,10 @@ static void print_key_value(struct bpf_map_info *info, void *key, > >> void *value) > >> { > >> json_writer_t *btf_wtr; > >> - struct btf *btf = NULL; > >> - int err; > >> + struct btf *btf; > >> > >> - err = btf__get_from_id(info->btf_id, &btf); > >> - if (err) { > >> + btf = btf__load_from_kernel_by_id(info->btf_id); > >> + if (libbpf_get_error(btf)) { > >> p_err("failed to get btf"); > >> return; > >> } > > > > [...] > > > >> > >> func_info = u64_to_ptr(info->func_info); > >> @@ -781,6 +784,8 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, > >> kernel_syms_destroy(&dd); > >> } > >> > >> + btf__free(btf); > >> + > > > > warrants a Fixes: tag? > > I don't mind adding the tags, but do they have any advantage here? My > understanding is that they tend to be neon signs for backports to stable > branches, but this patch depends on btf__load_from_kernel_by_id(), > meaning more patches to pull. I'll see if I can move the btf__free() > fixes to a separate commit, maybe. Having Fixes: allows to keep track of where the issue originated. It doesn't necessarily mean something has to be backported, as far as I understand. So it's good to do regardless. Splitting fixes into a separate patch works for me as well, but I don't care all that much given they are small.
2021-07-23 08:57 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com> > On Fri, Jul 23, 2021 at 2:52 AM Quentin Monnet <quentin@isovalent.com> wrote: >> >> 2021-07-22 17:48 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com> >>> On Wed, Jul 21, 2021 at 8:38 AM Quentin Monnet <quentin@isovalent.com> wrote: >>>> >>>> Replace the calls to deprecated function btf__get_from_id() with calls >>>> to btf__load_from_kernel_by_id() in tools/ (bpftool, perf, selftests). >>>> Update the surrounding code accordingly (instead of passing a pointer to >>>> the btf struct, get it as a return value from the function). Also make >>>> sure that btf__free() is called on the pointer after use. >>>> >>>> v2: >>>> - Given that btf__load_from_kernel_by_id() has changed since v1, adapt >>>> the code accordingly instead of just renaming the function. Also add a >>>> few calls to btf__free() when necessary. >>>> >>>> Signed-off-by: Quentin Monnet <quentin@isovalent.com> >>>> Acked-by: John Fastabend <john.fastabend@gmail.com> >>>> --- >>>> tools/bpf/bpftool/btf.c | 8 ++---- >>>> tools/bpf/bpftool/btf_dumper.c | 6 ++-- >>>> tools/bpf/bpftool/map.c | 16 +++++------ >>>> tools/bpf/bpftool/prog.c | 29 ++++++++++++++------ >>>> tools/perf/util/bpf-event.c | 11 ++++---- >>>> tools/perf/util/bpf_counter.c | 12 ++++++-- >>>> tools/testing/selftests/bpf/prog_tests/btf.c | 4 ++- >>>> 7 files changed, 51 insertions(+), 35 deletions(-) >>>> >>> >>> [...] >>> >>>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c >>>> index 09ae0381205b..12787758ce03 100644 >>>> --- a/tools/bpf/bpftool/map.c >>>> +++ b/tools/bpf/bpftool/map.c >>>> @@ -805,12 +805,11 @@ static struct btf *get_map_kv_btf(const struct bpf_map_info *info) >>>> } >>>> return btf_vmlinux; >>>> } else if (info->btf_value_type_id) { >>>> - int err; >>>> - >>>> - err = btf__get_from_id(info->btf_id, &btf); >>>> - if (err || !btf) { >>>> + btf = btf__load_from_kernel_by_id(info->btf_id); >>>> + if (libbpf_get_error(btf)) { >>>> p_err("failed to get btf"); >>>> - btf = err ? ERR_PTR(err) : ERR_PTR(-ESRCH); >>>> + if (!btf) >>>> + btf = ERR_PTR(-ESRCH); >>> >>> why not do a simpler (less conditionals) >>> >>> err = libbpf_get_error(btf); >>> if (err) { >>> btf = ERR_PTR(err); >>> } >>> >>> ? >> >> Because if btf is NULL at this stage, this would change the return value >> from -ESRCH to NULL. This would be problematic in mapdump(), since we >> check this value ("if (IS_ERR(btf))") to detect a failure in >> get_map_kv_btf(). > > see my reply on previous patch. libbpf_get_error() handles this > transparently regardless of CLEAN_PTRS mode, as long as it is called > right after API call. So the above sample will work as you'd expect, > preserving errors. Right, it looks like I got confused on this one. I'll update it. > >> >> I could change that check in mapdump() to use libbpf_get_error() >> instead, but in that case it would similarly change the return value for >> mapdump() (and errno), which I think would be propagated up to main() >> and would return 0 instead of -ESRCH. This does not seem suitable and >> would play badly with batch mode, among other things. >> >> So I'm considering keeping the one additional if. >> >>> >>>> } >>>> } >>>> >>>> @@ -1039,11 +1038,10 @@ static void print_key_value(struct bpf_map_info *info, void *key, >>>> void *value) >>>> { >>>> json_writer_t *btf_wtr; >>>> - struct btf *btf = NULL; >>>> - int err; >>>> + struct btf *btf; >>>> >>>> - err = btf__get_from_id(info->btf_id, &btf); >>>> - if (err) { >>>> + btf = btf__load_from_kernel_by_id(info->btf_id); >>>> + if (libbpf_get_error(btf)) { >>>> p_err("failed to get btf"); >>>> return; >>>> } >>> >>> [...] >>> >>>> >>>> func_info = u64_to_ptr(info->func_info); >>>> @@ -781,6 +784,8 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, >>>> kernel_syms_destroy(&dd); >>>> } >>>> >>>> + btf__free(btf); >>>> + >>> >>> warrants a Fixes: tag? >> >> I don't mind adding the tags, but do they have any advantage here? My >> understanding is that they tend to be neon signs for backports to stable >> branches, but this patch depends on btf__load_from_kernel_by_id(), >> meaning more patches to pull. I'll see if I can move the btf__free() >> fixes to a separate commit, maybe. > > Having Fixes: allows to keep track of where the issue originated. It > doesn't necessarily mean something has to be backported, as far as I > understand. So it's good to do regardless. Splitting fixes into a > separate patch works for me as well, but I don't care all that much > given they are small. > OK, thank you for the clarification :). I'll keep a single patch in that case.
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c index 385d5c955cf3..9162a18e84c0 100644 --- a/tools/bpf/bpftool/btf.c +++ b/tools/bpf/bpftool/btf.c @@ -580,16 +580,12 @@ static int do_dump(int argc, char **argv) } if (!btf) { - err = btf__get_from_id(btf_id, &btf); + btf = btf__load_from_kernel_by_id(btf_id); + err = libbpf_get_error(btf); if (err) { p_err("get btf by id (%u): %s", btf_id, strerror(err)); goto done; } - if (!btf) { - err = -ENOENT; - p_err("can't find btf with ID (%u)", btf_id); - goto done; - } } if (dump_c) { diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c index 7ca54d046362..9c25286a5c73 100644 --- a/tools/bpf/bpftool/btf_dumper.c +++ b/tools/bpf/bpftool/btf_dumper.c @@ -64,8 +64,10 @@ static int dump_prog_id_as_func_ptr(const struct btf_dumper *d, } info = &prog_info->info; - if (!info->btf_id || !info->nr_func_info || - btf__get_from_id(info->btf_id, &prog_btf)) + if (!info->btf_id || !info->nr_func_info) + goto print; + prog_btf = btf__load_from_kernel_by_id(info->btf_id); + if (libbpf_get_error(prog_btf)) goto print; finfo = u64_to_ptr(info->func_info); func_type = btf__type_by_id(prog_btf, finfo->type_id); diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c index 09ae0381205b..12787758ce03 100644 --- a/tools/bpf/bpftool/map.c +++ b/tools/bpf/bpftool/map.c @@ -805,12 +805,11 @@ static struct btf *get_map_kv_btf(const struct bpf_map_info *info) } return btf_vmlinux; } else if (info->btf_value_type_id) { - int err; - - err = btf__get_from_id(info->btf_id, &btf); - if (err || !btf) { + btf = btf__load_from_kernel_by_id(info->btf_id); + if (libbpf_get_error(btf)) { p_err("failed to get btf"); - btf = err ? ERR_PTR(err) : ERR_PTR(-ESRCH); + if (!btf) + btf = ERR_PTR(-ESRCH); } } @@ -1039,11 +1038,10 @@ static void print_key_value(struct bpf_map_info *info, void *key, void *value) { json_writer_t *btf_wtr; - struct btf *btf = NULL; - int err; + struct btf *btf; - err = btf__get_from_id(info->btf_id, &btf); - if (err) { + btf = btf__load_from_kernel_by_id(info->btf_id); + if (libbpf_get_error(btf)) { p_err("failed to get btf"); return; } diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index cc48726740ad..b1996b8f1d42 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -249,10 +249,10 @@ static void show_prog_metadata(int fd, __u32 num_maps) struct bpf_map_info map_info; struct btf_var_secinfo *vsi; bool printed_header = false; - struct btf *btf = NULL; unsigned int i, vlen; void *value = NULL; const char *name; + struct btf *btf; int err; if (!num_maps) @@ -263,8 +263,8 @@ static void show_prog_metadata(int fd, __u32 num_maps) if (!value) return; - err = btf__get_from_id(map_info.btf_id, &btf); - if (err || !btf) + btf = btf__load_from_kernel_by_id(map_info.btf_id); + if (libbpf_get_error(btf)) goto out_free; t_datasec = btf__type_by_id(btf, map_info.btf_value_type_id); @@ -646,9 +646,12 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, member_len = info->xlated_prog_len; } - if (info->btf_id && btf__get_from_id(info->btf_id, &btf)) { - p_err("failed to get btf"); - return -1; + if (info->btf_id) { + btf = btf__load_from_kernel_by_id(info->btf_id); + if (libbpf_get_error(btf)) { + p_err("failed to get btf"); + return -1; + } } func_info = u64_to_ptr(info->func_info); @@ -781,6 +784,8 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, kernel_syms_destroy(&dd); } + btf__free(btf); + return 0; } @@ -2002,8 +2007,8 @@ static char *profile_target_name(int tgt_fd) struct bpf_prog_info_linear *info_linear; struct bpf_func_info *func_info; const struct btf_type *t; + struct btf *btf = NULL; char *name = NULL; - struct btf *btf; info_linear = bpf_program__get_prog_info_linear( tgt_fd, 1UL << BPF_PROG_INFO_FUNC_INFO); @@ -2012,12 +2017,17 @@ static char *profile_target_name(int tgt_fd) return NULL; } - if (info_linear->info.btf_id == 0 || - btf__get_from_id(info_linear->info.btf_id, &btf)) { + if (info_linear->info.btf_id == 0) { p_err("prog FD %d doesn't have valid btf", tgt_fd); goto out; } + btf = btf__load_from_kernel_by_id(info_linear->info.btf_id); + if (libbpf_get_error(btf)) { + p_err("failed to load btf for prog FD %d", tgt_fd); + goto out; + } + func_info = u64_to_ptr(info_linear->info.func_info); t = btf__type_by_id(btf, func_info[0].type_id); if (!t) { @@ -2027,6 +2037,7 @@ static char *profile_target_name(int tgt_fd) } name = strdup(btf__name_by_offset(btf, t->name_off)); out: + btf__free(btf); free(info_linear); return name; } diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c index cdecda1ddd36..996d025b8ed8 100644 --- a/tools/perf/util/bpf-event.c +++ b/tools/perf/util/bpf-event.c @@ -223,10 +223,10 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session, free(info_linear); return -1; } - if (btf__get_from_id(info->btf_id, &btf)) { + btf = btf__load_from_kernel_by_id(info->btf_id); + if (libbpf_get_error(btf)) { pr_debug("%s: failed to get BTF of id %u, aborting\n", __func__, info->btf_id); err = -1; - btf = NULL; goto out; } perf_env__fetch_btf(env, info->btf_id, btf); @@ -296,7 +296,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session, out: free(info_linear); - free(btf); + btf__free(btf); return err ? -1 : 0; } @@ -478,7 +478,8 @@ static void perf_env__add_bpf_info(struct perf_env *env, u32 id) if (btf_id == 0) goto out; - if (btf__get_from_id(btf_id, &btf)) { + btf = btf__load_from_kernel_by_id(btf_id); + if (libbpf_get_error(btf)) { pr_debug("%s: failed to get BTF of id %u, aborting\n", __func__, btf_id); goto out; @@ -486,7 +487,7 @@ static void perf_env__add_bpf_info(struct perf_env *env, u32 id) perf_env__fetch_btf(env, btf_id, btf); out: - free(btf); + btf__free(btf); close(fd); } diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c index 8150e03367bb..ba0f20853651 100644 --- a/tools/perf/util/bpf_counter.c +++ b/tools/perf/util/bpf_counter.c @@ -64,8 +64,8 @@ static char *bpf_target_prog_name(int tgt_fd) struct bpf_prog_info_linear *info_linear; struct bpf_func_info *func_info; const struct btf_type *t; + struct btf *btf = NULL; char *name = NULL; - struct btf *btf; info_linear = bpf_program__get_prog_info_linear( tgt_fd, 1UL << BPF_PROG_INFO_FUNC_INFO); @@ -74,12 +74,17 @@ static char *bpf_target_prog_name(int tgt_fd) return NULL; } - if (info_linear->info.btf_id == 0 || - btf__get_from_id(info_linear->info.btf_id, &btf)) { + if (info_linear->info.btf_id == 0) { pr_debug("prog FD %d doesn't have valid btf\n", tgt_fd); goto out; } + btf = btf__load_from_kernel_by_id(info_linear->info.btf_id); + if (libbpf_get_error(btf)) { + pr_debug("failed to load btf for prog FD %d\n", tgt_fd); + goto out; + } + func_info = u64_to_ptr(info_linear->info.func_info); t = btf__type_by_id(btf, func_info[0].type_id); if (!t) { @@ -89,6 +94,7 @@ static char *bpf_target_prog_name(int tgt_fd) } name = strdup(btf__name_by_offset(btf, t->name_off)); out: + btf__free(btf); free(info_linear); return name; } diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c index 857e3f26086f..649f87382c8d 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf.c +++ b/tools/testing/selftests/bpf/prog_tests/btf.c @@ -4350,7 +4350,8 @@ static void do_test_file(unsigned int test_num) goto done; } - err = btf__get_from_id(info.btf_id, &btf); + btf = btf__load_from_kernel_by_id(info.btf_id); + err = libbpf_get_error(btf); if (CHECK(err, "cannot get btf from kernel, err: %d", err)) goto done; @@ -4386,6 +4387,7 @@ static void do_test_file(unsigned int test_num) fprintf(stderr, "OK"); done: + btf__free(btf); free(func_info); bpf_object__close(obj); }