Date: Tue, 8 Mar 2016 14:24:34 -0500
From: Steven Rostedt <rostedt@goodmis.org>
To: Taeung Song <treeze.taeung@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>, linux-kernel@vger.kernel.org,
        Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>
Subject: Re: [RFC][PATCH v4 1/2] tracing/syscalls: Rename a field 'nr' to
 '__syscall_nr' in format
Message-ID: <20160308142434.4cabfc74@gandalf.local.home>
In-Reply-To: <1456591311-10915-1-git-send-email-treeze.taeung@gmail.com>
References: <1456591311-10915-1-git-send-email-treeze.taeung@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4523
Lines: 108

On Sun, 28 Feb 2016 01:41:51 +0900
Taeung Song <treeze.taeung@gmail.com> wrote:


Have we decided what we are going to do with this patch? Pull it as is
and see what the fall out is?

Was there a perf patch with this too?

-- Steve

> Some syscalls tracepoint duplicated fields with the same name 'nr' i.e.
> 
>     # cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_io_getevents/format
>     name: sys_enter_io_getevents
>     ID: 739
>     format:
>             field:unsigned short common_type; offset:0;  size:2; signed:0;
>             field:unsigned char common_flags; offset:2;  size:1; signed:0;
>             field:unsigned char common_preempt_count; offset:3; size:1; signed:0;
>             field:int common_pid;offset:4;size:4;signed:1;
>             field:int nr;                     offset:8;  size:4; signed:1;
>             field:aio_context_t ctx_id;       offset:16; size:8; signed:0;
>             field:long min_nr;                offset:24; size:8; signed:0;
>             field:long nr;                    offset:32; size:8; signed:0;
>             field:struct io_event * events;   offset:40; size:8; signed:0;
>             field:struct timespec * timeout;  offset:48; size:8; signed:0;
> 
>             print fmt: "ctx_id: 0x%08lx, min_nr: 0x%08lx, nr: 0x%08lx,
>                         events: 0x%08lx, timeout: 0x%08lx", ((unsigned long)(REC->ctx_id)),
>                         ((unsigned long)(REC->min_nr)), ((unsigned long)(REC->nr)),
>                         ((unsigned long)(REC->events)), ((unsigned long)(REC->timeout))
> 
> As above 'int nr;' and 'long nr;' fields have
> duplicated name so problems are occurred in perf-script i.e.
> 
>     # perf record -e syscalls:*
>     # perf script -g python
>     # perf script -s perf-script.py
>       File "perf-script.py", line 8694
>         def syscalls__sys_enter_io_getevents(event_name, context, common_cpu,
>     SyntaxError: duplicate argument 'nr' in function definition
>     Error running python script perf-script.py
> 
> As above, problems about duplicated argument occurred.
> Not only sys_enter_io_getevent() but also sys_enter_io_submit()
> have relevevance to this problem.
> 
> So rename a field 'nr' to '__syscall_nr' for system call number
> without renaming a member 'nr' of two structures 'syscall_trace_enter'
> and 'syscall_trace_exit'.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Lai Jiangshan <jiangshanlai@gmail.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  kernel/trace/trace_syscalls.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 0655afb..d166308 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -186,11 +186,11 @@ print_syscall_exit(struct trace_iterator *iter, int flags,
>  
>  extern char *__bad_type_size(void);
>  
> -#define SYSCALL_FIELD(type, name)					\
> -	sizeof(type) != sizeof(trace.name) ?				\
> +#define SYSCALL_FIELD(type, field, name)				\
> +	sizeof(type) != sizeof(trace.field) ?				\
>  		__bad_type_size() :					\
> -		#type, #name, offsetof(typeof(trace), name),		\
> -		sizeof(trace.name), is_signed_type(type)
> +		#type, #name, offsetof(typeof(trace), field),		\
> +		sizeof(trace.field), is_signed_type(type)
>  
>  static int __init
>  __set_enter_print_fmt(struct syscall_metadata *entry, char *buf, int len)
> @@ -261,7 +261,8 @@ static int __init syscall_enter_define_fields(struct trace_event_call *call)
>  	int i;
>  	int offset = offsetof(typeof(trace), args);
>  
> -	ret = trace_define_field(call, SYSCALL_FIELD(int, nr), FILTER_OTHER);
> +	ret = trace_define_field(call, SYSCALL_FIELD(int, nr, __syscall_nr),
> +				 FILTER_OTHER);
>  	if (ret)
>  		return ret;
>  
> @@ -281,11 +282,12 @@ static int __init syscall_exit_define_fields(struct trace_event_call *call)
>  	struct syscall_trace_exit trace;
>  	int ret;
>  
> -	ret = trace_define_field(call, SYSCALL_FIELD(int, nr), FILTER_OTHER);
> +	ret = trace_define_field(call, SYSCALL_FIELD(int, nr, __syscall_nr),
> +				 FILTER_OTHER);
>  	if (ret)
>  		return ret;
>  
> -	ret = trace_define_field(call, SYSCALL_FIELD(long, ret),
> +	ret = trace_define_field(call, SYSCALL_FIELD(long, ret, ret),
>  				 FILTER_OTHER);
>  
>  	return ret;