Mailing List Archive: Linux: Kernel

[PATCH] trace_syscalls: add missed field

Index | Next | Previous | View Threaded

laijs at cn

Nov 25, 2009, 11:49 PM

Post #1 of 4 (102 views)
Permalink

[PATCH] trace_syscalls: add missed 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 [at] cn>
---
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);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

fweisbec at gmail

Nov 26, 2009, 2:31 PM

Post #2 of 4 (95 views)
Permalink

Re: [PATCH] trace_syscalls: add missed field [In reply to]

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 [at] cn>

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

laijs at cn

Nov 26, 2009, 8:02 PM

Post #3 of 4 (94 views)
Permalink

Re: [PATCH] trace_syscalls: add missed field [In reply to]

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 [at] cn>
>
>

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

fweisbec at gmail

Nov 26, 2009, 8:15 PM

Post #4 of 4 (90 views)
Permalink

Re: [PATCH] trace_syscalls: add missed field [In reply to]

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 [at] cn>
> >
> >
>
>
> 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 [at] gmail>

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads