[PATCH 1/6] ftrace syscalls: don't add events for unmapped syscalls

[PATCH 1/6] ftrace syscalls: don't add events for unmapped syscalls

[Ian Munsie]

From: Ian Munsie <imunsie@au1.ibm.com>

FTRACE_SYSCALLS would create events for each and every system call, even
if it had failed to map the system call's name with it's number. This
resulted in a number of events being created that would not behave as
expected.

This could happen, for example, on architectures who's symbol names are
unusual and will not match the system call name. It could also happen
with system calls which were mapped to sys_ni_syscall.

This patch changes the default system call number in the metadata to -1.
If the system call name from the metadata is not successfully mapped to
a system call number during boot, than the event initialisation routine
will now return an error, preventing the event from being created.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 include/linux/syscalls.h      |    2 ++
 kernel/trace/trace_syscalls.c |    8 ++++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 18cd068..2e5a68d 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -160,6 +160,7 @@ extern struct trace_event_functions exit_syscall_print_funcs;
 	  __attribute__((section("__syscalls_metadata")))	\
 	  __syscall_meta_##sname = {				\
 		.name 		= "sys"#sname,			\
+		.syscall_nr	= -1,	/* Filled in at boot */	\
 		.nb_args 	= nb,				\
 		.types		= types_##sname,		\
 		.args		= args_##sname,			\
@@ -176,6 +177,7 @@ extern struct trace_event_functions exit_syscall_print_funcs;
 	  __attribute__((section("__syscalls_metadata")))	\
 	  __syscall_meta__##sname = {				\
 		.name 		= "sys_"#sname,			\
+		.syscall_nr	= -1,	/* Filled in at boot */	\
 		.nb_args 	= 0,				\
 		.enter_event	= &event_enter__##sname,	\
 		.exit_event	= &event_exit__##sname,		\
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index bac752f..14f4c02 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -432,6 +432,14 @@ void unreg_event_syscall_exit(struct ftrace_event_call *call)
 int init_syscall_trace(struct ftrace_event_call *call)
 {
 	int id;
+	int num;
+
+	num = ((struct syscall_metadata *)call->data)->syscall_nr;
+	if (num < 0 || num >= NR_syscalls) {
+		pr_debug("syscall %s metadata not mapped, disabling ftrace event\n",
+				((struct syscall_metadata *)call->data)->name);
+		return -ENOSYS;
+	}
 
 	if (set_syscall_print_fmt(call) < 0)
 		return -ENOMEM;
-- 
1.7.2.3

[PATCH 2/6] trace syscalls: Remove redundant syscall_nr checks

[Ian Munsie]

From: Ian Munsie <imunsie@au1.ibm.com>

With the ftrace events now checking if the syscall_nr is valid upon
initialisation, there is no need to verify it when registering and
unregistering the events, so remove the check.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 kernel/trace/trace_syscalls.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 14f4c02..5e34b49 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -367,8 +367,6 @@ int reg_event_syscall_enter(struct ftrace_event_call *call)
 	int num;
 
 	num = ((struct syscall_metadata *)call->data)->syscall_nr;
-	if (num < 0 || num >= NR_syscalls)
-		return -ENOSYS;
 	mutex_lock(&syscall_trace_lock);
 	if (!sys_refcount_enter)
 		ret = register_trace_sys_enter(ftrace_syscall_enter, NULL);
@@ -385,8 +383,6 @@ void unreg_event_syscall_enter(struct ftrace_event_call *call)
 	int num;
 
 	num = ((struct syscall_metadata *)call->data)->syscall_nr;
-	if (num < 0 || num >= NR_syscalls)
-		return;
 	mutex_lock(&syscall_trace_lock);
 	sys_refcount_enter--;
 	clear_bit(num, enabled_enter_syscalls);
@@ -401,8 +397,7 @@ int reg_event_syscall_exit(struct ftrace_event_call *call)
 	int num;
 
 	num = ((struct syscall_metadata *)call->data)->syscall_nr;
-	if (num < 0 || num >= NR_syscalls)
-		return -ENOSYS;
+
 	mutex_lock(&syscall_trace_lock);
 	if (!sys_refcount_exit)
 		ret = register_trace_sys_exit(ftrace_syscall_exit, NULL);
@@ -419,8 +414,6 @@ void unreg_event_syscall_exit(struct ftrace_event_call *call)
 	int num;
 
 	num = ((struct syscall_metadata *)call->data)->syscall_nr;
-	if (num < 0 || num >= NR_syscalls)
-		return;
 	mutex_lock(&syscall_trace_lock);
 	sys_refcount_exit--;
 	clear_bit(num, enabled_exit_syscalls);
-- 
1.7.2.3

[PATCH 3/6] ftrace syscalls: Make arch_syscall_addr weak

[Ian Munsie]

From: Ian Munsie <imunsie@au1.ibm.com>

Some architectures use non-trivial system call tables and will not work
with the generic arch_syscall_addr code. For example, PowerPC64 uses a
table of twin long longs.

This patch makes the generic arch_syscall_addr weak to allow
architectures with non-trivial system call tables to override it.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 Documentation/trace/ftrace-design.txt |    3 +++
 kernel/trace/trace_syscalls.c         |    2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
index dc52bd4..6fca17b 100644
--- a/Documentation/trace/ftrace-design.txt
+++ b/Documentation/trace/ftrace-design.txt
@@ -247,6 +247,9 @@ You need very few things to get the syscalls tracing in an arch.
 - Support the TIF_SYSCALL_TRACEPOINT thread flags.
 - Put the trace_sys_enter() and trace_sys_exit() tracepoints calls from ptrace
   in the ptrace syscalls tracing path.
+- If the system call table on this arch is more complicated than a simple array
+  of addresses of the system calls, implement an arch_syscall_addr to return
+  the address of a given system call.
 - Tag this arch as HAVE_SYSCALL_TRACEPOINTS.
 
 
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 5e34b49..b31ea2d 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -447,7 +447,7 @@ int init_syscall_trace(struct ftrace_event_call *call)
 	return id;
 }
 
-unsigned long __init arch_syscall_addr(int nr)
+unsigned long __init __weak arch_syscall_addr(int nr)
 {
 	return (unsigned long)sys_call_table[nr];
 }
-- 
1.7.2.3

[PATCH 4/6] ftrace syscalls: Allow arch specific syscall symbol matching

[Ian Munsie]

From: Ian Munsie <imunsie@au1.ibm.com>

Some architectures have unusual symbol names and the generic code to
match the symbol name with the function name for the syscall metadata
will fail. For example, symbols on PPC64 start with a period and the
generic code will fail to match them.

This patch splits out the match logic into a standalone weak function
that can be overridden on archs with unusual symbol names.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 Documentation/trace/ftrace-design.txt |    3 +++
 include/linux/ftrace.h                |    1 +
 kernel/trace/trace_syscalls.c         |   19 ++++++++++++-------
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
index 6fca17b..d996d0a 100644
--- a/Documentation/trace/ftrace-design.txt
+++ b/Documentation/trace/ftrace-design.txt
@@ -250,6 +250,9 @@ You need very few things to get the syscalls tracing in an arch.
 - If the system call table on this arch is more complicated than a simple array
   of addresses of the system calls, implement an arch_syscall_addr to return
   the address of a given system call.
+- If the symbol names of the system calls do not match the function names on
+  this arch, implement an arch_syscall_match_sym_name with the appropriate
+  logic to return true if the function name corresponds with the symbol name.
 - Tag this arch as HAVE_SYSCALL_TRACEPOINTS.
 
 
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index dcd6a7c..8f6290a 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -527,6 +527,7 @@ extern enum ftrace_dump_mode ftrace_dump_on_oops;
 #ifdef CONFIG_FTRACE_SYSCALLS
 
 unsigned long arch_syscall_addr(int nr);
+bool arch_syscall_match_sym_name(const char *sym, const char *name);
 
 #endif /* CONFIG_FTRACE_SYSCALLS */
 
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index b31ea2d..85983fd 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -81,13 +81,7 @@ static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
 	kallsyms_lookup(syscall, NULL, NULL, NULL, str);
 
 	for ( ; start < stop; start++) {
-		/*
-		 * Only compare after the "sys" prefix. Archs that use
-		 * syscall wrappers may have syscalls symbols aliases prefixed
-		 * with "SyS" instead of "sys", leading to an unwanted
-		 * mismatch.
-		 */
-		if (start->name && !strcmp(start->name + 3, str + 3))
+		if (start->name && arch_syscall_match_sym_name(str, start->name))
 			return start;
 	}
 	return NULL;
@@ -452,6 +446,17 @@ unsigned long __init __weak arch_syscall_addr(int nr)
 	return (unsigned long)sys_call_table[nr];
 }
 
+bool __weak arch_syscall_match_sym_name(const char *sym, const char *name)
+{
+	/*
+	 * Only compare after the "sys" prefix. Archs that use
+	 * syscall wrappers may have syscalls symbols aliases prefixed
+	 * with "SyS" instead of "sys", leading to an unwanted
+	 * mismatch.
+	 */
+	return (!strcmp(sym + 3, name + 3));
+}
+
 int __init init_ftrace_syscalls(void)
 {
 	struct syscall_metadata *meta;
-- 
1.7.2.3

[PATCH 5/6] trace, powerpc: Implement raw syscall tracepoints on PowerPC

[Ian Munsie]

From: Ian Munsie <imunsie@au.ibm.com>

This patch implements the raw syscall tracepoints on PowerPC and exports
them for ftrace syscalls to use.

To minimise reworking existing code, I slightly re-ordered the thread
info flags such that the new TIF_SYSCALL_TRACEPOINT bit would still fit
within the 16 bits of the andi. instruction's UI field. The instructions
in question are in /arch/powerpc/kernel/entry_{32,64}.S to and the
_TIF_SYSCALL_T_OR_A with the thread flags to see if system call tracing
is enabled.

In the case of 64bit PowerPC, arch_syscall_addr and
arch_syscall_match_sym_name are overridden to allow ftrace syscalls to
work given the unusual system call table structure and symbol names that
start with a period.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 arch/powerpc/Kconfig                   |    1 +
 arch/powerpc/include/asm/syscall.h     |    5 +++++
 arch/powerpc/include/asm/thread_info.h |    7 +++++--
 arch/powerpc/kernel/Makefile           |    1 +
 arch/powerpc/kernel/ftrace.c           |   19 +++++++++++++++++++
 arch/powerpc/kernel/ptrace.c           |   10 ++++++++++
 6 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index e625e9e..6614b69 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -140,6 +140,7 @@ config PPC
 	select HAVE_PERF_EVENTS
 	select HAVE_REGS_AND_STACK_ACCESS_API
 	select HAVE_HW_BREAKPOINT if PERF_EVENTS && PPC_BOOK3S_64
+	select HAVE_SYSCALL_TRACEPOINTS
 
 config EARLY_PRINTK
 	bool
diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index 23913e9..b54b2ad 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -15,6 +15,11 @@
 
 #include <linux/sched.h>
 
+/* ftrace syscalls requires exporting the sys_call_table */
+#ifdef CONFIG_FTRACE_SYSCALLS
+extern const unsigned long *sys_call_table;
+#endif /* CONFIG_FTRACE_SYSCALLS */
+
 static inline long syscall_get_nr(struct task_struct *task,
 				  struct pt_regs *regs)
 {
diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 65eb859..4403f09 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -110,7 +110,8 @@ static inline struct thread_info *current_thread_info(void)
 #define TIF_NOERROR		12	/* Force successful syscall return */
 #define TIF_NOTIFY_RESUME	13	/* callback before returning to user */
 #define TIF_FREEZE		14	/* Freezing for suspend */
-#define TIF_RUNLATCH		15	/* Is the runlatch enabled? */
+#define TIF_SYSCALL_TRACEPOINT	15	/* syscall tracepoint instrumentation */
+#define TIF_RUNLATCH		16	/* Is the runlatch enabled? */
 
 /* as above, but as bit values */
 #define _TIF_SYSCALL_TRACE	(1<<TIF_SYSCALL_TRACE)
@@ -127,8 +128,10 @@ static inline struct thread_info *current_thread_info(void)
 #define _TIF_NOERROR		(1<<TIF_NOERROR)
 #define _TIF_NOTIFY_RESUME	(1<<TIF_NOTIFY_RESUME)
 #define _TIF_FREEZE		(1<<TIF_FREEZE)
+#define _TIF_SYSCALL_TRACEPOINT	(1<<TIF_SYSCALL_TRACEPOINT)
 #define _TIF_RUNLATCH		(1<<TIF_RUNLATCH)
-#define _TIF_SYSCALL_T_OR_A	(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SECCOMP)
+#define _TIF_SYSCALL_T_OR_A	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
+				 _TIF_SECCOMP | _TIF_SYSCALL_TRACEPOINT)
 
 #define _TIF_USER_WORK_MASK	(_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
 				 _TIF_NOTIFY_RESUME)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 36c30f3..13d6eb0 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -106,6 +106,7 @@ obj64-$(CONFIG_AUDIT)		+= compat_audit.o
 
 obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o
 obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o
+obj-$(CONFIG_FTRACE_SYSCALLS)	+= ftrace.o
 obj-$(CONFIG_PERF_EVENTS)	+= perf_callchain.o
 
 obj-$(CONFIG_PPC_PERF_CTRS)	+= perf_event.o
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index ce1f3e4..f5fadbb 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -22,6 +22,7 @@
 #include <asm/cacheflush.h>
 #include <asm/code-patching.h>
 #include <asm/ftrace.h>
+#include <asm/syscall.h>
 
 
 #ifdef CONFIG_DYNAMIC_FTRACE
@@ -600,3 +601,21 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
 	}
 }
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
+
+#if defined(CONFIG_FTRACE_SYSCALLS) && defined(CONFIG_PPC64)
+unsigned long __init arch_syscall_addr(int nr)
+{
+	return sys_call_table[nr*2];
+}
+
+inline bool arch_syscall_match_sym_name(const char *sym, const char *name)
+{
+	/*
+	 * Compare the symbol name with the system call name. Skip the .sys or
+	 * .SyS prefix from the symbol name and the sys prefix from the system
+	 * call name and just match the rest. 32bit can use the generic
+	 * function since their symbol names don't start with a period.
+	 */
+	return (!strcmp(sym + 4, name + 3));
+}
+#endif /* CONFIG_FTRACE_SYSCALLS && CONFIG_PPC64 */
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index a9b3296..f70d144 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -29,6 +29,7 @@
 #include <linux/signal.h>
 #include <linux/seccomp.h>
 #include <linux/audit.h>
+#include <trace/syscall.h>
 #ifdef CONFIG_PPC32
 #include <linux/module.h>
 #endif
@@ -40,6 +41,9 @@
 #include <asm/pgtable.h>
 #include <asm/system.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/syscalls.h>
+
 /*
  * The parameter save area on the stack is used to store arguments being passed
  * to callee function and is located at fixed offset from stack pointer.
@@ -1681,6 +1685,9 @@ long do_syscall_trace_enter(struct pt_regs *regs)
 		 */
 		ret = -1L;
 
+	if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
+		trace_sys_enter(regs, regs->gpr[0]);
+
 	if (unlikely(current->audit_context)) {
 #ifdef CONFIG_PPC64
 		if (!is_32bit_task())
@@ -1709,6 +1716,9 @@ void do_syscall_trace_leave(struct pt_regs *regs)
 		audit_syscall_exit((regs->ccr&0x10000000)?AUDITSC_FAILURE:AUDITSC_SUCCESS,
 				   regs->result);
 
+	if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
+		trace_sys_exit(regs, regs->result);
+
 	step = test_thread_flag(TIF_SINGLESTEP);
 	if (step || test_thread_flag(TIF_SYSCALL_TRACE))
 		tracehook_report_syscall_exit(regs, step);
-- 
1.7.2.3

[PATCH 6/6] trace syscalls: Early terminate search for sys_ni_syscall

[Ian Munsie]

From: Ian Munsie <imunsie@au1.ibm.com>

Many system calls are unimplemented and mapped to sys_ni_syscall, but at
boot ftrace would still search through every syscall metadata entry for
a match which wouldn't be there.

This patch adds causes the search to terminate early if the system call
is not mapped.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 kernel/trace/trace_syscalls.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 85983fd..6e17886 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -80,6 +80,9 @@ static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
 	stop = (struct syscall_metadata *)__stop_syscalls_metadata;
 	kallsyms_lookup(syscall, NULL, NULL, NULL, str);
 
+	if (arch_syscall_match_sym_name(str, "sys_ni_syscall"))
+		return NULL;
+
 	for ( ; start < stop; start++) {
 		if (start->name && arch_syscall_match_sym_name(str, start->name))
 			return start;
-- 
1.7.2.3

Re: [PATCH 4/6] ftrace syscalls: Allow arch specific syscall symbol matching

[Mike Frysinger]

On Mon, Dec 6, 2010 at 23:29, Ian Munsie wrote:
> From: Ian Munsie <imunsie@au1.ibm.com>
>
> Some architectures have unusual symbol names and the generic code to
> match the symbol name with the function name for the syscall metadata
> will fail. For example, symbols on PPC64 start with a period and the
> generic code will fail to match them.
>
> This patch splits out the match logic into a standalone weak function
> that can be overridden on archs with unusual symbol names.
>
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -527,6 +527,7 @@ extern enum ftrace_dump_mode ftrace_dump_on_oops;
>  #ifdef CONFIG_FTRACE_SYSCALLS
>
>  unsigned long arch_syscall_addr(int nr);
> +bool arch_syscall_match_sym_name(const char *sym, const char *name);
>
>  #endif /* CONFIG_FTRACE_SYSCALLS */
>
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -81,13 +81,7 @@ static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
>        kallsyms_lookup(syscall, NULL, NULL, NULL, str);
>
>        for ( ; start < stop; start++) {
> -               /*
> -                * Only compare after the "sys" prefix. Archs that use
> -                * syscall wrappers may have syscalls symbols aliases prefixed
> -                * with "SyS" instead of "sys", leading to an unwanted
> -                * mismatch.
> -                */
> -               if (start->name && !strcmp(start->name + 3, str + 3))
> +               if (start->name && arch_syscall_match_sym_name(str, start->name))
>                        return start;
>        }
>        return NULL;
> @@ -452,6 +446,17 @@ unsigned long __init __weak arch_syscall_addr(int nr)
>        return (unsigned long)sys_call_table[nr];
>  }
>
> +bool __weak arch_syscall_match_sym_name(const char *sym, const char *name)
> +{
> +       /*
> +        * Only compare after the "sys" prefix. Archs that use
> +        * syscall wrappers may have syscalls symbols aliases prefixed
> +        * with "SyS" instead of "sys", leading to an unwanted
> +        * mismatch.
> +        */
> +       return (!strcmp(sym + 3, name + 3));
> +}

useless set of parenthesis, and this overhead sucks.  weak +
additional function call just for a strcmp for most people ?  why not
make it into a define in the header:
#ifndef arch_syscall_match_sym_name
#define arch_syscall_match_sym_name(sym, name) !strcmp(sym + 3, name + 3)
#endif
-mike

PowerPC, ftrace: Add PPC raw syscall tracepoints & ftrace fixes (subset from v2)

[Ian Munsie]

Oops, It seems that I missed LKML in the to address of the cover email
for this set. Guess I shouldn't send a patch set just as the fire alarm
goes off.  Resending:


Hi all,

This is a partial version of my 'ftrace syscalls, PowerPC: Various fixes,
Compat Syscall support and PowerPC implementation'.

I've been tied up with other work and haven't had time to complete the
work on the v3 of the full set, but the patches I'm including here are
pretty stable and there has been a request for the raw syscall
tracepoints in PowerPC.

The actual patches included here have not been changed from the v2 other than
rebasing them on tip/master. They add support for *raw* syscall tracepoints on
PowerPC, while fixing ftrace syscalls to ensure that events will only be created
for syscalls that successfully map their metadata to a syscall number,
so that non-working phantom events are not created.

Patch #2 and #6 in this series are not strictly necessary for this, they just
optimise ftrace syscalls a bit.


What's missing from this series that was in the v2 is the conversion of
all the syscalls implemented under /arch/powerpc, Jason Baron's compat
syscall support and the conversion of the remaining native and compat
syscalls to this infrastructure.

Cheers,
-Ian

GIT: [PATCH 1/6] ftrace syscalls: don't add events for unmapped syscalls
GIT: [PATCH 2/6] trace syscalls: Remove redundant syscall_nr checks
GIT: [PATCH 3/6] ftrace syscalls: Make arch_syscall_addr weak
GIT: [PATCH 4/6] ftrace syscalls: Allow arch specific syscall symbol matching
GIT: [PATCH 5/6] trace, powerpc: Implement raw syscall tracepoints on PowerPC
GIT: [PATCH 6/6] trace syscalls: Early terminate search for sys_ni_syscall

Re: [PATCH 4/6] ftrace syscalls: Allow arch specific syscall symbol matching

[Ian Munsie]

Excerpts from Mike Frysinger's message of Tue Dec 07 15:56:49 +1100 2010:
> useless set of parenthesis, and this overhead sucks.  weak +
> additional function call just for a strcmp for most people ?  why not
> make it into a define in the header:
> #ifndef arch_syscall_match_sym_name
> #define arch_syscall_match_sym_name(sym, name) !strcmp(sym + 3, name + 3)
> #endif
> -mike

No problem, will change it.

Cheers,
-Ian

Re: [PATCH 4/6] ftrace syscalls: Allow arch specific syscall symbol matching

[Steven Rostedt]

On Tue, 2010-12-07 at 16:06 +1100, Ian Munsie wrote:
> Excerpts from Mike Frysinger's message of Tue Dec 07 15:56:49 +1100 2010:
> > useless set of parenthesis, and this overhead sucks.  weak +
> > additional function call just for a strcmp for most people ?  why not
> > make it into a define in the header:
> > #ifndef arch_syscall_match_sym_name
> > #define arch_syscall_match_sym_name(sym, name) !strcmp(sym + 3, name + 3)

Make it a static inline function please.

-- Steve

> > #endif
> > -mike
> 
> No problem, will change it.
> 
> Cheers,
> -Ian

Re: [PATCH 2/6] trace syscalls: Remove redundant syscall_nr checks

[Steven Rostedt]

On Tue, 2010-12-07 at 15:29 +1100, Ian Munsie wrote:
> From: Ian Munsie <imunsie@au1.ibm.com>
> 
> With the ftrace events now checking if the syscall_nr is valid upon
> initialisation, there is no need to verify it when registering and
> unregistering the events, so remove the check.

I still like to keep these checks. I don't mind redundant checks that
are in slow paths, as they may catch a bug on a change in the future.

What you could do is change these to:

	if (WARN_ON_ONCE(num < 0 || num >= NR_syscalls))

-- Steve

> 
> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> ---
>  kernel/trace/trace_syscalls.c |    9 +--------
>  1 files changed, 1 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 14f4c02..5e34b49 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -367,8 +367,6 @@ int reg_event_syscall_enter(struct ftrace_event_call *call)
>  	int num;
>  
>  	num = ((struct syscall_metadata *)call->data)->syscall_nr;
> -	if (num < 0 || num >= NR_syscalls)
> -		return -ENOSYS;
>  	mutex_lock(&syscall_trace_lock);
>  	if (!sys_refcount_enter)
>  		ret = register_trace_sys_enter(ftrace_syscall_enter, NULL);
> @@ -385,8 +383,6 @@ void unreg_event_syscall_enter(struct ftrace_event_call *call)
>  	int num;
>  
>  	num = ((struct syscall_metadata *)call->data)->syscall_nr;
> -	if (num < 0 || num >= NR_syscalls)
> -		return;
>  	mutex_lock(&syscall_trace_lock);
>  	sys_refcount_enter--;
>  	clear_bit(num, enabled_enter_syscalls);
> @@ -401,8 +397,7 @@ int reg_event_syscall_exit(struct ftrace_event_call *call)
>  	int num;
>  
>  	num = ((struct syscall_metadata *)call->data)->syscall_nr;
> -	if (num < 0 || num >= NR_syscalls)
> -		return -ENOSYS;
> +
>  	mutex_lock(&syscall_trace_lock);
>  	if (!sys_refcount_exit)
>  		ret = register_trace_sys_exit(ftrace_syscall_exit, NULL);
> @@ -419,8 +414,6 @@ void unreg_event_syscall_exit(struct ftrace_event_call *call)
>  	int num;
>  
>  	num = ((struct syscall_metadata *)call->data)->syscall_nr;
> -	if (num < 0 || num >= NR_syscalls)
> -		return;
>  	mutex_lock(&syscall_trace_lock);
>  	sys_refcount_exit--;
>  	clear_bit(num, enabled_exit_syscalls);

Re: [PATCH 2/6] trace syscalls: Remove redundant syscall_nr checks

[Ian Munsie]

Excerpts from Steven Rostedt's message of Wed Dec 08 01:54:42 +1100 2010:
> On Tue, 2010-12-07 at 15:29 +1100, Ian Munsie wrote:
> > From: Ian Munsie <imunsie@au1.ibm.com>
> > 
> > With the ftrace events now checking if the syscall_nr is valid upon
> > initialisation, there is no need to verify it when registering and
> > unregistering the events, so remove the check.
> 
> I still like to keep these checks. I don't mind redundant checks that
> are in slow paths, as they may catch a bug on a change in the future.
> 
> What you could do is change these to:
> 
>     if (WARN_ON_ONCE(num < 0 || num >= NR_syscalls))

Hi Steve,

Thanks for the feedback. Will update and resubmit.

Cheers,
-Ian