[PATCH] perf probe: Fix failure to probe events on arm

[He Kuang]

Fix failure to probe events on arm, problem is introduced by commit
5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of
.text"). For some architectures, label '_etext' is not in the .text
section(in .notes section for arm/arm64). Label out of .text section is
not loaded as symbols and we got a zero value when look up its address,
which causes all events be wrongly skiped.

This patch uses kernel map->end when failed to get the address of
'_etext' and fixes the problem.

Problem can be reproduced on arm as following:

# perf probe --add='generic_perform_write'
generic_perform_write+0 is out of .text, skip it.
Probe point 'generic_perform_write' not found.
Error: Failed to add events. Reason: No such file or directory (Code: -2)

After this patch:

# perf probe --add='generic_perform_write'
Added new event:
probe:generic_perform_write (on generic_perform_write)

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

perf record -e probe:generic_perform_write -aR sleep 1

Signed-off-by: He Kuang <hek...@huawei.com>
---
tools/perf/util/probe-event.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index daa24a2..ee26961 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -575,8 +575,22 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
pr_warning("Relocated base symbol is not found!\n");
return -EINVAL;
}
- /* Get the address of _etext for checking non-probable text symbol */
+ /* Get the address of _etext for checking non-probable text symbol,
+ for some architectures (e.g. arm, arm64), _etext is out of .text
+ section and not loaded as symbols, use kernel map->end instead.
+ */
etext_addr = kernel_get_symbol_address_by_name("_etext", false);
+ if (etext_addr == 0) {
+ struct map *map;
+
+ map = kernel_get_module_map(NULL);
+ if (!map) {
+ pr_err("Failed to get a map for kernel\n");
+ return -EINVAL;
+ }
+
+ etext_addr = map->end;
+ }

for (i = 0; i < ntevs; i++) {
if (tevs[i].point.address && !tevs[i].point.retprobe) {
--
1.8.5.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Arnaldo Carvalho de Melo]

Em Mon, Jun 15, 2015 at 08:06:53AM +0000, He Kuang escreveu:
> Fix failure to probe events on arm, problem is introduced by commit
> 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of
> .text"). For some architectures, label '_etext' is not in the .text
> section(in .notes section for arm/arm64). Label out of .text section is
> not loaded as symbols and we got a zero value when look up its address,
> which causes all events be wrongly skiped.
>
> This patch uses kernel map->end when failed to get the address of
> '_etext' and fixes the problem.

Masami, can't we always use map->end then? Can you please take a look at
this patch and ack/nack it?

- Arnaldo

[Masami Hiramatsu]

On 2015/06/15 23:49, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jun 15, 2015 at 08:06:53AM +0000, He Kuang escreveu:
>> Fix failure to probe events on arm, problem is introduced by commit
>> 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of
>> .text"). For some architectures, label '_etext' is not in the .text
>> section(in .notes section for arm/arm64). Label out of .text section is
>> not loaded as symbols and we got a zero value when look up its address,
>> which causes all events be wrongly skiped.
>>
>> This patch uses kernel map->end when failed to get the address of
>> '_etext' and fixes the problem.
>
> Masami, can't we always use map->end then? Can you please take a look at
> this patch and ack/nack it?

Yeah, it looks good to me, and we finally will replace "_etext" with it :)

----
void map__fixup_end(struct map *map)
{
struct rb_root *symbols = &map->dso->symbols[map->type];
struct rb_node *nd = rb_last(symbols);
if (nd != NULL) {
struct symbol *sym = rb_entry(nd, struct symbol, rb_node);
map->end = sym->end;
}
}
----
So, the map->end shows the end address of the symbols.

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>


Thanks!

--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hi...@hitachi.com

[Arnaldo Carvalho de Melo]

Em Tue, Jun 16, 2015 at 03:05:17PM +0900, Masami Hiramatsu escreveu:
> On 2015/06/15 23:49, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Jun 15, 2015 at 08:06:53AM +0000, He Kuang escreveu:
> >> Fix failure to probe events on arm, problem is introduced by commit
> >> 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of
> >> .text"). For some architectures, label '_etext' is not in the .text
> >> section(in .notes section for arm/arm64). Label out of .text section is
> >> not loaded as symbols and we got a zero value when look up its address,
> >> which causes all events be wrongly skiped.
> >>
> >> This patch uses kernel map->end when failed to get the address of
> >> '_etext' and fixes the problem.
> >
> > Masami, can't we always use map->end then? Can you please take a look at
> > this patch and ack/nack it?
>
> Yeah, it looks good to me, and we finally will replace "_etext" with it :)
>
> ----
> void map__fixup_end(struct map *map)
> {
> struct rb_root *symbols = &map->dso->symbols[map->type];
> struct rb_node *nd = rb_last(symbols);
> if (nd != NULL) {
> struct symbol *sym = rb_entry(nd, struct symbol, rb_node);
> map->end = sym->end;
> }
> }
> ----
> So, the map->end shows the end address of the symbols.
>
> Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>

I can and will apply it, but the question remains, if this approach is
good then when should we bother looking at "_etext" at all, i.e. why not
just use map->end all the time?

I have not thought this thru, this is just based on this ``its ok to use
map->end if "_etext" is not found'' idea that He proposed and you acked.

- Arnaldo

[hekuang]

hi, Arnaldo

On 06/15/2015 10:49 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jun 15, 2015 at 08:06:53AM +0000, He Kuang escreveu:
>> Fix failure to probe events on arm, problem is introduced by commit
>> 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of
>> .text"). For some architectures, label '_etext' is not in the .text
>> section(in .notes section for arm/arm64). Label out of .text section is
>> not loaded as symbols and we got a zero value when look up its address,
>> which causes all events be wrongly skiped.
>>
>> This patch uses kernel map->end when failed to get the address of
>> '_etext' and fixes the problem.
> Masami, can't we always use map->end then? Can you please take a look at
> this patch and ack/nack it?
>
> - Arnaldo

I think _etext is more accurate than kernel map->end, because
__map_groups__fixup_end() is called at the end of
dso__load_sym(), which fixes map->end to next_map->start.

Comparative result as this:
etext_addr=ffffffff819a1b85, map->end=ffffffff81ff1000.

So if possible, we should use _etext.

Thanks.

[Masami Hiramatsu]

On 2015/06/17 0:26, hekuang wrote:
> hi, Arnaldo
>
> On 06/15/2015 10:49 PM, Arnaldo Carvalho de Melo wrote:
>> Em Mon, Jun 15, 2015 at 08:06:53AM +0000, He Kuang escreveu:
>>> Fix failure to probe events on arm, problem is introduced by commit
>>> 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of
>>> .text"). For some architectures, label '_etext' is not in the .text
>>> section(in .notes section for arm/arm64). Label out of .text section is
>>> not loaded as symbols and we got a zero value when look up its address,
>>> which causes all events be wrongly skiped.
>>>
>>> This patch uses kernel map->end when failed to get the address of
>>> '_etext' and fixes the problem.
>> Masami, can't we always use map->end then? Can you please take a look at
>> this patch and ack/nack it?
>>
>> - Arnaldo
>
> I think _etext is more accurate than kernel map->end, because
> __map_groups__fixup_end() is called at the end of
> dso__load_sym(), which fixes map->end to next_map->start.
>
> Comparative result as this:
> etext_addr=ffffffff819a1b85, map->end=ffffffff81ff1000.
>
> So if possible, we should use _etext.

Hmm, this seems to have another problem. If etext_addr != map->end,
we can't relay on that. Is there any good way to get the ".text"
address range from symbol map? Until we find it, I'd rather like to
skip checking text address range on arm, because it looks meaningless :(

Thank you,

--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hi...@hitachi.com

[He Kuang]

hi, Masami

OK. Maybe I thought using kernel map->end(always > _etext) is a way to
skip checking .text range on arm ..

[Masami Hiramatsu]

Yeah, if map->end is always bigger than _etext, it is OK.
But I'm not sure that. In that case, we'd better add checking
etext_addr != 0 where comparing it.

Thank you,

--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hi...@hitachi.com

[He Kuang]

Fix failure to probe events on arm, problem is introduced by commit
5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of
.text"). For some architectures, label '_etext' is not in the .text
section(in .notes section for arm/arm64). Label out of .text section is
not loaded as symbols and we got a zero value when look up its address,
which causes all events be wrongly skiped.

This patch skip checking text address range when failed to get the

address of '_etext' and fixes the problem.

Problem can be reproduced on arm as following:

# perf probe --add='generic_perform_write'
generic_perform_write+0 is out of .text, skip it.
Probe point 'generic_perform_write' not found.
Error: Failed to add events.

After this patch:

# perf probe --add='generic_perform_write'
Added new event:
probe:generic_perform_write (on generic_perform_write)

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

perf record -e probe:generic_perform_write -aR sleep 1

Signed-off-by: He Kuang <hek...@huawei.com>
---

tools/perf/util/probe-event.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 076527b..381f23a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -249,8 +249,12 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
static bool kprobe_blacklist__listed(unsigned long address);
static bool kprobe_warn_out_range(const char *symbol, unsigned long address)
{
+ u64 etext_addr;
+
/* Get the address of _etext for checking non-probable text symbol */
- if (kernel_get_symbol_address_by_name("_etext", false) < address)
+ etext_addr = kernel_get_symbol_address_by_name("_etext", false);
+
+ if (etext_addr != 0 && etext_addr < address)
pr_warning("%s is out of .text, skip it.\n", symbol);
else if (kprobe_blacklist__listed(address))
pr_warning("%s is blacklisted function, skip it.\n", symbol);

[Arnaldo Carvalho de Melo]

Em Thu, Jun 18, 2015 at 02:49:10AM +0000, He Kuang escreveu:
> Fix failure to probe events on arm, problem is introduced by commit
> 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of
> .text"). For some architectures, label '_etext' is not in the .text
> section(in .notes section for arm/arm64). Label out of .text section is
> not loaded as symbols and we got a zero value when look up its address,
> which causes all events be wrongly skiped.
>
> This patch skip checking text address range when failed to get the
> address of '_etext' and fixes the problem.

Masami, since you guys discussed this patch, can I have your Acked-by?

- Arnaldo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

[Masami Hiramatsu]

On 2015/06/20 6:08, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 18, 2015 at 02:49:10AM +0000, He Kuang escreveu:
>> Fix failure to probe events on arm, problem is introduced by commit
>> 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of
>> .text"). For some architectures, label '_etext' is not in the .text
>> section(in .notes section for arm/arm64). Label out of .text section is
>> not loaded as symbols and we got a zero value when look up its address,
>> which causes all events be wrongly skiped.
>>
>> This patch skip checking text address range when failed to get the
>> address of '_etext' and fixes the problem.
>
> Masami, since you guys discussed this patch, can I have your Acked-by?

Oops, I missed the mails..

OK I'll send it.

Thanks!

--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hi...@hitachi.com

[Masami Hiramatsu]

On 2015/06/18 11:49, He Kuang wrote:
> Fix failure to probe events on arm, problem is introduced by commit
> 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of
> .text"). For some architectures, label '_etext' is not in the .text
> section(in .notes section for arm/arm64). Label out of .text section is
> not loaded as symbols and we got a zero value when look up its address,
> which causes all events be wrongly skiped.
>
> This patch skip checking text address range when failed to get the
> address of '_etext' and fixes the problem.
>
> Problem can be reproduced on arm as following:
>
> # perf probe --add='generic_perform_write'
> generic_perform_write+0 is out of .text, skip it.
> Probe point 'generic_perform_write' not found.
> Error: Failed to add events.
>
> After this patch:
>
> # perf probe --add='generic_perform_write'
> Added new event:
> probe:generic_perform_write (on generic_perform_write)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:generic_perform_write -aR sleep 1

Looks good to me :)

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>

Thanks!

>

> Signed-off-by: He Kuang <hek...@huawei.com>
> ---
> tools/perf/util/probe-event.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 076527b..381f23a 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -249,8 +249,12 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
> static bool kprobe_blacklist__listed(unsigned long address);
> static bool kprobe_warn_out_range(const char *symbol, unsigned long address)
> {
> + u64 etext_addr;
> +
> /* Get the address of _etext for checking non-probable text symbol */
> - if (kernel_get_symbol_address_by_name("_etext", false) < address)
> + etext_addr = kernel_get_symbol_address_by_name("_etext", false);
> +
> + if (etext_addr != 0 && etext_addr < address)
> pr_warning("%s is out of .text, skip it.\n", symbol);
> else if (kprobe_blacklist__listed(address))
> pr_warning("%s is blacklisted function, skip it.\n", symbol);
>


--

Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hi...@hitachi.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

[tip-bot for He Kuang]

Commit-ID: 7c31bb8c95ed269062ff7c7cc4a28b84a2b0f3a6
Gitweb: http://git.kernel.org/tip/7c31bb8c95ed269062ff7c7cc4a28b84a2b0f3a6
Author: He Kuang <hek...@huawei.com>
AuthorDate: Thu, 18 Jun 2015 02:49:10 +0000
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Tue, 23 Jun 2015 18:21:44 -0300

perf probe: Fix failure to probe events on arm

Fix failure to probe events on arm, the problem was introduced by commit

5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of .text").

For some architectures, the '_etext' label is not in the .text section
(in the .notes section for arm/arm64). Labels out of the .text section
are not loaded as symbols and we get a zero value when looking up its
addresses, which causes all events to be wrongly skipped.

This patch skips checking the text address range when failing to get the
address of '_etext' and thus fixes the problem.

The problem can be reproduced on arm as follows:

# perf probe --add='generic_perform_write'
generic_perform_write+0 is out of .text, skip it.
Probe point 'generic_perform_write' not found.
Error: Failed to add events.

After this patch:

# perf probe --add='generic_perform_write'
Added new event:
probe:generic_perform_write (on generic_perform_write)

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

perf record -e probe:generic_perform_write -aR sleep 1

Signed-off-by: He Kuang <hek...@huawei.com>

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Namhyung Kim <namh...@kernel.org>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Wang Nan <wang...@huawei.com>
Link: http://lkml.kernel.org/r/1434595750-129791-1-g...@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>

---
tools/perf/util/probe-event.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 076527b..381f23a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -249,8 +249,12 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
static bool kprobe_blacklist__listed(unsigned long address);
static bool kprobe_warn_out_range(const char *symbol, unsigned long address)
{
+ u64 etext_addr;
+
/* Get the address of _etext for checking non-probable text symbol */
- if (kernel_get_symbol_address_by_name("_etext", false) < address)
+ etext_addr = kernel_get_symbol_address_by_name("_etext", false);
+
+ if (etext_addr != 0 && etext_addr < address)
pr_warning("%s is out of .text, skip it.\n", symbol);
else if (kprobe_blacklist__listed(address))
pr_warning("%s is blacklisted function, skip it.\n", symbol);
--