From: Jason Baron <jbaron@redhat.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Tom Zanussi <tzanussi@gmail.com>,
linux-kernel@vger.kernel.org, fweisbec@gmail.com,
laijs@cn.fujitsu.com, rostedt@goodmis.org, peterz@infradead.org,
mathieu.desnoyers@polymtl.ca, jiayingz@google.com,
mbligh@google.com, roland@redhat.com, fche@redhat.com
Subject: Re: [RFC] convert ftrace syscall tracer to TRACE_EVENT()
Date: Mon, 11 May 2009 18:16:59 -0400 [thread overview]
Message-ID: <20090511221659.GA3223@redhat.com> (raw)
In-Reply-To: <20090509083737.GE3656@elte.hu>
On Sat, May 09, 2009 at 10:37:37AM +0200, Ingo Molnar wrote:
> I'm not sure about the implementation as you've posted it though:
>
> Firstly, it adds two new tracepoints to every system call. That is
> unnecessary - we already have the TIF flag based callbacks, and we
> can use the existing syscall attributes table to get to tracepoints
> - without slow down (or impacting) the fast path in any way.
>
> Secondly, we should reuse the information we get in SYSCALL_DEFINE,
> to construct the TRACE_EVENT tracepoints directly - without having
> to list all syscalls again in a separate file.
>
> Ingo
ok, i've been playing around with this a bit...by adding a few macros to
include/trace/syscalls.h (conceptually in addition to the previous patch i
posted), I can address #1. For example, for getpid() I did:
+#define MAX_SYS_ARGS 10
+
+#define create_syscall_case_args0(sysnr, regs, sysname) \
+ case sysnr: \
+ trace_sysenter_##sysname(); \
+ break;
+
+#define create_syscall_case_args1(sysnr, regs, sysname) \
+ case sysnr: \
+ syscall_get_arguments(current, regs, 0, 1, sys_args); \
+ trace_sysenter_#sysname((meta->types[0])sys_args[0]); \
+ break;
+
+#define define_syscall_tracepoints() \
+ DEFINE_TRACE(sysenter_getpid);
+
+#define wrapper_syscall_tracepoints_enter(regs) \
+do { \
+ int syscall_nr; \
+ long sys_args[MAX_SYS_ARGS]; \
+ struct syscall_metadata *meta; \
+ \
+ syscall_nr = syscall_get_nr(current, regs); \
+ meta = syscall_nr_to_meta(syscall_nr); \
+ switch (syscall_nr) { \
+ create_syscall_case_args0(__NR_getpid, regs, getpid);\
+ } \
+} while (0)
This should extend to the rest of the syscalls.
Then, in arch/x86/kernel/ptrace.c, i just add:
if (unlikely(test_thread_flag(TIF_SYSCALL_FTRACE)))
- ftrace_syscall_enter(regs);
+ wrapper_syscall_tracepoints_enter(regs);
Regarding issue #2, the '__SYSCALL_DEFINEx()' macros are expanding in
the context of the various .c source files that define the system calls.
Thus, I'm not sure how we are going to reference them in
arch/x86/kernel/ptrace.c. Also, by not defining the tracepoints in a .h
file, modules and other code that want to make use of these tracepoints
are going to have a harder time. Further, there has been mention in this
thread of exceptions that some of the syscalls are going to create. I
think it would be easier to follow the exceptions if they are contained
in 1 file, rather than scattered around the code.
thanks,
-Jason
next prev parent reply other threads:[~2009-05-11 22:20 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-08 21:03 [RFC] convert ftrace syscall tracer to TRACE_EVENT() Jason Baron
2009-05-09 8:37 ` Ingo Molnar
2009-05-09 12:53 ` Frédéric Weisbecker
2009-05-09 13:33 ` Ingo Molnar
2009-05-09 13:50 ` Mathieu Desnoyers
2009-05-09 14:06 ` Frédéric Weisbecker
2009-05-09 14:15 ` Ingo Molnar
2009-05-09 14:29 ` Mathieu Desnoyers
2009-05-09 15:01 ` Frédéric Weisbecker
2009-05-09 15:24 ` Mathieu Desnoyers
2009-05-09 14:47 ` Frédéric Weisbecker
2009-05-09 17:44 ` David Wagner
2009-05-09 14:02 ` Frédéric Weisbecker
2009-05-09 14:07 ` Ingo Molnar
2009-05-09 14:12 ` Frédéric Weisbecker
2009-05-09 15:36 ` Frank Ch. Eigler
2009-05-09 15:57 ` Mathieu Desnoyers
2009-05-09 16:32 ` Mathieu Desnoyers
2009-05-10 6:59 ` Tom Zanussi
2009-05-11 22:16 ` Jason Baron [this message]
2009-05-12 2:44 ` Roland McGrath
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=20090511221659.GA3223@redhat.com \
--to=jbaron@redhat.com \
--cc=fche@redhat.com \
--cc=fweisbec@gmail.com \
--cc=jiayingz@google.com \
--cc=laijs@cn.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@polymtl.ca \
--cc=mbligh@google.com \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=roland@redhat.com \
--cc=rostedt@goodmis.org \
--cc=tzanussi@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox