Subject: Re: [PATCH] perf probe: Fix failure to probe events on arm
To: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
        hekuang <hekuang@zoho.com>, Arnaldo Carvalho de Melo <acme@kernel.org>
References: <1434355613-110216-1-git-send-email-hekuang@huawei.com>
 <20150615144911.GA5845@kernel.org> <55804023.1080409@zoho.com>
 <5581355B.3010906@hitachi.com>
CC: <a.p.zijlstra@chello.nl>, <mingo@redhat.com>, <namhyung@kernel.org>,
        <wangnan0@huawei.com>, <linux-kernel@vger.kernel.org>
From: He Kuang <hekuang@huawei.com>
Message-ID: <5581414E.2070903@huawei.com>
Date: Wed, 17 Jun 2015 17:43:42 +0800
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101
 Thunderbird/38.0
MIME-Version: 1.0
In-Reply-To: <5581355B.3010906@hitachi.com>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4409
Lines: 120

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 <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/