Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757103AbZGGTxk (ORCPT ); Tue, 7 Jul 2009 15:53:40 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754753AbZGGTxb (ORCPT ); Tue, 7 Jul 2009 15:53:31 -0400 Received: from mx2.redhat.com ([66.187.237.31]:39466 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751418AbZGGTxa (ORCPT ); Tue, 7 Jul 2009 15:53:30 -0400 Message-ID: <4A53A830.3000305@redhat.com> Date: Tue, 07 Jul 2009 15:55:28 -0400 From: Masami Hiramatsu User-Agent: Thunderbird 2.0.0.21 (X11/20090320) MIME-Version: 1.0 To: Frederic Weisbecker CC: Ingo Molnar , Steven Rostedt , lkml , systemtap , kvm , DLE , Ananth N Mavinakayanahalli , Christoph Hellwig , Tom Zanussi Subject: Re: [PATCH -tip -v10 7/7] tracing: add kprobe-based event tracer References: <20090701010838.32547.62843.stgit@localhost.localdomain> <20090701010923.32547.74203.stgit@localhost.localdomain> <20090707073103.GA6173@nowhere> In-Reply-To: <20090707073103.GA6173@nowhere> X-Enigmail-Version: 0.95.7 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5619 Lines: 187 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 >> + * >> + * 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/