lkml.org 
[lkml]   [2015]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 1/3] perf probe: Init symbol as kprobe if any pev is kprobe


On 2015/6/27 16:30, Masami Hiramatsu wrote:
> Hi Wang,
>
> On 2015/06/27 16:34, Wangnan (F) wrote:
>>
>> On 2015/6/27 15:29, Masami Hiramatsu wrote:
>>> On 2015/06/25 19:37, Wang Nan wrote:
>>>> Before this patch, add_perf_probe_events() init symbol maps only for
>>>> uprobe if the first pev passed to it is a uprobe event. However, with
>>>> the incoming BPF uprobe support, now it will be possible to pass an
>>>> array with combined kprobe and uprobe events to add_perf_probe_events().
>>> This description is not correct. Actually, add_perf_probe_events already
>>> supports mix of uprobes and kprobes. However, from the command line syntax
>>> constrains the first elements of the probe_event arrays must be kprobes.
>>> So, if the array starts with uprobes, no kprobes should be there.
>>>
>>>> This patch check all pevs instead of the first one, and init kernel
>>>> symbol if any events is not uprobe.
>>> Anyway, I prefer to call init_symbol_maps() with "false" :)
>> I also prefer "false", so I try to find whether all events are uprobe
>> instead
>> of init_symbol_maps(true).
>>
>> So for this patch only commit message needs to be corrected, the code is
>> no problem, right?
>>
> No, I meant you just need to change one line, as below.
>
> - ret = init_symbol_maps(pevs->uprobes);
> + ret = init_symbol_maps(false);
>
> That's enough, I don't like to introduce a bool flag and loop.
>
> Thanks,
>

Then basic structure of kernel map will be build even if all probe
points are
uprobes. It costs some memory. But sure, we can skip the loop.

Will post an updated version.

Thank you.

>
>> Thank you.
>>
>>> Thank you,
>>>
>>>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>>>> ---
>>>> tools/perf/util/probe-event.c | 15 ++++++++++++++-
>>>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>>>> index b386d2f..a2b3026 100644
>>>> --- a/tools/perf/util/probe-event.c
>>>> +++ b/tools/perf/util/probe-event.c
>>>> @@ -2802,8 +2802,21 @@ int cleanup_perf_probe_event(struct perf_probe_event *pev)
>>>> int add_perf_probe_events(struct perf_probe_event *pevs, int npevs, bool cleanup)
>>>> {
>>>> int i, ret;
>>>> + bool user_only = true;
>>>>
>>>> - ret = init_symbol_maps(pevs->uprobes);
>>>> + /* If any pev is kprobe, init kernel symbols. */
>>>> + for (i = 0; i < npevs; i++) {
>>>> + if (!pevs[i].uprobes) {
>>>> + user_only = false;
>>>> + break;
>>>> + }
>>>> + }
>>>> +
>>>> + /*
>>>> + * Compiler can drop user_only:
>>>> + * ret = init_symbol_maps(i >= npevs);
>>>> + */
>>>> + ret = init_symbol_maps(user_only);
>>>> if (ret < 0)
>>>> return ret;
>>>>
>>>>
>>
>>
>




\
 
 \ /
  Last update: 2015-06-27 13:21    [W:0.043 / U:0.876 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site