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

[Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>]

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 <hekuang@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 majordomo@vger.kernel.org
>>>> 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 majordomo@vger.kernel.org
>>> 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 majordomo@vger.kernel.org
> 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: masami.hiramatsu.pt@hitachi.com