All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@redhat.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>, LKML <linux-kernel@vger.kernel.org>,
	Lai Jiangshan <laijs@cn.fujitsu.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>,
	Jiaying Zhang <jiayingz@google.com>,
	Martin Bligh <mbligh@google.com>, Li Zefan <lizf@cn.fujitsu.com>,
	Jason Baron <jbaron@redhat.com>
Subject: Re: [RFD] Kprobes/Kretprobes perf support
Date: Wed, 12 Aug 2009 16:20:38 -0400	[thread overview]
Message-ID: <4A832416.2010209@redhat.com> (raw)
In-Reply-To: <20090812191305.GB5974@nowhere>



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@redhat.com


  reply	other threads:[~2009-08-12 20:17 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-11 18:48 [GIT PULL] tracing: Syscalls trace events + perf support Frederic Weisbecker
2009-08-11 18:48 ` [PATCH 01/16] tracing: Rename set_tracer_flags()'s local variable trace_flags Frederic Weisbecker
2009-08-11 18:48 ` [PATCH 02/16] tracing: Map syscall name to number Frederic Weisbecker
2009-08-11 18:48 ` [PATCH 03/16] tracing: Call arch_init_ftrace_syscalls at boot Frederic Weisbecker
2009-08-11 18:48 ` [PATCH 04/16] tracing: Add DECLARE_TRACE_WITH_CALLBACK() macro Frederic Weisbecker
2009-08-11 18:48 ` [PATCH 05/16] tracing: Add syscall tracepoints Frederic Weisbecker
2009-08-11 18:48 ` [PATCH 06/16] tracing: Update FTRACE_SYSCALL_MAX Frederic Weisbecker
2009-08-11 18:48 ` [PATCH 07/16] tracing: Raw_init() bailout in trace event register fail case Frederic Weisbecker
2009-08-11 18:48 ` [PATCH 08/16] tracing: Add ftrace_event_call void * 'data' field Frederic Weisbecker
2009-08-11 18:48 ` [PATCH 09/16] tracing: Add trace events for each syscall entry/exit Frederic Weisbecker
2009-08-11 18:48 ` [PATCH 10/16] tracing: Add individual syscalls tracepoint id support Frederic Weisbecker
2009-08-11 18:49 ` [PATCH 11/16] tracing: Add perf counter support for syscalls tracing Frederic Weisbecker
2009-08-11 18:49 ` [PATCH 12/16] tracing: Add more namespace area to 'perf list' output Frederic Weisbecker
2009-08-11 18:49 ` [PATCH 13/16] tracing: Convert x86_64 mmap and uname to use DEFINE_SYSCALL Frederic Weisbecker
2009-08-11 18:49 ` [PATCH 14/16] tracing: Add ftrace event call parameter to its field descriptor handler Frederic Weisbecker
2009-08-11 18:49 ` [PATCH 15/16] tracing: Add fields format definition for syscall events Frederic Weisbecker
2009-08-19 17:12   ` Masami Hiramatsu
2009-08-19 17:37     ` Frederic Weisbecker
2009-08-20  1:07       ` Li Zefan
2009-08-11 18:49 ` [PATCH 16/16] tracing: Support for syscall events raw records in perfcounters Frederic Weisbecker
2009-08-12  9:11 ` [GIT PULL] tracing: Syscalls trace events + perf support Ingo Molnar
2009-08-12 11:03   ` Ingo Molnar
2009-08-12 11:14     ` Ingo Molnar
2009-08-12 14:25       ` Jason Baron
2009-08-12 14:29         ` Ingo Molnar
2009-08-12 14:37           ` Jason Baron
2009-08-12 11:33     ` Frederic Weisbecker
2009-08-12 13:59       ` Jason Baron
2009-08-12 14:30         ` Ingo Molnar
2009-08-18  0:46   ` Paul Mundt
2009-08-18  7:32     ` Ingo Molnar
2009-08-18  7:32       ` Ingo Molnar
2009-08-18  8:51       ` [S390] ftrace: update system call tracer support Ingo Molnar
2009-08-18  8:59         ` Martin Schwidefsky
2009-08-18 10:05           ` Ingo Molnar
2009-08-18 10:22             ` Martin Schwidefsky
2009-08-18 14:56         ` Jason Baron
2009-08-18  8:51       ` Ingo Molnar
2009-08-18 10:25     ` [GIT PULL] tracing: Syscalls trace events + perf support Frederic Weisbecker
2009-08-18 10:25       ` Frederic Weisbecker
2009-08-18 11:06       ` Ingo Molnar
2009-08-18 11:56         ` Paul Mundt
2009-08-12 16:33 ` Masami Hiramatsu
2009-08-12 17:02   ` Masami Hiramatsu
2009-08-12 19:13     ` [RFD] Kprobes/Kretprobes " Frederic Weisbecker
2009-08-12 20:20       ` Masami Hiramatsu [this message]
2009-08-13  8:02         ` Ingo Molnar
2009-08-12 21:09       ` Peter Zijlstra
2009-08-12 21:27         ` Masami Hiramatsu
2009-08-12 21:37           ` Frederic Weisbecker
2009-08-12 21:35         ` Frederic Weisbecker
2009-08-14 15:05       ` Masami Hiramatsu
2009-08-15 14:33         ` Ingo Molnar
2009-08-17 21:58           ` Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A832416.2010209@redhat.com \
    --to=mhiramat@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=jbaron@redhat.com \
    --cc=jiayingz@google.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=mathieu.desnoyers@polymtl.ca \
    --cc=mbligh@google.com \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.