From: =?utf-8?B?5bmz5p2+6ZuF5bezIC8gSElSQU1BVFXvvIxNQVNBTUk=?= 
	<masami.hiramatsu.pt@hitachi.com>
To: "'ltc-kernel@ml.yrl.intra.hitachi.co.jp'" 
	<ltc-kernel@ml.yrl.intra.hitachi.co.jp>,
        "'acme@kernel.org'" <acme@kernel.org>
CC: "'Wang Nan'" <wangnan0@huawei.com>,
        "namhyung@kernel.org" <namhyung@kernel.org>,
        "lizefan@huawei.com" <lizefan@huawei.com>,
        "pi3orama@163.com" <pi3orama@163.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "jolsa@kernel.org" <jolsa@kernel.org>
Subject: RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map()
 which incorrectly returns success
Thread-Topic: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map()
 which incorrectly returns success
Thread-Index: AQHRF8ymF/ZtKuEPKE6WPBpva1M0JZ6Naimw//+VagCAAYlX0IAACF/w
Date: Fri, 6 Nov 2015 07:12:12 +0000
Message-ID: <50399556C9727B4D88A595C8584AAB37526070A4@GSjpTKYDCembx32.service.hitachi.net>
References: <1446729565-27592-1-git-send-email-wangnan0@huawei.com>
 <1446729565-27592-3-git-send-email-wangnan0@huawei.com>
 <50399556C9727B4D88A595C8584AAB3752604844@GSjpTKYDCembx32.service.hitachi.net>
 <20151105160000.GX13236@kernel.org>
 <50399556C9727B4D88A595C8584AAB3752606DC9@GSjpTKYDCembx32.service.hitachi.net>
In-Reply-To: <50399556C9727B4D88A595C8584AAB3752606DC9@GSjpTKYDCembx32.service.hitachi.net>
Accept-Language: ja-JP, en-US
Content-Language: ja-JP
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Transfer-Encoding: 8bit
Content-Length: 2393
Lines: 53

From: acme@kernel.org [mailto:acme@kernel.org]
>>
>>Em Thu, Nov 05, 2015 at 02:08:48PM +0000, 平松雅巳 / HIRAMATU，MASAMI escreveu:
>>> From: Wang Nan [mailto:wangnan0@huawei.com]
>>> >
>>> >It is possible that find_perf_probe_point_from_map() fails to find
>>> >symbol but still returns 0 because of an small error when coding:
>>> >find_perf_probe_point_from_map() set 'ret' to error code at first,
>>> >but also use it to hold return value of
>>> >kernel_get_symbol_address_by_name().
>>>
>>> OK, I didn't expect that there is a symbol which can be found by
>>> kernel_get_symbol_address_by_name() but not by __find_kernel_function()...
>>
>>> Would you have any example of the error?
>>>
>>> >
>>> >This patch resets 'ret' to error even kernel_get_symbol_address_by_name()
>>> >success, so if !sym, the whole function returns error correctly.
>>>
>>> Hmm, that sounds tricky. I'd rather like to add *psym to kernel_get_symbol_address_by_name()
>>> to save symbol and don't use __find_kernel_function() instead.
>>
>>Tricky? I don't think so, suboptimal? possibly, but it fixes an error,
>>so should be processed quickly, right? I'm applying his patch and then
>>whatever improvement can be done on top.
>
>OK, then I'll send an improvement patch.

Ah, finally I got what happened. I guess the problem may happen when we put
a probe on the kernel somewhere outside of any functions and run "perf probe -l".
I think it should not be allowed to put the probe outside any symbol.

The background is here, at first "perf-probe -a somewhere" defines a probe in
the kernel but its address is relative from "_text". (thus, vfs_read becomes "_text+2348080"
 for example). Since it is not readable by human, perf probe -l tries to get an appropriate
symbol from the "_text+OFFSET".
For the purpose, the first kernel_get_symbol_address_by_name() is for translating _text to
an address, and the second  __find_kernel_function() is for finding a symbol from the
address+OFFSET.
Then, if the address+OFFSET is out of the symbol map, the second one can fail.
This means the first symbol and the second symbol is not same.

So, the direction of Wang solution is good :). Just a cleanup is required.

Thank you!

>
>Thanks,
>
>>
>>- Arnaldo
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?