Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

[Masami Hiramatsu <mhiramat@redhat.com>]

Ingo Molnar wrote:
>>> Here are a few syntax suggestions
>>>
>>> The simpest probe syntax should be to add a probe to a single 
>>> function name:
>>>
>>>   perf probe +schedule
>>>
>>> _nothing else_.
>>>
>>> To remove it, the user should just do something like:
>>>
>>>   perf probe -schedule
>>>
>>> (to be symmetric 'perf probe +schedule' should work as well)
>>
>> I think '-<symbol>' syntax doesn't work good with other command-line 
>> options and multiple definitions. (However, it will be good for 
>> input-from-file syntax. :-))
> 
> dash can be used too - perf has the options library from Git and there's 
> a wide spectrum of option parsing available, via 
> tools/perf/util/parse-options.h.
> 
> But yes, it complicates things a bit.

Yeah, what I'm concerning about is that user will confuse when deleting
probe points which starts with other option, like 'k'.
(-kmalloc can mean -k malloc too)

>> So, what would you think about using -D (def) and -U (undef) ?
> 
> The simpest case should be no extra character at all:
> 
>   perf probe schedule
> 
> There's a few well-known command line idioms to add/remove stuff, but -D 
> / -U is not one of them i'm afraid =B-)
> 
> The following ones might work too:
> 
>   perf probe +schedule
>   perf probe -schedule
> 
>   perf probe add schedule
>   perf probe del schedule
> 
>   perf probe --add schedule
>   perf probe --del schedule
> 
> [ Plain 'add/del' has a minor complication as we could have a similar 
>   symbol defined. ]
> 
> + / - is certainly the simplest.
> 
> --add/--del works like routes do, so that's intuitive as well. As long 
> as we have the simple default to add a new probe at a function, without 
> any extra options we can do this too initially.

How about the following syntax?
<adding>
perf probe schedule
perf probe --add schedule

<deleting>
perf probe --del schedule
perf probe --del all	/* delete all probepoints */

So, this doesn't symmetric, but provides simple way to add a probe.

>>> All the other extensions and possibilities - arguments, variables, 
>>> source code lines, etc. should be natural and intuitive extensions 
>>> of this basic, minimal syntax.
>>
>> Don't you like current space(' ') separated arguments? :-) I mean, 
>> what is 'natural' syntax in your opinion?
> 
> Yeah, space separated arguments are nice too. The question is how to 
> specify a more precise coordinate for the bit we want to probe - and how 
> to specify the information we want to extract. Something like:
> 
>   perf schedule+15
> 
> would be a rather intuitive shortcut for '15 lines into the schedule() 
> function' - and it might even be a largely cross-kernel-version 
> compatible way of specifying probe points.

I agreed with the cross-kernel-version issue. I'd rather like

 perf probe symbol:relative-line

and

 perf probe file:absolute-line

since it will be familiar for GDB users.

And I'd like to preserve

 perf probe symbol+offs-byte

for assembly users who might want to trace assembly code with
objdump.

> Or this:
> 
>   perf schedule:'switch_count = &prev->nivcsw'
> 
> would insert the probe to the source code that matches that statement 
> pattern. Rarely will people want to insert a probe to an absolutely line 
> number - that's a usage mode for higher level tools. (so we definitely 
> want to support it - but it should not use up valuable spots in our 
> options space.) Same goes for symbol offsets, etc. - humans will rarely 
> use them.

Hmm, maybe, it's possible. I should investigate dwarf more...


Thank you!

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com