On 11/3/20 11:23 AM, Andrii Nakryiko wrote:
On Tue, Nov 3, 2020 at 11:06 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
On Tue, Nov 03, 2020 at 10:58:58AM -0800, Andrii Nakryiko wrote:
On Mon, Nov 2, 2020 at 2:57 PM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
On Mon, Nov 02, 2020 at 10:59:08PM +0100, Jiri Olsa wrote:
On Sat, Oct 31, 2020 at 11:31:31PM +0100, Jiri Olsa wrote:
We need to generate just single BTF instance for the
function, while DWARF data contains multiple instances
of DW_TAG_subprogram tag.
Unfortunately we can no longer rely on DW_AT_declaration
tag (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060)
Instead we apply following checks:
- argument names are defined for the function
- there's symbol and address defined for the function
- function is generated only once
Also because we want to follow kernel's ftrace traceable
functions, this patchset is adding extra check that the
function is one of the ftrace's functions.
All ftrace functions addresses are stored in vmlinux
binary within symbols:
__start_mcount_loc
__stop_mcount_loc
hum, for some reason this does not pass through bpf internal
functions like bpf_iter_bpf_map.. I learned it hard way ;-)
what's the exact name of the function that was missing?
bpf_iter_bpf_map doesn't exist. And if it's __init function, why does
it matter, it's not going to be even available at runtime, right?
bpf_map iter definition:
DEFINE_BPF_ITER_FUNC(bpf_map, struct bpf_iter_meta *meta, struct bpf_map *map)
goes to:
#define DEFINE_BPF_ITER_FUNC(target, args...) \
extern int bpf_iter_ ## target(args); \
int __init bpf_iter_ ## target(args) { return 0; }
that creates __init bpf_iter_bpf_map function that will make
it into BTF where it's expected when opening iterator, but the
code will be freed because it's __init function
hm... should we just drop __init there?
Yonghong, is __init strictly necessary, or was just an optimization to
save a tiny bit of space?
It is an optimization to save some space. We only need function
signature, not function body, for bpf_iter.
The macro definition is in include/linux/bpf.h.
#define DEFINE_BPF_ITER_FUNC(target, args...) \
extern int bpf_iter_ ## target(args); \
int __init bpf_iter_ ## target(args) { return 0; }
Maybe you could have a section, e.g., called
.init.bpf.preserve_type
which you can scan through to preserve the types.
Alternatively you can drop the above __init, the saving is
indeed tiny. But this adds overhead to ksymbol lookup and
may not be desirable.
there are few iteratos functions like that, and I was going to
check if there's more
will check
so it gets filtered out because it's __init function
I'll check if the fix below catches all internal functions,
but I guess we should do something more robust
jirka
---
diff --git a/btf_encoder.c b/btf_encoder.c
index 0a378aa92142..3cd94280c35b 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -143,7 +143,8 @@ static int filter_functions(struct btf_elf *btfe, struct mcount_symbols *ms)
/* Do not enable .init section functions. */
if (init_filter &&
func->addr >= ms->init_begin &&
- func->addr < ms->init_end)
+ func->addr < ms->init_end &&
+ strncmp("bpf_", func->name, 4))
this looks like a very wrong way to do this? Can you please elaborate
on what's missing and why it shouldn't be missing?
yes, it's just a hack, we should do something more
robust as I mentioned above
it just allowed me to use iterators finaly ;-)
sure, I get it, I was just trying to understand why there is such a
problem in the first place. Turns out we need FUNCs not just for
fentry/fexit and similar, but also for bpf_iter, which is an entirely
different use case (similar to raw_tp, but raw_tp is using typedef ->
func_proto approach).
So I don't know, we might as well just not do mcount checks?.. As an
alternative, but it's not great as well.
jirka
