[PATCH 0/7] V2 add syscall tracepoints

[PATCH 0/7] V2 add syscall tracepoints

[Jason Baron]

hi,

The following is an implementation of Frederic's syscall tracer on top of
tracepoints. It adds the ability to toggle the entry/exit of each syscall
via the standard events/syscalls/syscall_blah/enable interface. The
implementation is done by adding 2 tracepoints. One on entry and one for exit.

I've tried to address all of the comments from the last posting...updates include:
-2/7: make arch_init_ftrace_syscalls static, removed locking, make patch bi-sectable
-3/7: rename DECLARE_TRACE_REG as DECLARE_TRACE_WITH_CALLBACK
-4/7: make mutex and counter static
-7/7: add event tracing for syscalls that take no args

thanks,

-Jason

 arch/x86/include/asm/ftrace.h |    4 +-
 arch/x86/kernel/ftrace.c      |   31 +++++---
 arch/x86/kernel/ptrace.c      |    6 +-
 include/linux/syscalls.h      |   75 +++++++++++++++++-
 include/linux/tracepoint.h    |   29 ++++++-
 include/trace/syscall.h       |   37 +++++++---
 kernel/trace/trace_events.c   |   29 +++++---
 kernel/trace/trace_syscalls.c |  172 +++++++++++++++++++----------------------
 kernel/tracepoint.c           |   38 +++++++++
 9 files changed, 289 insertions(+), 132 deletions(-)

[PATCH 1/7] V2 add syscall tracepoints

[Jason Baron]

Add a new function to support translating a syscall name to number at runtime.
This allows the syscall event tracer to map syscall names to number.


Signed-off-by: Jason Baron <jbaron@redhat.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>


---
 arch/x86/kernel/ftrace.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index b79c553..b2374db 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -502,6 +502,22 @@ struct syscall_metadata *syscall_nr_to_meta(int nr)
 	return syscalls_metadata[nr];
 }
 
+int syscall_name_to_nr(char *name)
+{
+	int i;
+
+	if (!syscalls_metadata)
+		return -1;
+
+	for (i = 0; i < FTRACE_SYSCALL_MAX; i++) {
+		if (syscalls_metadata[i]) {
+			if (!strcmp(syscalls_metadata[i]->name, name))
+				return i;
+		}
+	}
+	return -1;
+}
+
 void arch_init_ftrace_syscalls(void)
 {
 	int i;
-- 
1.6.0.6

[PATCH 2/7] V2 add syscall tracepoints

[Jason Baron]

Call arch_init_ftrace_syscalls at boot, so we can determine the set of syscalls
for the syscall trace events.


Signed-off-by: Jason Baron <jbaron@redhat.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>


---
 arch/x86/kernel/ftrace.c      |   15 ++++-----------
 include/trace/syscall.h       |    1 -
 kernel/trace/trace_syscalls.c |    1 -
 3 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index b2374db..f877ae4 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -518,31 +518,24 @@ int syscall_name_to_nr(char *name)
 	return -1;
 }
 
-void arch_init_ftrace_syscalls(void)
+static int __init arch_init_ftrace_syscalls(void)
 {
 	int i;
 	struct syscall_metadata *meta;
 	unsigned long **psys_syscall_table = &sys_call_table;
-	static atomic_t refs;
-
-	if (atomic_inc_return(&refs) != 1)
-		goto end;
 
 	syscalls_metadata = kzalloc(sizeof(*syscalls_metadata) *
 					FTRACE_SYSCALL_MAX, GFP_KERNEL);
 	if (!syscalls_metadata) {
 		WARN_ON(1);
-		return;
+		return -ENOMEM;
 	}
 
 	for (i = 0; i < FTRACE_SYSCALL_MAX; i++) {
 		meta = find_syscall_meta(psys_syscall_table[i]);
 		syscalls_metadata[i] = meta;
 	}
-	return;
-
-	/* Paranoid: avoid overflow */
-end:
-	atomic_dec(&refs);
+	return 0;
 }
+arch_initcall(arch_init_ftrace_syscalls);
 #endif
diff --git a/include/trace/syscall.h b/include/trace/syscall.h
index 8cfe515..c55fcce 100644
--- a/include/trace/syscall.h
+++ b/include/trace/syscall.h
@@ -19,7 +19,6 @@ struct syscall_metadata {
 };
 
 #ifdef CONFIG_FTRACE_SYSCALLS
-extern void arch_init_ftrace_syscalls(void);
 extern struct syscall_metadata *syscall_nr_to_meta(int nr);
 extern void start_ftrace_syscalls(void);
 extern void stop_ftrace_syscalls(void);
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 5e57964..08aed43 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -106,7 +106,6 @@ void start_ftrace_syscalls(void)
 	if (++refcount != 1)
 		goto unlock;
 
-	arch_init_ftrace_syscalls();
 	read_lock_irqsave(&tasklist_lock, flags);
 
 	do_each_thread(g, t) {
-- 
1.6.0.6

[PATCH 3/7] V2 add syscall tracepoints

[Jason Baron]

Introduce a new 'DECLARE_TRACE_WITH_CALLBACK()' macro, so that tracepoints can 
associate an external register/unregister function.


Signed-off-by: Jason Baron <jbaron@redhat.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

---
 include/linux/tracepoint.h |   29 +++++++++++++++++++++++++----
 1 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index b9dc4ca..778e018 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -60,8 +60,10 @@ struct tracepoint {
  * Make sure the alignment of the structure in the __tracepoints section will
  * not add unwanted padding between the beginning of the section and the
  * structure. Force alignment to the same alignment as the section start.
+ * An optional set of (un)registration functions can be passed to perform any
+ * additional (un)registration work.
  */
-#define DECLARE_TRACE(name, proto, args)				\
+#define DECLARE_TRACE_WITH_CALLBACK(name, proto, args, reg, unreg)	\
 	extern struct tracepoint __tracepoint_##name;			\
 	static inline void trace_##name(proto)				\
 	{								\
@@ -71,13 +73,29 @@ struct tracepoint {
 	}								\
 	static inline int register_trace_##name(void (*probe)(proto))	\
 	{								\
-		return tracepoint_probe_register(#name, (void *)probe);	\
+		int ret;						\
+		void (*func)(void) = (void (*)(void))reg;		\
+									\
+		ret = tracepoint_probe_register(#name, (void *)probe);	\
+		if (func && !ret)					\
+			func();						\
+		return ret;						\
 	}								\
 	static inline int unregister_trace_##name(void (*probe)(proto))	\
 	{								\
-		return tracepoint_probe_unregister(#name, (void *)probe);\
+		int ret;						\
+		void (*func)(void) = (void (*)(void))unreg;		\
+									\
+		ret = tracepoint_probe_unregister(#name, (void *)probe);\
+		if (func && !ret)					\
+			func();						\
+		return ret;						\
 	}
 
+
+#define DECLARE_TRACE(name, proto, args)		\
+	DECLARE_TRACE_WITH_CALLBACK(name, TP_PROTO(proto), TP_ARGS(args), 0, 0);
+
 #define DEFINE_TRACE(name)						\
 	static const char __tpstrtab_##name[]				\
 	__attribute__((section("__tracepoints_strings"))) = #name;	\
@@ -94,7 +112,7 @@ extern void tracepoint_update_probe_range(struct tracepoint *begin,
 	struct tracepoint *end);
 
 #else /* !CONFIG_TRACEPOINTS */
-#define DECLARE_TRACE(name, proto, args)				\
+#define DECLARE_TRACE_WITH_CALLBACK(name, proto, args, reg, unreg)	\
 	static inline void _do_trace_##name(struct tracepoint *tp, proto) \
 	{ }								\
 	static inline void trace_##name(proto)				\
@@ -108,6 +126,9 @@ extern void tracepoint_update_probe_range(struct tracepoint *begin,
 		return -ENOSYS;						\
 	}
 
+#define DECLARE_TRACE(name, proto, args)                \
+	DECLARE_TRACE_WITH_CALLBACK(name, TP_PROTO(proto), TP_ARGS(args), 0, 0);
+
 #define DEFINE_TRACE(name)
 #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
 #define EXPORT_TRACEPOINT_SYMBOL(name)
-- 
1.6.0.6

Re: [PATCH 3/7] V2 add syscall tracepoints

[Li Zefan]

> -#define DECLARE_TRACE(name, proto, args)				\
> +#define DECLARE_TRACE_WITH_CALLBACK(name, proto, args, reg, unreg)	\
>  	extern struct tracepoint __tracepoint_##name;			\
>  	static inline void trace_##name(proto)				\
>  	{								\
> @@ -71,13 +73,29 @@ struct tracepoint {
>  	}								\
>  	static inline int register_trace_##name(void (*probe)(proto))	\
>  	{								\
> -		return tracepoint_probe_register(#name, (void *)probe);	\
> +		int ret;						\
> +		void (*func)(void) = (void (*)(void))reg;		\

The explicit cast seems unnecessary.

> +									\
> +		ret = tracepoint_probe_register(#name, (void *)probe);	\
> +		if (func && !ret)					\
> +			func();						\
> +		return ret;						\
>  	}								\
>  	static inline int unregister_trace_##name(void (*probe)(proto))	\
>  	{								\
> -		return tracepoint_probe_unregister(#name, (void *)probe);\
> +		int ret;						\
> +		void (*func)(void) = (void (*)(void))unreg;		\
> +									\
> +		ret = tracepoint_probe_unregister(#name, (void *)probe);\
> +		if (func && !ret)					\
> +			func();						\
> +		return ret;						\
>  	}
>  
> +
> +#define DECLARE_TRACE(name, proto, args)		\
> +	DECLARE_TRACE_WITH_CALLBACK(name, TP_PROTO(proto), TP_ARGS(args), 0, 0);

I think DECLARE_TRACE_WITH_CALLBACK(..., NULL, NULL) is better.

> +
>  #define DEFINE_TRACE(name)						\
>  	static const char __tpstrtab_##name[]				\
>  	__attribute__((section("__tracepoints_strings"))) = #name;	\
> @@ -94,7 +112,7 @@ extern void tracepoint_update_probe_range(struct tracepoint *begin,
>  	struct tracepoint *end);
>  
>  #else /* !CONFIG_TRACEPOINTS */
> -#define DECLARE_TRACE(name, proto, args)				\
> +#define DECLARE_TRACE_WITH_CALLBACK(name, proto, args, reg, unreg)	\
>  	static inline void _do_trace_##name(struct tracepoint *tp, proto) \
>  	{ }								\
>  	static inline void trace_##name(proto)				\
> @@ -108,6 +126,9 @@ extern void tracepoint_update_probe_range(struct tracepoint *begin,
>  		return -ENOSYS;						\
>  	}
>  
> +#define DECLARE_TRACE(name, proto, args)                \
> +	DECLARE_TRACE_WITH_CALLBACK(name, TP_PROTO(proto), TP_ARGS(args), 0, 0);

ditto

> +
>  #define DEFINE_TRACE(name)
>  #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
>  #define EXPORT_TRACEPOINT_SYMBOL(name)

Re: [PATCH 3/7] V2 add syscall tracepoints

[Frederic Weisbecker]

On Thu, Jun 25, 2009 at 10:20:41AM +0800, Li Zefan wrote:
> > -#define DECLARE_TRACE(name, proto, args)				\
> > +#define DECLARE_TRACE_WITH_CALLBACK(name, proto, args, reg, unreg)	\
> >  	extern struct tracepoint __tracepoint_##name;			\
> >  	static inline void trace_##name(proto)				\
> >  	{								\
> > @@ -71,13 +73,29 @@ struct tracepoint {
> >  	}								\
> >  	static inline int register_trace_##name(void (*probe)(proto))	\
> >  	{								\
> > -		return tracepoint_probe_register(#name, (void *)probe);	\
> > +		int ret;						\
> > +		void (*func)(void) = (void (*)(void))reg;		\
> 
> The explicit cast seems unnecessary.


It seems even dangerous.
That might hide unmatching callback types passed in macro
arguments.

Frederic.


 
> > +									\
> > +		ret = tracepoint_probe_register(#name, (void *)probe);	\
> > +		if (func && !ret)					\
> > +			func();						\
> > +		return ret;						\
> >  	}								\
> >  	static inline int unregister_trace_##name(void (*probe)(proto))	\
> >  	{								\
> > -		return tracepoint_probe_unregister(#name, (void *)probe);\
> > +		int ret;						\
> > +		void (*func)(void) = (void (*)(void))unreg;		\
> > +									\
> > +		ret = tracepoint_probe_unregister(#name, (void *)probe);\
> > +		if (func && !ret)					\
> > +			func();						\
> > +		return ret;						\
> >  	}
> >  
> > +
> > +#define DECLARE_TRACE(name, proto, args)		\
> > +	DECLARE_TRACE_WITH_CALLBACK(name, TP_PROTO(proto), TP_ARGS(args), 0, 0);
> 
> I think DECLARE_TRACE_WITH_CALLBACK(..., NULL, NULL) is better.
> 
> > +
> >  #define DEFINE_TRACE(name)						\
> >  	static const char __tpstrtab_##name[]				\
> >  	__attribute__((section("__tracepoints_strings"))) = #name;	\
> > @@ -94,7 +112,7 @@ extern void tracepoint_update_probe_range(struct tracepoint *begin,
> >  	struct tracepoint *end);
> >  
> >  #else /* !CONFIG_TRACEPOINTS */
> > -#define DECLARE_TRACE(name, proto, args)				\
> > +#define DECLARE_TRACE_WITH_CALLBACK(name, proto, args, reg, unreg)	\
> >  	static inline void _do_trace_##name(struct tracepoint *tp, proto) \
> >  	{ }								\
> >  	static inline void trace_##name(proto)				\
> > @@ -108,6 +126,9 @@ extern void tracepoint_update_probe_range(struct tracepoint *begin,
> >  		return -ENOSYS;						\
> >  	}
> >  
> > +#define DECLARE_TRACE(name, proto, args)                \
> > +	DECLARE_TRACE_WITH_CALLBACK(name, TP_PROTO(proto), TP_ARGS(args), 0, 0);
> 
> ditto
> 
> > +
> >  #define DEFINE_TRACE(name)
> >  #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
> >  #define EXPORT_TRACEPOINT_SYMBOL(name)
> 
> 

[PATCH 4/7] V2 add syscall tracepoints

[Jason Baron]

add two tracepoints in syscall exit and entry path, conditioned on
TIF_SYSCALL_FTRACE. Supports the syscall trace event code.


Signed-off-by: Jason Baron <jbaron@redhat.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

---
 arch/x86/kernel/ptrace.c |    6 ++++--
 include/trace/syscall.h  |   18 ++++++++++++++++++
 kernel/tracepoint.c      |   38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index b457f78..3a6449b 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -37,6 +37,8 @@
 #include <asm/hw_breakpoint.h>
 
 #include <trace/syscall.h>
+DEFINE_TRACE(syscall_enter);
+DEFINE_TRACE(syscall_exit);
 
 #include "tls.h"
 
@@ -1549,7 +1551,7 @@ asmregparm long syscall_trace_enter(struct pt_regs *regs)
 		ret = -1L;
 
 	if (unlikely(test_thread_flag(TIF_SYSCALL_FTRACE)))
-		ftrace_syscall_enter(regs);
+		trace_syscall_enter(regs, regs->orig_ax);
 
 	if (unlikely(current->audit_context)) {
 		if (IS_IA32)
@@ -1575,7 +1577,7 @@ asmregparm void syscall_trace_leave(struct pt_regs *regs)
 		audit_syscall_exit(AUDITSC_RESULT(regs->ax), regs->ax);
 
 	if (unlikely(test_thread_flag(TIF_SYSCALL_FTRACE)))
-		ftrace_syscall_exit(regs);
+		trace_syscall_exit(regs, regs->ax);
 
 	if (test_thread_flag(TIF_SYSCALL_TRACE))
 		tracehook_report_syscall_exit(regs, 0);
diff --git a/include/trace/syscall.h b/include/trace/syscall.h
index c55fcce..4416c6f 100644
--- a/include/trace/syscall.h
+++ b/include/trace/syscall.h
@@ -2,6 +2,24 @@
 #define _TRACE_SYSCALL_H
 
 #include <asm/ptrace.h>
+#include <linux/tracepoint.h>
+
+extern void syscall_regfunc(void);
+extern void syscall_unregfunc(void);
+
+DECLARE_TRACE_WITH_CALLBACK(syscall_enter,
+	TP_PROTO(struct pt_regs *regs, long id),
+	TP_ARGS(regs, id),
+	syscall_regfunc,
+	syscall_unregfunc
+);
+
+DECLARE_TRACE_WITH_CALLBACK(syscall_exit,
+	TP_PROTO(struct pt_regs *regs, long ret),
+	TP_ARGS(regs, ret),
+	syscall_regfunc,
+	syscall_unregfunc
+);
 
 /*
  * A syscall entry in the ftrace syscalls array.
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 1ef5d3a..070a42b 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -24,6 +24,7 @@
 #include <linux/tracepoint.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/sched.h>
 
 extern struct tracepoint __start___tracepoints[];
 extern struct tracepoint __stop___tracepoints[];
@@ -577,3 +578,40 @@ static int init_tracepoints(void)
 __initcall(init_tracepoints);
 
 #endif /* CONFIG_MODULES */
+
+static DEFINE_MUTEX(regfunc_mutex);
+static int sys_tracepoint_refcount;
+
+void syscall_regfunc(void)
+{
+	unsigned long flags;
+	struct task_struct *g, *t;
+
+	mutex_lock(&regfunc_mutex);
+	if (!sys_tracepoint_refcount) {
+		read_lock_irqsave(&tasklist_lock, flags);
+		do_each_thread(g, t) {
+			set_tsk_thread_flag(t, TIF_SYSCALL_FTRACE);
+		} while_each_thread(g, t);
+		read_unlock_irqrestore(&tasklist_lock, flags);
+	}
+	sys_tracepoint_refcount++;
+	mutex_unlock(&regfunc_mutex);
+}
+
+void syscall_unregfunc(void)
+{
+	unsigned long flags;
+	struct task_struct *g, *t;
+
+	mutex_lock(&regfunc_mutex);
+	sys_tracepoint_refcount--;
+	if (!sys_tracepoint_refcount) {
+		read_lock_irqsave(&tasklist_lock, flags);
+		do_each_thread(g, t) {
+			clear_tsk_thread_flag(t, TIF_SYSCALL_FTRACE);
+		} while_each_thread(g, t);
+		read_unlock_irqrestore(&tasklist_lock, flags);
+	}
+	mutex_unlock(&regfunc_mutex);
+}
-- 
1.6.0.6

Re: [PATCH 4/7] V2 add syscall tracepoints

[Li Zefan]

> diff --git a/include/trace/syscall.h b/include/trace/syscall.h
> index c55fcce..4416c6f 100644
> --- a/include/trace/syscall.h
> +++ b/include/trace/syscall.h
> @@ -2,6 +2,24 @@
>  #define _TRACE_SYSCALL_H
>  
>  #include <asm/ptrace.h>
> +#include <linux/tracepoint.h>

Normal style:

#include <linux/err.h>
#include <linux/tracepoint.h>

#include <asm/ptrace.h>

> +
> +extern void syscall_regfunc(void);
> +extern void syscall_unregfunc(void);

[PATCH 5/7] V2 add syscall tracepoints

[Jason Baron]

update FTRACE_SYSCALL_MAX to the current number of syscalls


Signed-off-by: Jason Baron <jbaron@redhat.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

---
 arch/x86/include/asm/ftrace.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index bd2c651..d16d195 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -30,9 +30,9 @@
 
 /* FIXME: I don't want to stay hardcoded */
 #ifdef CONFIG_X86_64
-# define FTRACE_SYSCALL_MAX     296
+# define FTRACE_SYSCALL_MAX     298
 #else
-# define FTRACE_SYSCALL_MAX     333
+# define FTRACE_SYSCALL_MAX     336
 #endif
 
 #ifdef CONFIG_FUNCTION_TRACER
-- 
1.6.0.6

[PATCH 6/7] V2 add syscall tracepoints

[Jason Baron]

Allow the return value of raw_init() to bail us out of creating a trace event
file.

Signed-off-by: Jason Baron <jbaron@redhat.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

---
 kernel/trace/trace_events.c |   29 +++++++++++++++++++----------
 1 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index aa08be6..aa341ff 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -904,15 +904,6 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
 	if (strcmp(call->system, TRACE_SYSTEM) != 0)
 		d_events = event_subsystem_dir(call->system, d_events);
 
-	if (call->raw_init) {
-		ret = call->raw_init();
-		if (ret < 0) {
-			pr_warning("Could not initialize trace point"
-				   " events/%s\n", call->name);
-			return ret;
-		}
-	}
-
 	call->dir = debugfs_create_dir(call->name, d_events);
 	if (!call->dir) {
 		pr_warning("Could not create debugfs "
@@ -1011,6 +1002,7 @@ static void trace_module_add_events(struct module *mod)
 	struct ftrace_module_file_ops *file_ops = NULL;
 	struct ftrace_event_call *call, *start, *end;
 	struct dentry *d_events;
+	int ret;
 
 	start = mod->trace_events;
 	end = mod->trace_events + mod->num_trace_events;
@@ -1026,7 +1018,15 @@ static void trace_module_add_events(struct module *mod)
 		/* The linker may leave blanks */
 		if (!call->name)
 			continue;
-
+		if (call->raw_init) {
+			ret = call->raw_init();
+			if (ret < 0) {
+				if (ret != -ENOSYS)
+					pr_warning("Could not initialize trace"
+					"point events/%s\n", call->name);
+				continue;
+			}
+		}
 		/*
 		 * This module has events, create file ops for this module
 		 * if not already done.
@@ -1163,6 +1163,15 @@ static __init int event_trace_init(void)
 		/* The linker may leave blanks */
 		if (!call->name)
 			continue;
+		if (call->raw_init) {
+			ret = call->raw_init();
+			if (ret < 0) {
+				if (ret != -ENOSYS)
+					pr_warning("Could not initialize trace"
+					"point events/%s\n", call->name);
+				continue;
+			}
+		}
 		list_add(&call->list, &ftrace_events);
 		event_create_dir(call, d_events, &ftrace_event_id_fops,
 				 &ftrace_enable_fops, &ftrace_event_filter_fops,
-- 
1.6.0.6

Re: [PATCH 6/7] V2 add syscall tracepoints

[Li Zefan]

Jason Baron wrote:
> Allow the return value of raw_init() to bail us out of creating a trace event
> file.
> 
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> 
> ---
>  kernel/trace/trace_events.c |   29 +++++++++++++++++++----------
>  1 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index aa08be6..aa341ff 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -904,15 +904,6 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
>  	if (strcmp(call->system, TRACE_SYSTEM) != 0)
>  		d_events = event_subsystem_dir(call->system, d_events);
>  
> -	if (call->raw_init) {
> -		ret = call->raw_init();
> -		if (ret < 0) {
> -			pr_warning("Could not initialize trace point"

It was "trace point"

> -				   " events/%s\n", call->name);
> -			return ret;
> -		}
> -	}
> -
>  	call->dir = debugfs_create_dir(call->name, d_events);
>  	if (!call->dir) {
>  		pr_warning("Could not create debugfs "
> @@ -1011,6 +1002,7 @@ static void trace_module_add_events(struct module *mod)
>  	struct ftrace_module_file_ops *file_ops = NULL;
>  	struct ftrace_event_call *call, *start, *end;
>  	struct dentry *d_events;
> +	int ret;
>  
>  	start = mod->trace_events;
>  	end = mod->trace_events + mod->num_trace_events;
> @@ -1026,7 +1018,15 @@ static void trace_module_add_events(struct module *mod)
>  		/* The linker may leave blanks */
>  		if (!call->name)
>  			continue;
> -
> +		if (call->raw_init) {
> +			ret = call->raw_init();
> +			if (ret < 0) {
> +				if (ret != -ENOSYS)
> +					pr_warning("Could not initialize trace"
> +					"point events/%s\n", call->name);

Now it is "tracepoint"

> +				continue;
> +			}
> +		}
>  		/*
>  		 * This module has events, create file ops for this module
>  		 * if not already done.
> @@ -1163,6 +1163,15 @@ static __init int event_trace_init(void)
>  		/* The linker may leave blanks */
>  		if (!call->name)
>  			continue;
> +		if (call->raw_init) {
> +			ret = call->raw_init();
> +			if (ret < 0) {
> +				if (ret != -ENOSYS)
> +					pr_warning("Could not initialize trace"

ditto

> +					"point events/%s\n", call->name);
> +				continue;
> +			}
> +		}
>  		list_add(&call->list, &ftrace_events);
>  		event_create_dir(call, d_events, &ftrace_event_id_fops,
>  				 &ftrace_enable_fops, &ftrace_event_filter_fops,

[PATCH 7/7] V2 add syscall tracepoints

[Jason Baron]

Layer Frederic's syscall tracer on tracepoints. We create trace events via
hooking into the SYCALL_DEFINE macros. This allows us to individually toggle
syscall entry and exit points on/off.


Signed-off-by: Jason Baron <jbaron@redhat.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

---
 include/linux/syscalls.h      |   75 +++++++++++++++++-
 include/trace/syscall.h       |   18 ++--
 kernel/trace/trace_syscalls.c |  171 +++++++++++++++++++---------------------
 3 files changed, 163 insertions(+), 101 deletions(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index fa4242c..766502e 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -64,6 +64,7 @@ struct perf_counter_attr;
 #include <linux/sem.h>
 #include <asm/siginfo.h>
 #include <asm/signal.h>
+#include <linux/unistd.h>
 #include <linux/quota.h>
 #include <linux/key.h>
 #include <trace/syscall.h>
@@ -112,6 +113,73 @@ struct perf_counter_attr;
 #define __SC_STR_TDECL5(t, a, ...)	#t, __SC_STR_TDECL4(__VA_ARGS__)
 #define __SC_STR_TDECL6(t, a, ...)	#t, __SC_STR_TDECL5(__VA_ARGS__)
 
+
+#define SYSCALL_TRACE_ENTER_EVENT(sname)			\
+	static struct ftrace_event_call event_enter_##sname;	\
+	static int init_enter_##sname(void)			\
+	{							\
+		int num;					\
+		num = syscall_name_to_nr("sys"#sname);		\
+		if (num < 0)					\
+			return -ENOSYS;				\
+		register_ftrace_event(&event_syscall_enter);	\
+		INIT_LIST_HEAD(&event_enter_##sname.fields);	\
+		init_preds(&event_enter_##sname);		\
+		return 0;					\
+	}							\
+	static int reg_enter_##sname(void)			\
+	{							\
+		return reg_event_syscall_enter("sys"#sname);	\
+	}							\
+	static void unreg_enter_##sname(void)			\
+	{							\
+		unreg_event_syscall_enter("sys"#sname);		\
+	}							\
+	static struct ftrace_event_call __used			\
+	  __attribute__((__aligned__(4)))			\
+	  __attribute__((section("_ftrace_events")))		\
+	  event_enter_##sname = {				\
+		.name                   = "sys_enter"#sname,	\
+		.system                 = "syscalls",		\
+		.event                  = &event_syscall_enter,	\
+		.raw_init		= init_enter_##sname,	\
+		.regfunc		= reg_enter_##sname,	\
+		.unregfunc		= unreg_enter_##sname,	\
+	}
+
+#define SYSCALL_TRACE_EXIT_EVENT(sname)				\
+	static struct ftrace_event_call event_exit_##sname;	\
+	static int init_exit_##sname(void)			\
+	{							\
+		int num;					\
+		num = syscall_name_to_nr("sys"#sname);		\
+		if (num < 0)					\
+			return -ENOSYS;				\
+		register_ftrace_event(&event_syscall_exit);	\
+		INIT_LIST_HEAD(&event_exit_##sname.fields);	\
+		init_preds(&event_exit_##sname);		\
+		return 0;					\
+	}							\
+	static int reg_exit_##sname(void)			\
+	{							\
+		return reg_event_syscall_exit("sys"#sname);	\
+	}							\
+	static void unreg_exit_##sname(void)			\
+	{							\
+		unreg_event_syscall_exit("sys"#sname);		\
+	}							\
+	static struct ftrace_event_call __used			\
+	  __attribute__((__aligned__(4)))			\
+	  __attribute__((section("_ftrace_events")))		\
+	  event_exit_##sname = {				\
+		.name                   = "sys_exit"#sname,	\
+		.system                 = "syscalls",		\
+		.event                  = &event_syscall_exit,	\
+		.raw_init		= init_exit_##sname,	\
+		.regfunc		= reg_exit_##sname,	\
+		.unregfunc		= unreg_exit_##sname,	\
+	}
+
 #define SYSCALL_METADATA(sname, nb)				\
 	static const struct syscall_metadata __used		\
 	  __attribute__((__aligned__(4)))			\
@@ -121,7 +189,9 @@ struct perf_counter_attr;
 		.nb_args 	= nb,				\
 		.types		= types_##sname,		\
 		.args		= args_##sname,			\
-	}
+	};							\
+	SYSCALL_TRACE_ENTER_EVENT(sname);			\
+	SYSCALL_TRACE_EXIT_EVENT(sname);
 
 #define SYSCALL_DEFINE0(sname)					\
 	static const struct syscall_metadata __used		\
@@ -131,8 +201,9 @@ struct perf_counter_attr;
 		.name 		= "sys_"#sname,			\
 		.nb_args 	= 0,				\
 	};							\
+	SYSCALL_TRACE_ENTER_EVENT(_##sname);			\
+	SYSCALL_TRACE_EXIT_EVENT(_##sname);			\
 	asmlinkage long sys_##sname(void)
-
 #else
 #define SYSCALL_DEFINE0(name)	   asmlinkage long sys_##name(void)
 #endif
diff --git a/include/trace/syscall.h b/include/trace/syscall.h
index 4416c6f..53e7d14 100644
--- a/include/trace/syscall.h
+++ b/include/trace/syscall.h
@@ -3,6 +3,8 @@
 
 #include <asm/ptrace.h>
 #include <linux/tracepoint.h>
+#include <linux/unistd.h>
+#include <linux/ftrace_event.h>
 
 extern void syscall_regfunc(void);
 extern void syscall_unregfunc(void);
@@ -38,15 +40,13 @@ struct syscall_metadata {
 
 #ifdef CONFIG_FTRACE_SYSCALLS
 extern struct syscall_metadata *syscall_nr_to_meta(int nr);
-extern void start_ftrace_syscalls(void);
-extern void stop_ftrace_syscalls(void);
-extern void ftrace_syscall_enter(struct pt_regs *regs);
-extern void ftrace_syscall_exit(struct pt_regs *regs);
-#else
-static inline void start_ftrace_syscalls(void)			{ }
-static inline void stop_ftrace_syscalls(void)			{ }
-static inline void ftrace_syscall_enter(struct pt_regs *regs)	{ }
-static inline void ftrace_syscall_exit(struct pt_regs *regs)	{ }
+extern int syscall_name_to_nr(char *name);
+extern struct trace_event event_syscall_enter;
+extern struct trace_event event_syscall_exit;
+extern int reg_event_syscall_enter(char *name);
+extern void unreg_event_syscall_enter(char *name);
+extern int reg_event_syscall_exit(char *name);
+extern void unreg_event_syscall_exit(char *name);
 #endif
 
 #endif /* _TRACE_SYSCALL_H */
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 08aed43..cc283d6 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -1,15 +1,16 @@
 #include <trace/syscall.h>
 #include <linux/kernel.h>
+#include <linux/ftrace.h>
 #include <asm/syscall.h>
 
 #include "trace_output.h"
 #include "trace.h"
 
-/* Keep a counter of the syscall tracing users */
-static int refcount;
-
-/* Prevent from races on thread flags toggling */
 static DEFINE_MUTEX(syscall_trace_lock);
+static int sys_refcount_enter;
+static int sys_refcount_exit;
+static DECLARE_BITMAP(enabled_enter_syscalls, FTRACE_SYSCALL_MAX + 1);
+static DECLARE_BITMAP(enabled_exit_syscalls, FTRACE_SYSCALL_MAX + 1);
 
 /* Option to display the parameters types */
 enum {
@@ -95,53 +96,7 @@ print_syscall_exit(struct trace_iterator *iter, int flags)
 	return TRACE_TYPE_HANDLED;
 }
 
-void start_ftrace_syscalls(void)
-{
-	unsigned long flags;
-	struct task_struct *g, *t;
-
-	mutex_lock(&syscall_trace_lock);
-
-	/* Don't enable the flag on the tasks twice */
-	if (++refcount != 1)
-		goto unlock;
-
-	read_lock_irqsave(&tasklist_lock, flags);
-
-	do_each_thread(g, t) {
-		set_tsk_thread_flag(t, TIF_SYSCALL_FTRACE);
-	} while_each_thread(g, t);
-
-	read_unlock_irqrestore(&tasklist_lock, flags);
-
-unlock:
-	mutex_unlock(&syscall_trace_lock);
-}
-
-void stop_ftrace_syscalls(void)
-{
-	unsigned long flags;
-	struct task_struct *g, *t;
-
-	mutex_lock(&syscall_trace_lock);
-
-	/* There are perhaps still some users */
-	if (--refcount)
-		goto unlock;
-
-	read_lock_irqsave(&tasklist_lock, flags);
-
-	do_each_thread(g, t) {
-		clear_tsk_thread_flag(t, TIF_SYSCALL_FTRACE);
-	} while_each_thread(g, t);
-
-	read_unlock_irqrestore(&tasklist_lock, flags);
-
-unlock:
-	mutex_unlock(&syscall_trace_lock);
-}
-
-void ftrace_syscall_enter(struct pt_regs *regs)
+void ftrace_syscall_enter(struct pt_regs *regs, long id)
 {
 	struct syscall_trace_enter *entry;
 	struct syscall_metadata *sys_data;
@@ -150,6 +105,8 @@ void ftrace_syscall_enter(struct pt_regs *regs)
 	int syscall_nr;
 
 	syscall_nr = syscall_get_nr(current, regs);
+	if (!test_bit(syscall_nr, enabled_enter_syscalls))
+		return;
 
 	sys_data = syscall_nr_to_meta(syscall_nr);
 	if (!sys_data)
@@ -170,7 +127,7 @@ void ftrace_syscall_enter(struct pt_regs *regs)
 	trace_wake_up();
 }
 
-void ftrace_syscall_exit(struct pt_regs *regs)
+void ftrace_syscall_exit(struct pt_regs *regs, long ret)
 {
 	struct syscall_trace_exit *entry;
 	struct syscall_metadata *sys_data;
@@ -178,6 +135,8 @@ void ftrace_syscall_exit(struct pt_regs *regs)
 	int syscall_nr;
 
 	syscall_nr = syscall_get_nr(current, regs);
+	if (!test_bit(syscall_nr, enabled_exit_syscalls))
+		return;
 
 	sys_data = syscall_nr_to_meta(syscall_nr);
 	if (!sys_data)
@@ -196,54 +155,86 @@ void ftrace_syscall_exit(struct pt_regs *regs)
 	trace_wake_up();
 }
 
-static int init_syscall_tracer(struct trace_array *tr)
+int reg_event_syscall_enter(char *name)
 {
-	start_ftrace_syscalls();
+	int ret = 0;
+	int num;
 
-	return 0;
+	num = syscall_name_to_nr(name);
+	if (num < 0 || num > FTRACE_SYSCALL_MAX)
+		return -ENOSYS;
+	mutex_lock(&syscall_trace_lock);
+	if (!sys_refcount_enter)
+		ret = register_trace_syscall_enter(ftrace_syscall_enter);
+	if (ret) {
+		pr_info("event trace: Could not activate"
+				"syscall entry trace point");
+	} else {
+		set_bit(num, enabled_enter_syscalls);
+		sys_refcount_enter++;
+	}
+	mutex_unlock(&syscall_trace_lock);
+	return ret;
 }
 
-static void reset_syscall_tracer(struct trace_array *tr)
+void unreg_event_syscall_enter(char *name)
 {
-	stop_ftrace_syscalls();
-	tracing_reset_online_cpus(tr);
-}
-
-static struct trace_event syscall_enter_event = {
-	.type	 	= TRACE_SYSCALL_ENTER,
-	.trace		= print_syscall_enter,
-};
-
-static struct trace_event syscall_exit_event = {
-	.type	 	= TRACE_SYSCALL_EXIT,
-	.trace		= print_syscall_exit,
-};
+	int num;
 
-static struct tracer syscall_tracer __read_mostly = {
-	.name	     	= "syscall",
-	.init		= init_syscall_tracer,
-	.reset		= reset_syscall_tracer,
-	.flags		= &syscalls_flags,
-};
+	num = syscall_name_to_nr(name);
+	if (num < 0 || num > FTRACE_SYSCALL_MAX)
+		return;
+	mutex_lock(&syscall_trace_lock);
+	sys_refcount_enter--;
+	clear_bit(num, enabled_enter_syscalls);
+	if (!sys_refcount_enter)
+		unregister_trace_syscall_enter(ftrace_syscall_enter);
+	mutex_unlock(&syscall_trace_lock);
+}
 
-__init int register_ftrace_syscalls(void)
+int reg_event_syscall_exit(char *name)
 {
-	int ret;
+	int ret = 0;
+	int num;
 
-	ret = register_ftrace_event(&syscall_enter_event);
-	if (!ret) {
-		printk(KERN_WARNING "event %d failed to register\n",
-		       syscall_enter_event.type);
-		WARN_ON_ONCE(1);
+	num = syscall_name_to_nr(name);
+	if (num < 0 || num > FTRACE_SYSCALL_MAX)
+		return -ENOSYS;
+	mutex_lock(&syscall_trace_lock);
+	if (!sys_refcount_exit)
+		ret = register_trace_syscall_exit(ftrace_syscall_exit);
+	if (ret) {
+		pr_info("event trace: Could not activate"
+				"syscall exit trace point");
+	} else {
+		set_bit(num, enabled_exit_syscalls);
+		sys_refcount_exit++;
 	}
+	mutex_unlock(&syscall_trace_lock);
+	return ret;
+}
 
-	ret = register_ftrace_event(&syscall_exit_event);
-	if (!ret) {
-		printk(KERN_WARNING "event %d failed to register\n",
-		       syscall_exit_event.type);
-		WARN_ON_ONCE(1);
-	}
+void unreg_event_syscall_exit(char *name)
+{
+	int num;
 
-	return register_tracer(&syscall_tracer);
+	num = syscall_name_to_nr(name);
+	if (num < 0 || num > FTRACE_SYSCALL_MAX)
+		return;
+	mutex_lock(&syscall_trace_lock);
+	sys_refcount_exit--;
+	clear_bit(num, enabled_exit_syscalls);
+	if (!sys_refcount_exit)
+		unregister_trace_syscall_exit(ftrace_syscall_exit);
+	mutex_unlock(&syscall_trace_lock);
 }
-device_initcall(register_ftrace_syscalls);
+
+struct trace_event event_syscall_enter = {
+	.trace			= print_syscall_enter,
+	.type			= TRACE_SYSCALL_ENTER
+};
+
+struct trace_event event_syscall_exit = {
+	.trace			= print_syscall_exit,
+	.type			= TRACE_SYSCALL_EXIT
+};
-- 
1.6.0.6

Re: [PATCH 7/7] V2 add syscall tracepoints

[Li Zefan]

>  #endif /* _TRACE_SYSCALL_H */
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 08aed43..cc283d6 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -1,15 +1,16 @@
>  #include <trace/syscall.h>
>  #include <linux/kernel.h>
> +#include <linux/ftrace.h>
>  #include <asm/syscall.h>
>  
>  #include "trace_output.h"
>  #include "trace.h"
>  
> -/* Keep a counter of the syscall tracing users */
> -static int refcount;
> -
> -/* Prevent from races on thread flags toggling */
>  static DEFINE_MUTEX(syscall_trace_lock);
> +static int sys_refcount_enter;
> +static int sys_refcount_exit;
> +static DECLARE_BITMAP(enabled_enter_syscalls, FTRACE_SYSCALL_MAX + 1);
> +static DECLARE_BITMAP(enabled_exit_syscalls, FTRACE_SYSCALL_MAX + 1);

Why "+ 1" ?

> -static int init_syscall_tracer(struct trace_array *tr)
> +int reg_event_syscall_enter(char *name)
>  {
> -	start_ftrace_syscalls();
> +	int ret = 0;
> +	int num;
>  
> -	return 0;
> +	num = syscall_name_to_nr(name);
> +	if (num < 0 || num > FTRACE_SYSCALL_MAX)
> +		return -ENOSYS;

syscall_name_to_nr() returns [0, FTRACE_SYSCALL_MAX) or -1,
so you don't need to check num > MAX, instead it should be
WAR_ON(num >= MAX).

> +	mutex_lock(&syscall_trace_lock);
> +	if (!sys_refcount_enter)
> +		ret = register_trace_syscall_enter(ftrace_syscall_enter);
> +	if (ret) {
> +		pr_info("event trace: Could not activate"
> +				"syscall entry trace point");

Should be:

+		pr_info("event trace: Could not activate "
+				"syscall entry trace point");

> +	} else {
> +		set_bit(num, enabled_enter_syscalls);
> +		sys_refcount_enter++;
> +	}
> +	mutex_unlock(&syscall_trace_lock);
> +	return ret;
>  }

> +int reg_event_syscall_exit(char *name)
>  {
> -	int ret;
> +	int ret = 0;
> +	int num;
>  
> -	ret = register_ftrace_event(&syscall_enter_event);
> -	if (!ret) {
> -		printk(KERN_WARNING "event %d failed to register\n",
> -		       syscall_enter_event.type);
> -		WARN_ON_ONCE(1);
> +	num = syscall_name_to_nr(name);
> +	if (num < 0 || num > FTRACE_SYSCALL_MAX)
> +		return -ENOSYS;

ditto

> +	mutex_lock(&syscall_trace_lock);
> +	if (!sys_refcount_exit)
> +		ret = register_trace_syscall_exit(ftrace_syscall_exit);
> +	if (ret) {
> +		pr_info("event trace: Could not activate"
> +				"syscall exit trace point");

ditto

> +	} else {
> +		set_bit(num, enabled_exit_syscalls);
> +		sys_refcount_exit++;
>  	}
> +	mutex_unlock(&syscall_trace_lock);
> +	return ret;
> +}
>  
> -	ret = register_ftrace_event(&syscall_exit_event);
> -	if (!ret) {
> -		printk(KERN_WARNING "event %d failed to register\n",
> -		       syscall_exit_event.type);
> -		WARN_ON_ONCE(1);
> -	}
> +void unreg_event_syscall_exit(char *name)
> +{
> +	int num;
>  
> -	return register_tracer(&syscall_tracer);
> +	num = syscall_name_to_nr(name);
> +	if (num < 0 || num > FTRACE_SYSCALL_MAX)
> +		return;

ditto

> +	mutex_lock(&syscall_trace_lock);
> +	sys_refcount_exit--;
> +	clear_bit(num, enabled_exit_syscalls);
> +	if (!sys_refcount_exit)
> +		unregister_trace_syscall_exit(ftrace_syscall_exit);
> +	mutex_unlock(&syscall_trace_lock);
>  }

Re: [PATCH 0/7] V2 add syscall tracepoints

[Li Zefan]

Jason Baron wrote:
> hi,
> 
> The following is an implementation of Frederic's syscall tracer on top of
> tracepoints. It adds the ability to toggle the entry/exit of each syscall
> via the standard events/syscalls/syscall_blah/enable interface. The
> implementation is done by adding 2 tracepoints. One on entry and one for exit.
> 
> I've tried to address all of the comments from the last posting...updates include:
> -2/7: make arch_init_ftrace_syscalls static, removed locking, make patch bi-sectable
> -3/7: rename DECLARE_TRACE_REG as DECLARE_TRACE_WITH_CALLBACK
> -4/7: make mutex and counter static
> -7/7: add event tracing for syscalls that take no args
> 

Each patch should have a different title.

Re: [PATCH 0/7] V2 add syscall tracepoints

[Li Zefan]

Jason Baron wrote:
> hi,
> 
> The following is an implementation of Frederic's syscall tracer on top of
> tracepoints. It adds the ability to toggle the entry/exit of each syscall
> via the standard events/syscalls/syscall_blah/enable interface. The
> implementation is done by adding 2 tracepoints. One on entry and one for exit.
> 
> I've tried to address all of the comments from the last posting...updates include:
> -2/7: make arch_init_ftrace_syscalls static, removed locking, make patch bi-sectable
> -3/7: rename DECLARE_TRACE_REG as DECLARE_TRACE_WITH_CALLBACK
> -4/7: make mutex and counter static
> -7/7: add event tracing for syscalls that take no args
> 

The whole patchset:

Reviewed-by: Li Zefan <lizf@cn.fujitsu.com>