| Message ID | 20200216193005.144157-16-jolsa@kernel.org |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: Add trampoline and dispatcher to /proc/kallsyms | expand |
On Sun, Feb 16, 2020 at 08:30:02PM +0100, Jiri Olsa wrote: > Currently we don't sort bpf_kallsyms and display symbols > in proc/kallsyms as they come in via __bpf_ksym_add. > > Using the latch tree to get the next bpf_ksym object > and insert the new symbol ahead of it. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > kernel/bpf/core.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > index 2f857bbfe05c..fa814179730c 100644 > --- a/kernel/bpf/core.c > +++ b/kernel/bpf/core.c > @@ -651,9 +651,28 @@ static struct latch_tree_root bpf_progs_tree __cacheline_aligned; > > static void __bpf_ksym_add(struct bpf_ksym *ksym) > { > + struct list_head *head = &bpf_kallsyms; > + struct rb_node *next; > + > WARN_ON_ONCE(!list_empty(&ksym->lnode)); > - list_add_tail_rcu(&ksym->lnode, &bpf_kallsyms); > latch_tree_insert(&ksym->tnode, &bpf_ksym_tree, &bpf_ksym_tree_ops); > + > + /* > + * Add ksym into bpf_kallsyms in ordered position, > + * which is prepared for us by latch tree addition. > + * > + * Find out the next symbol and insert ksym right > + * ahead of it. If ksym is the last one, just tail > + * add to the bpf_kallsyms. > + */ > + next = rb_next(&ksym->tnode.node[0]); > + if (next) { > + struct bpf_ksym *ptr; > + > + ptr = container_of(next, struct bpf_ksym, tnode.node[0]); > + head = &ptr->lnode; > + } > + list_add_tail_rcu(&ksym->lnode, head); what is the motivation for sorting? do you want perf and other user space to depend on it? Or purely aesthetics?
On Tue, Feb 18, 2020 at 03:18:17PM -0800, Alexei Starovoitov wrote: > On Sun, Feb 16, 2020 at 08:30:02PM +0100, Jiri Olsa wrote: > > Currently we don't sort bpf_kallsyms and display symbols > > in proc/kallsyms as they come in via __bpf_ksym_add. > > > > Using the latch tree to get the next bpf_ksym object > > and insert the new symbol ahead of it. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > kernel/bpf/core.c | 21 ++++++++++++++++++++- > > 1 file changed, 20 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > > index 2f857bbfe05c..fa814179730c 100644 > > --- a/kernel/bpf/core.c > > +++ b/kernel/bpf/core.c > > @@ -651,9 +651,28 @@ static struct latch_tree_root bpf_progs_tree __cacheline_aligned; > > > > static void __bpf_ksym_add(struct bpf_ksym *ksym) > > { > > + struct list_head *head = &bpf_kallsyms; > > + struct rb_node *next; > > + > > WARN_ON_ONCE(!list_empty(&ksym->lnode)); > > - list_add_tail_rcu(&ksym->lnode, &bpf_kallsyms); > > latch_tree_insert(&ksym->tnode, &bpf_ksym_tree, &bpf_ksym_tree_ops); > > + > > + /* > > + * Add ksym into bpf_kallsyms in ordered position, > > + * which is prepared for us by latch tree addition. > > + * > > + * Find out the next symbol and insert ksym right > > + * ahead of it. If ksym is the last one, just tail > > + * add to the bpf_kallsyms. > > + */ > > + next = rb_next(&ksym->tnode.node[0]); > > + if (next) { > > + struct bpf_ksym *ptr; > > + > > + ptr = container_of(next, struct bpf_ksym, tnode.node[0]); > > + head = &ptr->lnode; > > + } > > + list_add_tail_rcu(&ksym->lnode, head); > > what is the motivation for sorting? do you want perf and other user space > to depend on it? Or purely aesthetics? initially I thought perf depends on it, but it does its own sorting but it turned out to be really easy and fast to sort bpf symbols at the end, so I included it, because core symbols in kallsyms are also sorted, I should have mentioned this in changelog jirka
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 2f857bbfe05c..fa814179730c 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -651,9 +651,28 @@ static struct latch_tree_root bpf_progs_tree __cacheline_aligned; static void __bpf_ksym_add(struct bpf_ksym *ksym) { + struct list_head *head = &bpf_kallsyms; + struct rb_node *next; + WARN_ON_ONCE(!list_empty(&ksym->lnode)); - list_add_tail_rcu(&ksym->lnode, &bpf_kallsyms); latch_tree_insert(&ksym->tnode, &bpf_ksym_tree, &bpf_ksym_tree_ops); + + /* + * Add ksym into bpf_kallsyms in ordered position, + * which is prepared for us by latch tree addition. + * + * Find out the next symbol and insert ksym right + * ahead of it. If ksym is the last one, just tail + * add to the bpf_kallsyms. + */ + next = rb_next(&ksym->tnode.node[0]); + if (next) { + struct bpf_ksym *ptr; + + ptr = container_of(next, struct bpf_ksym, tnode.node[0]); + head = &ptr->lnode; + } + list_add_tail_rcu(&ksym->lnode, head); } void bpf_ksym_add(struct bpf_ksym *ksym)
Currently we don't sort bpf_kallsyms and display symbols in proc/kallsyms as they come in via __bpf_ksym_add. Using the latch tree to get the next bpf_ksym object and insert the new symbol ahead of it. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- kernel/bpf/core.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)