Message-ID: <4A832416.2010209@redhat.com>
Date:	Wed, 12 Aug 2009 16:20:38 -0400
From:	Masami Hiramatsu <mhiramat@...hat.com>
To:	Frederic Weisbecker <fweisbec@...il.com>
CC:	Ingo Molnar <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>,
	Jiaying Zhang <jiayingz@...gle.com>,
	Martin Bligh <mbligh@...gle.com>,
	Li Zefan <lizf@...fujitsu.com>, Jason Baron <jbaron@...hat.com>
Subject: Re: [RFD] Kprobes/Kretprobes perf support



Frederic Weisbecker wrote:
> On Wed, Aug 12, 2009 at 01:02:24PM -0400, Masami Hiramatsu wrote:
>>
>> Masami Hiramatsu wrote:
>>> Hi Frederic and Jason,
>>>
>>> Frederic Weisbecker wrote:
>>>> Frederic Weisbecker (3):
>>>>          tracing: Add ftrace event call parameter to its field descriptor handler
>>>> Jason Baron (12):
>>>>          tracing: Add ftrace_event_call void * 'data' field
>>> Both of you added a parameter to ftrace_event_call for passing
>>> sycall name (call->data) to handlers, but one passes 'ftrace_event_call *'
>>> and another passes 'void *'. It seems not enough unified.
>>>
>>> And also, I'm now updating my patch for 'dynamic ftrace_event_call'
>>> http://lkml.org/lkml/2009/7/24/234
>>> which adds 'ftrace_event_call *' for all handlers.
>>>
>>> I think passing 'ftrace_event_call *' is more generic way
>>> to do that. What would you think about that?
>> Hmm, I changed my mind that passing 'void *' is enough, since
>> all other fields of ftrace_event_call will be handled in
>> trace_events.c.
>>
>> Thank you,
>
>
>
> Well, actually I agree with you because:
>
> - struct ftrace_event_call * is typed and let the compiler
>    be able to perform basic type checks.
>    (Even though that only delays the use of a void * type through
>    call->data)
>
> - Further dynamic trace events might need other fields of struct ftrace_event_call *

Hmm, so would you think passing 'struct ftrace_event_call *' is better?

> While adding the struct ftrace_event_call * as parameter in the show_format
> callback yesterday, I first thought about applying your "dynamic ftrace
> event creation" patch.
>
> But it was just too much for what I needed.

Sure, syscall events can be defined in build time.

> Speaking about your patches. You told recently you would be willing
> to implement a perf support for kprobes, right? :-)

Hmm, perhaps, I meant a profiling interface(http://lkml.org/lkml/2009/7/24/240).
However, that is interesting idea too.

> I've thought about how to do that.
> Ftrace events are supported by perfcounter currently but Kprobes
> dynamic ftrace events are of a different nature: we must create them
> before any toggling.
>
> So a large part is already done through the ftrace events and the fact
> that you create one dynamically for each kprobes (we'll just need
> a little callback for perf sample submission but that's a small
> point).

Sure, even current implementation has some difference from tracepoint
events... (currently, all of those kprobes events shares same event id,
and each event can be identified by the event ip address)


> The largest work that remains is to port the current powerful interface
> to create these k{ret}probes (with requested  arguments, etc...) through
> ftrace but using perf open syscall.
>
> And I imagine it won't be trivial.
>
> Ingo, Peter do you have an idea on how we could do that?
> We should be able to choose between a kprobe and kretprobe (these can
> be two separate counters). And also one must be able to request the dump
> of random desired parameters (or return values in case of kretprobe)
> or registers...
>
> May be we should use the perf attr by passing a __user address to a buffer
> that contains all these options?
> Once we get that to the kernel, that can be passed to ftrace-kprobe that
> can parse it, create the desired trace event and rely on perf to create
> a counter for it.
>
> I guess that won't imply so much adds to Masami's patchset. Most of
> the work is on the perf tools (parsing the user request).
>
> ./perf kprobes -e (func|addr):(c|r):(a1,a2,a3,... | rax,rbx,rcx,...)
>                                ^  ^
>                             c = call = kprobe
>                             r = return = kretprobe
>

It could work. can it support some dereference format, like as +4(%sp)?

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@...hat.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists