Re: [tip:perf/core] trace_syscalls: Remove unused event_syscall_enter and event_syscall_exit

[Frederic Weisbecker <fweisbec@gmail.com>]

On Tue, Dec 01, 2009 at 04:42:35PM +0000, tip-bot for Lai Jiangshan wrote:
> Commit-ID:  bf56a4ea9f1683c5b223fd3a5dbea23f1fa91c34
> Gitweb:     http://git.kernel.org/tip/bf56a4ea9f1683c5b223fd3a5dbea23f1fa91c34
> Author:     Lai Jiangshan <laijs@cn.fujitsu.com>
> AuthorDate: Tue, 1 Dec 2009 16:23:20 +0800
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Tue, 1 Dec 2009 17:33:28 +0100
> 
> trace_syscalls: Remove unused event_syscall_enter and event_syscall_exit
> 
> fix event_enter_##sname->event
> fix event_exit_##sname->event
> 
> remove unused event_syscall_enter and event_syscall_exit
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> Acked-by: Jason Baron <jbaron@redhat.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> LKML-Reference: <4B14D278.4090209@cn.fujitsu.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---



I'm confused.
Each syscall has a enter_syscall_print_##sname,
all of them are identical (they all map to print_syscall_enter())

OTOH we have event_syscall_enter, which is exactly
the same than enter_syscall_print_##sname.

But we are using a strange mix of them: we do that on
init: register_ftrace_event(&enter_syscall_print_##sname)
but we use event_syscall_enter as the "event" field
of the trace_event. It means that we unregister the wrong
event when we remove a module.

Thanks for fixing this!
But I think you fixed it the wrong way.

Why do we need a copy of this event_syscall_enter
for each syscall event?
Can't we actually drop enter_syscall_print_##sname
and always use event_syscall_enter instead?

The same applies to exit syscall events.

Thanks.




>  include/linux/syscalls.h      |    4 ++--
>  include/trace/syscall.h       |    2 --
>  kernel/trace/trace_syscalls.c |    8 --------
>  3 files changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index b50974a..2f7c539 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -178,7 +178,7 @@ static void prof_sysexit_disable_##sname(struct ftrace_event_call *unused)     \
>  	  event_enter_##sname = {					\
>  		.name                   = "sys_enter"#sname,		\
>  		.system                 = "syscalls",			\
> -		.event                  = &event_syscall_enter,		\
> +		.event                  = &enter_syscall_print_##sname,	\
>  		.raw_init		= init_enter_##sname,		\
>  		.show_format		= syscall_enter_format,		\
>  		.define_fields		= syscall_enter_define_fields,	\
> @@ -214,7 +214,7 @@ static void prof_sysexit_disable_##sname(struct ftrace_event_call *unused)     \
>  	  event_exit_##sname = {					\
>  		.name                   = "sys_exit"#sname,		\
>  		.system                 = "syscalls",			\
> -		.event                  = &event_syscall_exit,		\
> +		.event                  = &exit_syscall_print_##sname,	\
>  		.raw_init		= init_exit_##sname,		\
>  		.show_format		= syscall_exit_format,		\
>  		.define_fields		= syscall_exit_define_fields,	\
> diff --git a/include/trace/syscall.h b/include/trace/syscall.h
> index 51ee17d..5f8827c 100644
> --- a/include/trace/syscall.h
> +++ b/include/trace/syscall.h
> @@ -37,8 +37,6 @@ extern unsigned long arch_syscall_addr(int nr);
>  extern int syscall_name_to_nr(char *name);
>  void set_syscall_enter_id(int num, int id);
>  void set_syscall_exit_id(int num, int id);
> -extern struct trace_event event_syscall_enter;
> -extern struct trace_event event_syscall_exit;
>  
>  extern int syscall_enter_format(struct ftrace_event_call *call,
>  				struct trace_seq *s);
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 63aa807..00d6e17 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -444,14 +444,6 @@ void unreg_event_syscall_exit(struct ftrace_event_call *call)
>  	mutex_unlock(&syscall_trace_lock);
>  }
>  
> -struct trace_event event_syscall_enter = {
> -	.trace			= print_syscall_enter,
> -};
> -
> -struct trace_event event_syscall_exit = {
> -	.trace			= print_syscall_exit,
> -};
> -
>  int __init init_ftrace_syscalls(void)
>  {
>  	struct syscall_metadata *meta;