[PATCH] trace_syscalls: add missed field

[PATCH] trace_syscalls: add missed field

[Lai Jiangshan]

Field syscall number is missed in syscall_enter_define_fields()/
syscall_exit_define_fields().

syscall number is also needed for event filter or other users.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 9189cbe..63aa807 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -261,6 +261,10 @@ int syscall_enter_define_fields(struct ftrace_event_call *call)
 	if (ret)
 		return ret;
 
+	ret = trace_define_field(call, SYSCALL_FIELD(int, nr), FILTER_OTHER);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < meta->nb_args; i++) {
 		ret = trace_define_field(call, meta->types[i],
 					 meta->args[i], offset,
@@ -281,6 +285,10 @@ int syscall_exit_define_fields(struct ftrace_event_call *call)
 	if (ret)
 		return ret;
 
+	ret = trace_define_field(call, SYSCALL_FIELD(int, nr), FILTER_OTHER);
+	if (ret)
+		return ret;
+
 	ret = trace_define_field(call, SYSCALL_FIELD(long, ret),
 				 FILTER_OTHER);
 

Re: [PATCH] trace_syscalls: add missed field

[Frederic Weisbecker]

On Thu, Nov 26, 2009 at 03:49:33PM +0800, Lai Jiangshan wrote:
> 
> Field syscall number is missed in syscall_enter_define_fields()/
> syscall_exit_define_fields().
> 
> syscall number is also needed for event filter or other users.
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>



Well, I don't think it's very useful for in-kernel filtering.
Filtering a syscall event by its number would mean filtering all
event for this syscall. This is the same as not tracing it.

Or do you have other usecases in mind?

Thanks.

Re: [PATCH] trace_syscalls: add missed field

[Lai Jiangshan]

Frederic Weisbecker wrote:
> On Thu, Nov 26, 2009 at 03:49:33PM +0800, Lai Jiangshan wrote:
>> Field syscall number is missed in syscall_enter_define_fields()/
>> syscall_exit_define_fields().
>>
>> syscall number is also needed for event filter or other users.
>>
>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> 
> 


For all kinds of tracer, all fields are "defined"
by trace_define_field(), except this one.
Maybe because I don't like inconsistent codes.

> 
> Well, I don't think it's very useful for in-kernel filtering.
> Filtering a syscall event by its number would mean filtering all
> event for this syscall. This is the same as not tracing it.
> 
> Or do you have other usecases in mind?
> 

Current, only filter use struct ftrace_event_call->fields,
so there is no other usecases ^_^.
But my next take of "tracing: use defined fields to print formats"
will use it.

Re: [PATCH] trace_syscalls: add missed field

[Frederic Weisbecker]

On Fri, Nov 27, 2009 at 12:02:43PM +0800, Lai Jiangshan wrote:
> Frederic Weisbecker wrote:
> > On Thu, Nov 26, 2009 at 03:49:33PM +0800, Lai Jiangshan wrote:
> >> Field syscall number is missed in syscall_enter_define_fields()/
> >> syscall_exit_define_fields().
> >>
> >> syscall number is also needed for event filter or other users.
> >>
> >> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> > 
> > 
> 
> 
> For all kinds of tracer, all fields are "defined"
> by trace_define_field(), except this one.
> Maybe because I don't like inconsistent codes.


:)


> > 
> > Well, I don't think it's very useful for in-kernel filtering.
> > Filtering a syscall event by its number would mean filtering all
> > event for this syscall. This is the same as not tracing it.
> > 
> > Or do you have other usecases in mind?
> > 
> 
> Current, only filter use struct ftrace_event_call->fields,
> so there is no other usecases ^_^.
> But my next take of "tracing: use defined fields to print formats"
> will use it.


Oh really you are planning such patch? That looks hard to do
considering the tricky things that happen in fields printing
sometimes. Also printing the fields is often a human
interpretation on how to handle them, so... Anyway, I'll just
wait for your patches and see.

That said I really have no opposition against this patch.
For consistency over what is done with every other trace event
fields, and to anticipate any further uses of trace event fields
definition other than filtering:

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

(Steve, Ingo, should I queue it or...what do you prefer?)

[tip:perf/core] trace_syscalls: Add syscall nr field

[tip-bot for Lai Jiangshan]

Commit-ID:  0f1ef51d244809f417bdf45cdb00109fb6005672
Gitweb:     http://git.kernel.org/tip/0f1ef51d244809f417bdf45cdb00109fb6005672
Author:     Lai Jiangshan <laijs@cn.fujitsu.com>
AuthorDate: Thu, 26 Nov 2009 15:49:33 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 27 Nov 2009 06:24:19 +0100

trace_syscalls: Add syscall nr field

Field syscall number is missed in syscall_enter_define_fields()/
syscall_exit_define_fields().

Syscall number is also needed for event filter or other users.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jason Baron <jbaron@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <4B0E330D.1070206@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/trace/trace_syscalls.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 9189cbe..63aa807 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -261,6 +261,10 @@ int syscall_enter_define_fields(struct ftrace_event_call *call)
 	if (ret)
 		return ret;
 
+	ret = trace_define_field(call, SYSCALL_FIELD(int, nr), FILTER_OTHER);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < meta->nb_args; i++) {
 		ret = trace_define_field(call, meta->types[i],
 					 meta->args[i], offset,
@@ -281,6 +285,10 @@ int syscall_exit_define_fields(struct ftrace_event_call *call)
 	if (ret)
 		return ret;
 
+	ret = trace_define_field(call, SYSCALL_FIELD(int, nr), FILTER_OTHER);
+	if (ret)
+		return ret;
+
 	ret = trace_define_field(call, SYSCALL_FIELD(long, ret),
 				 FILTER_OTHER);