From: Masami Hiramatsu <mhiramat@redhat.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>, Steven Rostedt <rostedt@goodmis.org>,
lkml <linux-kernel@vger.kernel.org>,
systemtap <systemtap@sources.redhat.com>,
kvm <kvm@vger.kernel.org>,
DLE <dle-develop@lists.sourceforge.net>,
Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
Christoph Hellwig <hch@infradead.org>,
Tom Zanussi <tzanussi@gmail.com>
Subject: Re: [PATCH -tip -v10 7/7] tracing: add kprobe-based event tracer
Date: Tue, 07 Jul 2009 19:52:00 -0000 [thread overview]
Message-ID: <4A53A830.3000305@redhat.com> (raw)
In-Reply-To: <20090707073103.GA6173@nowhere>
Frederic Weisbecker wrote:
>> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
>> index 206cb7d..65945eb 100644
>> --- a/kernel/trace/trace.h
>> +++ b/kernel/trace/trace.h
>> @@ -45,6 +45,8 @@ enum trace_type {
>> TRACE_POWER,
>> TRACE_BLK,
>> TRACE_KSYM,
>> + TRACE_KPROBE,
>> + TRACE_KRETPROBE,
>>
>> __TRACE_LAST_TYPE,
>> };
>> @@ -227,6 +229,22 @@ struct trace_ksym {
>> char ksym_name[KSYM_NAME_LEN];
>> char p_name[TASK_COMM_LEN];
>> };
>> +#define TRACE_KPROBE_ARGS 6
>> +
>> +struct kprobe_trace_entry {
>> + struct trace_entry ent;
>> + unsigned long ip;
>> + int nargs;
>> + unsigned long args[TRACE_KPROBE_ARGS];
>
>
>
> I see that you actually make use of arg as a dynamic sizeable
> array.
> For clarity, args[TRACE_KPROBE_ARGS] could be args[0].
>
> It's just a neat and wouldn't affect the code nor the data
> but would be clearer for readers of that code.
Hmm. In that case, I think we'll need a new macro for field
definition, like TRACE_FIELD_ZERO(type, item).
>> +};
>> +
>> +struct kretprobe_trace_entry {
>> + struct trace_entry ent;
>> + unsigned long func;
>> + unsigned long ret_ip;
>> + int nargs;
>> + unsigned long args[TRACE_KPROBE_ARGS];
>> +};
>
>
> ditto
>
>
>
>> /*
>> * trace_flag_type is an enumeration that holds different
>> @@ -344,6 +362,10 @@ extern void __ftrace_bad_type(void);
>> IF_ASSIGN(var, ent, struct syscall_trace_exit, \
>> TRACE_SYSCALL_EXIT); \
>> IF_ASSIGN(var, ent, struct trace_ksym, TRACE_KSYM); \
>> + IF_ASSIGN(var, ent, struct kprobe_trace_entry, \
>> + TRACE_KPROBE); \
>> + IF_ASSIGN(var, ent, struct kretprobe_trace_entry, \
>> + TRACE_KRETPROBE); \
>> __ftrace_bad_type(); \
>> } while (0)
>>
>> diff --git a/kernel/trace/trace_event_types.h b/kernel/trace/trace_event_types.h
>> index 6db005e..ec2e6f3 100644
>> --- a/kernel/trace/trace_event_types.h
>> +++ b/kernel/trace/trace_event_types.h
>> @@ -175,4 +175,24 @@ TRACE_EVENT_FORMAT(kmem_free, TRACE_KMEM_FREE, kmemtrace_free_entry, ignore,
>> TP_RAW_FMT("type:%u call_site:%lx ptr:%p")
>> );
>>
>> +TRACE_EVENT_FORMAT(kprobe, TRACE_KPROBE, kprobe_trace_entry, ignore,
>> + TRACE_STRUCT(
>> + TRACE_FIELD(unsigned long, ip, ip)
>> + TRACE_FIELD(int, nargs, nargs)
>> + TRACE_FIELD_SPECIAL(unsigned long args[TRACE_KPROBE_ARGS],
>> + args, TRACE_KPROBE_ARGS, args)
>> + ),
>> + TP_RAW_FMT("%08lx: args:0x%lx ...")
>> +);
>> +
>> +TRACE_EVENT_FORMAT(kretprobe, TRACE_KRETPROBE, kretprobe_trace_entry, ignore,
>> + TRACE_STRUCT(
>> + TRACE_FIELD(unsigned long, func, func)
>> + TRACE_FIELD(unsigned long, ret_ip, ret_ip)
>> + TRACE_FIELD(int, nargs, nargs)
>> + TRACE_FIELD_SPECIAL(unsigned long args[TRACE_KPROBE_ARGS],
>> + args, TRACE_KPROBE_ARGS, args)
>> + ),
>> + TP_RAW_FMT("%08lx <- %08lx: args:0x%lx ...")
>> +);
>> #undef TRACE_SYSTEM
>> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
>> new file mode 100644
>> index 0000000..0951512
>> --- /dev/null
>> +++ b/kernel/trace/trace_kprobe.c
>> @@ -0,0 +1,1183 @@
>> +/*
>> + * kprobe based kernel tracer
>> + *
>> + * Created by Masami Hiramatsu <mhiramat@redhat.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/kprobes.h>
>> +#include <linux/seq_file.h>
>> +#include <linux/slab.h>
>> +#include <linux/smp.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/types.h>
>> +#include <linux/string.h>
>> +#include <linux/ctype.h>
>> +#include <linux/ptrace.h>
>> +
>> +#include "trace.h"
>> +#include "trace_output.h"
>> +
>> +#define MAX_ARGSTR_LEN 63
>> +
>> +/* currently, trace_kprobe only supports X86. */
>> +
>> +struct fetch_func {
>> + unsigned long (*func)(struct pt_regs *, void *);
>> + void *data;
>> +};
>> +
>> +static __kprobes unsigned long call_fetch(struct fetch_func *f,
>> + struct pt_regs *regs)
>> +{
>> + return f->func(regs, f->data);
>> +}
>> +
>> +/* fetch handlers */
>> +static __kprobes unsigned long fetch_register(struct pt_regs *regs,
>> + void *offset)
>> +{
>> + return regs_get_register(regs, (unsigned)((unsigned long)offset));
>> +}
>> +
>> +static __kprobes unsigned long fetch_stack(struct pt_regs *regs,
>> + void *num)
>> +{
>> + return regs_get_kernel_stack_nth(regs, (unsigned)((unsigned long)num));
>
>
> You seem to often use unsigned as an implicit type.
> Would be better to explicitly use unsigned int.
Agreed.
> Anyway, I guess we can merge it in -tip and give it a try.
Thank you!
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: mhiramat@redhat.com
next prev parent reply other threads:[~2009-07-07 19:52 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-01 1:05 [PATCH -tip -v10 0/7] tracing: kprobe-based event tracer and x86 instruction decoder Masami Hiramatsu
2009-07-01 1:05 ` [PATCH -tip -v10 1/7] x86: instruction decoder API Masami Hiramatsu
2009-07-01 1:05 ` [PATCH -tip -v10 2/7] x86: x86 instruction decoder build-time selftest Masami Hiramatsu
2009-07-01 1:06 ` [PATCH -tip -v10 3/7] kprobes: checks probe address is instruction boudary on x86 Masami Hiramatsu
2009-07-01 1:06 ` [PATCH -tip -v10 7/7] tracing: add kprobe-based event tracer Masami Hiramatsu
[not found] ` <20090707073103.GA6173@nowhere>
2009-07-07 19:52 ` Masami Hiramatsu [this message]
2009-07-07 20:20 ` Frederic Weisbecker
2009-07-07 20:39 ` Masami Hiramatsu
2009-07-07 20:59 ` Frederic Weisbecker
2009-07-07 21:28 ` Masami Hiramatsu
2009-07-07 21:35 ` Frederic Weisbecker
2009-07-07 21:39 ` Masami Hiramatsu
2009-07-07 21:58 ` Masami Hiramatsu
2009-07-01 1:06 ` [PATCH -tip -v10 4/7] kprobes: cleanup fix_riprel() using insn decoder on x86 Masami Hiramatsu
2009-07-01 1:17 ` [PATCH -tip -v10 6/7] tracing: ftrace dynamic ftrace_event_call support Masami Hiramatsu
2009-07-06 2:00 ` Frederic Weisbecker
2009-07-01 1:17 ` [PATCH -tip -v10 5/7] x86: add pt_regs register and stack access APIs Masami Hiramatsu
2009-07-06 1:42 ` Frederic Weisbecker
2009-07-06 14:37 ` Andi Kleen
2009-07-06 19:25 ` Masami Hiramatsu
2009-07-06 20:06 ` Andi Kleen
2009-07-07 0:04 ` 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=4A53A830.3000305@redhat.com \
--to=mhiramat@redhat.com \
--cc=ananth@in.ibm.com \
--cc=dle-develop@lists.sourceforge.net \
--cc=fweisbec@gmail.com \
--cc=hch@infradead.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=rostedt@goodmis.org \
--cc=systemtap@sources.redhat.com \
--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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).