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 <[email protected]>
---
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
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
> 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 <[email protected]>
> ---
> 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
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 <[email protected]>
Thanks!
>
> - Arnaldo
>
>
>> 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 <[email protected]>
>> ---
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]
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 <[email protected]>
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
>
> Thanks!
>
> >
> > - Arnaldo
> >
> >
> >> 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 <[email protected]>
> >> ---
> >> 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 [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>
>
> --
> Masami HIRAMATSU
> Linux Technology Research Center, System Productivity Research Dept.
> Center for Technology Innovation - Systems Engineering
> Hitachi, Ltd., Research & Development Group
> E-mail: [email protected]
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.
>
>> 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 <[email protected]>
>> ---
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
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,
>
> Thanks.
>>
>>> 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 <[email protected]>
>>> ---
>>> 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 [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]
hi, Masami
On 2015/6/17 16:52, Masami Hiramatsu wrote:
> 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 :(
>
OK. Maybe I thought using kernel map->end(always > _etext) is a way to
skip checking .text range on arm ..
> Thank you,
>
>>
>> Thanks.
>>>
>>>> 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 <[email protected]>
>>>> ---
>>>> 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 [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>
>
On 2015/06/17 18:43, He Kuang wrote:
> hi, Masami
>
> On 2015/6/17 16:52, Masami Hiramatsu wrote:
>> 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 :(
>>
>
> OK. Maybe I thought using kernel map->end(always > _etext) is a way to
> skip checking .text range on arm ..
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,
>
>
>> Thank you,
>>
>>>
>>> Thanks.
>>>>
>>>>> 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 <[email protected]>
>>>>> ---
>>>>> 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 [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>> Please read the FAQ at http://www.tux.org/lkml/
>>>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]
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 <[email protected]>
---
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);
--
1.8.5.2
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
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: [email protected]
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 <[email protected]>
Thanks!
>
> Signed-off-by: He Kuang <[email protected]>
> ---
> 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: [email protected]
Commit-ID: 7c31bb8c95ed269062ff7c7cc4a28b84a2b0f3a6
Gitweb: http://git.kernel.org/tip/7c31bb8c95ed269062ff7c7cc4a28b84a2b0f3a6
Author: He Kuang <[email protected]>
AuthorDate: Thu, 18 Jun 2015 02:49:10 +0000
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Wang Nan <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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);