mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Wang Nan <wangnan0@huawei.com>
To: <acme@kernel.org>, <namhyung@kernel.org>
Cc: <lizefan@huawei.com>, <pi3orama@163.com>,
	<linux-kernel@vger.kernel.org>, <jolsa@kernel.org>,
	<masami.hiramatsu.pt@hitachi.com>, Wang Nan <wangnan0@huawei.com>,
	Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH] perf tools: Rebuild rbtree when adjusting symbols for kcore
Date: Fri, 6 Nov 2015 09:46:12 +0000	[thread overview]
Message-ID: <1446803172-83107-1-git-send-email-wangnan0@huawei.com> (raw)

In dso__split_kallsyms_for_kcore(), current code adjusts symbol's
address but only reinsert it into rbtree if the symbol belongs to
another map. However, the expression for adjusting symbol (pos->start -=
curr_map->start - curr_map->pgoff) can change the relative order between
two symbols (even if the affected symbols are in different maps, in
kcore case they are possible to share one same dso), which damages the
rbtree.

For example:

When using kcore:

# readelf -a /proc/kcore

  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  ...
  LOAD           0x0000000000002000 0xffffffc000000000 0x0000000000000000    <-- kernel
                 0x000000007fc00000 0x000000007fc00000  RWE    1000
  LOAD           0xfffffffffc002000 0xffffffbffc000000 0x0000000000000000    <-- module
                 0x0000000004000000 0x0000000004000000  RWE    1000

For modules memory area:
  map->start = 0xffffffbffc000000, map->pgoff = 0xfffffffffc002000
For normal kernel memory area:
  map->start = 0xffffffc000000000, map->pgoff = 0x0000000000002000

Function A is a normal kernel function at:	0xffffffc00021b428.
Function B is a function in module at: 		0xffffffbffc000000.

&A > &B before calling dso__split_kallsyms_for_kcore(), and they are
already in the rbtree.

During dso__split_kallsyms_for_kcore(), when adjusting symbols using
 pos->start -= curr_map->start - curr_map->pgoff

pos->start for A become: (0xffffffc00021b428 - 0xffffffc000000000 + 0x0000000000002000) = 0x21d428
pos->start for B become: (0xffffffbffc000000 - 0xffffffbffc000000 + 0xfffffffffc002000) = 0xfffffffffc002000

&A < &B, the order is changed.

This patch rebuild rbtree unconditionally to ensure the rbtree is
always healthy.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---

Here is my test result on my aarch64 system:

*Step 1: create kprobes*

[root@localhost ~]# ./perf_arm64 probe -m /tmp/kernel_module.ko my_func
Added new event:
  probe:my_func (on my_func in kernel_module)

You can now use it in all perf tools, such as:

	perf record -e probe:my_func -aR sleep 1

[root@localhost ~]# ./perf_arm64 probe sys_write
Added new event:
  probe:sys_write      (on sys_write)

You can now use it in all perf tools, such as:

	perf record -e probe:sys_write -aR sleep 1

[root@localhost ~]# cat /sys/kernel/debug/kprobes/list 
ffffffbffc000000  k  my_func+0x0  kernel_module [DISABLED]
ffffffc00021b428  k  SyS_write+0x0    [DISABLED]


*Step 2: rebuild perf without commit 98d3b25*

$ git log --oneline
3321d2b Revert "perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success"
e054731 perf stat: Make stat options global
0014de1 perf sched latency: Fix thread pid reuse issue
98d3b25 perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success
956959f perf trace: Fix documentation for -i


*Step 3: test and get the buggy result*

[root@localhost ~]# PAGER=cat ./perf_arm64 probe -l 
  Error: Failed to show event list.

[root@localhost ~]# PAGER=cat ./perf_arm64 probe -v -l 
map_groups__set_modules_path_dir: cannot open /lib/modules/4.1.12+ dir
Problems setting modules path maps, continuing anyway...
Opening /sys/kernel/debug/tracing//kprobe_events write=0
Opening /sys/kernel/debug/tracing//uprobe_events write=0
Parsing probe_events: p:probe/my_func kernel_module:my_func
Group:probe Event:my_func probe:p
Looking at the vmlinux_path (7 entries long)
symsrc__init: cannot get elf header.
Using /proc/kcore for kernel object code
Using /proc/kallsyms for symbols
try to find information at 3ffc000000 in kernel_module
Failed to find module kernel_module.
Failed to find the path for kernel_module: [kernel_module]
Failed to find corresponding probes from debuginfo.
Failed to synthesize perf probe point: 0
  Error: Failed to show event list. Reason: Invalid argument (Code: -22)


*Step 4: Introduce this patch*

$ git log --oneline
36a8201 perf tools: Rebuild rbtree when adjusting symbols for kcore
3321d2b Revert "perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success"
e054731 perf stat: Make stat options global
0014de1 perf sched latency: Fix thread pid reuse issue
98d3b25 perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success


*Step 5: Try again*

[root@localhost ~]# PAGER=cat ./perf_arm64 probe -l 
  probe:my_func (on my_func in kernel_module)
  probe:sys_write      (on sys_write)
[root@localhost ~]# PAGER=cat ./perf_arm64 probe -v -l 
map_groups__set_modules_path_dir: cannot open /lib/modules/4.1.12+ dir
Problems setting modules path maps, continuing anyway...
Opening /sys/kernel/debug/tracing//kprobe_events write=0
Opening /sys/kernel/debug/tracing//uprobe_events write=0
Parsing probe_events: p:probe/my_func kernel_module:my_func
Group:probe Event:my_func probe:p
Looking at the vmlinux_path (7 entries long)
symsrc__init: cannot get elf header.
Using /proc/kcore for kernel object code
Using /proc/kallsyms for symbols
Failed to find corresponding probes from debuginfo.
Failed to find probe point from both of dwarf and map.
  probe:my_func (on my_func in kernel_module)
Parsing probe_events: p:probe/sys_write _text+1684520
Group:probe Event:sys_write probe:p
try to find information at 19b428 in kernel
Looking at the vmlinux_path (7 entries long)
symsrc__init: cannot get elf header.
Failed to find the path for kernel: Invalid ELF file
Failed to find corresponding probes from debuginfo.
  probe:sys_write      (on sys_write)

---
 tools/perf/util/symbol.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b4cc766..09bb6e8 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -654,7 +654,7 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
 	struct map_groups *kmaps = map__kmaps(map);
 	struct map *curr_map;
 	struct symbol *pos;
-	int count = 0, moved = 0;
+	int count = 0;
 	struct rb_root *root = &dso->symbols[map->type];
 	struct rb_node *next = rb_first(root);
 
@@ -677,25 +677,23 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
 			rb_erase_init(&pos->rb_node, root);
 			symbol__delete(pos);
 		} else {
+			rb_erase_init(&pos->rb_node, root);
+
 			pos->start -= curr_map->start - curr_map->pgoff;
 			if (pos->end)
 				pos->end -= curr_map->start - curr_map->pgoff;
-			if (curr_map->dso != map->dso) {
-				rb_erase_init(&pos->rb_node, root);
-				symbols__insert(
-					&curr_map->dso->symbols[curr_map->type],
-					pos);
-				++moved;
-			} else {
-				++count;
-			}
+
+			symbols__insert(
+				&curr_map->dso->symbols[curr_map->type],
+				pos);
+			++count;
 		}
 	}
 
 	/* Symbols have been adjusted */
 	dso->adjust_symbols = 1;
 
-	return count + moved;
+	return count;
 }
 
 /*
-- 
1.8.3.4


             reply	other threads:[~2015-11-06 10:13 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-06  9:46 Wang Nan [this message]
2015-11-06 13:19 ` [PATCH] perf symbols/KCORE: " Arnaldo Carvalho de Melo
2015-11-06 13:34   ` Wangnan (F)
2015-11-06 14:03     ` Arnaldo Carvalho de Melo
2015-11-06 14:33       ` pi3orama
2015-11-06 13:59   ` Adrian Hunter
2015-11-06 14:31     ` pi3orama
2015-11-06 18:51     ` Arnaldo Carvalho de Melo
2015-11-09  8:26       ` Adrian Hunter
2015-11-09 14:56         ` Arnaldo Carvalho de Melo
2015-11-10 13:41           ` Adrian Hunter
2015-11-11  7:02     ` Wangnan (F)
2015-11-11 20:44       ` Arnaldo Carvalho de Melo
2015-11-12  6:42         ` Adrian Hunter
2015-11-16 10:58     ` [tip:perf/urgent] perf symbols: " tip-bot for Adrian Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1446803172-83107-1-git-send-email-wangnan0@huawei.com \
    --to=wangnan0@huawei.com \
    --cc=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=namhyung@kernel.org \
    --cc=pi3orama@163.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®