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 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.