2009-04-03 10:28:19

by Zhao Lei

[permalink] [raw]
Subject: [PATCH 1/2] ftrace: Correct a text align for event format output

If we cat debugfs/tracing/events/ftrace/bprint/format, we'll see:
name: bprint
ID: 6
format:
field:unsigned char common_type; offset:0; size:1;
field:unsigned char common_flags; offset:1; size:1;
field:unsigned char common_preempt_count; offset:2; size:1;
field:int common_pid; offset:4; size:4;
field:int common_tgid; offset:8; size:4;

field:unsigned long ip; offset:12; size:4;
field:char * fmt; offset:16; size:4;
field: char buf; offset:20; size:0;

print fmt: "%08lx (%d) fmt:%p %s"

There is a redundant blank before char buf, and it need to be deleted.

Signed-off-by: Zhao Lei <[email protected]>
---
kernel/trace/trace_export.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index 4d9952d..07a22c3 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -40,7 +40,7 @@

#undef TRACE_FIELD_ZERO_CHAR
#define TRACE_FIELD_ZERO_CHAR(item) \
- ret = trace_seq_printf(s, "\tfield: char " #item ";\t" \
+ ret = trace_seq_printf(s, "\tfield:char " #item ";\t" \
"offset:%u;\tsize:0;\n", \
(unsigned int)offsetof(typeof(field), item)); \
if (!ret) \
--
1.5.5.3


2009-04-03 10:29:41

by Zhao Lei

[permalink] [raw]
Subject: [PATCH 2/2] ftrace: Code cleanup for kernel/trace/trace_events_stage_*.h

Add TRACE_FORMAT's define for trace_events_stage_2.h.
Although it is already defined in trace_events_stage_1.h, we should make each
function independence.

Move TP_fast_assign's define from trace_events_stage_2.h to
trace_events_stage_3.h because it is used there.

Unify TRACE_EVENT's 5th argument's name to "assign"

Impact: cleanup, no functionality changed

Signed-off-by: Zhao Lei <[email protected]>
---
kernel/trace/trace_events_stage_2.h | 8 ++++----
kernel/trace/trace_events_stage_3.h | 3 +++
2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_events_stage_2.h b/kernel/trace/trace_events_stage_2.h
index 30743f7..9e47c39 100644
--- a/kernel/trace/trace_events_stage_2.h
+++ b/kernel/trace/trace_events_stage_2.h
@@ -32,6 +32,9 @@
* in binary.
*/

+#undef TRACE_FORMAT
+#define TRACE_FORMAT(call, proto, args, fmt)
+
#undef __entry
#define __entry field

@@ -110,11 +113,8 @@ ftrace_raw_output_##call(struct trace_iterator *iter, int flags) \
#undef TP_printk
#define TP_printk(fmt, args...) "%s, %s\n", #fmt, #args

-#undef TP_fast_assign
-#define TP_fast_assign(args...) args
-
#undef TRACE_EVENT
-#define TRACE_EVENT(call, proto, args, tstruct, func, print) \
+#define TRACE_EVENT(call, proto, args, tstruct, assign, print) \
static int \
ftrace_format_##call(struct trace_seq *s) \
{ \
diff --git a/kernel/trace/trace_events_stage_3.h b/kernel/trace/trace_events_stage_3.h
index 9d2fa78..49b305a 100644
--- a/kernel/trace/trace_events_stage_3.h
+++ b/kernel/trace/trace_events_stage_3.h
@@ -193,6 +193,9 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
_TRACE_PROFILE_INIT(call) \
}

+#undef TP_fast_assign
+#define TP_fast_assign(args...) args
+
#undef __entry
#define __entry entry

--
1.5.5.3

2009-04-07 01:50:46

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 1/2] ftrace: Correct a text align for event format output



On Fri, 3 Apr 2009, Zhaolei wrote:

> If we cat debugfs/tracing/events/ftrace/bprint/format, we'll see:
> name: bprint
> ID: 6
> format:
> field:unsigned char common_type; offset:0; size:1;
> field:unsigned char common_flags; offset:1; size:1;
> field:unsigned char common_preempt_count; offset:2; size:1;
> field:int common_pid; offset:4; size:4;
> field:int common_tgid; offset:8; size:4;
>
> field:unsigned long ip; offset:12; size:4;
> field:char * fmt; offset:16; size:4;
> field: char buf; offset:20; size:0;
>
> print fmt: "%08lx (%d) fmt:%p %s"
>
> There is a redundant blank before char buf, and it need to be deleted.
>
> Signed-off-by: Zhao Lei <[email protected]>

Thanks, applied.

-- Steve

2009-04-07 01:54:35

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 2/2] ftrace: Code cleanup for kernel/trace/trace_events_stage_*.h


On Fri, 3 Apr 2009, Zhaolei wrote:

> Add TRACE_FORMAT's define for trace_events_stage_2.h.
> Although it is already defined in trace_events_stage_1.h, we should make each
> function independence.
>
> Move TP_fast_assign's define from trace_events_stage_2.h to
> trace_events_stage_3.h because it is used there.
>
> Unify TRACE_EVENT's 5th argument's name to "assign"
>
> Impact: cleanup, no functionality changed
>
> Signed-off-by: Zhao Lei <[email protected]>

I see what you are doing here, but I'm a little hesitant to apply it.
I'm getting ready to travel, so I do not have the time to look deeper at
this today. I'll try to do it while I'm traveling.

Thanks,

-- Steve

2009-04-07 13:16:27

by Zhao Lei

[permalink] [raw]
Subject: [tip:tracing/urgent] ftrace: Correct a text align for event format output

Commit-ID: 1bbe2a83ab68e5cf8c66c372c7cb3b51910c2cfe
Gitweb: http://git.kernel.org/tip/1bbe2a83ab68e5cf8c66c372c7cb3b51910c2cfe
Author: Zhaolei <[email protected]>
AuthorDate: Fri, 3 Apr 2009 18:24:46 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 7 Apr 2009 14:02:42 +0200

ftrace: Correct a text align for event format output

If we cat debugfs/tracing/events/ftrace/bprint/format, we'll see:
name: bprint
ID: 6
format:
field:unsigned char common_type; offset:0; size:1;
field:unsigned char common_flags; offset:1; size:1;
field:unsigned char common_preempt_count; offset:2; size:1;
field:int common_pid; offset:4; size:4;
field:int common_tgid; offset:8; size:4;

field:unsigned long ip; offset:12; size:4;
field:char * fmt; offset:16; size:4;
field: char buf; offset:20; size:0;

print fmt: "%08lx (%d) fmt:%p %s"

There is an inconsistent blank before char buf.

Signed-off-by: Zhao Lei <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>


---
kernel/trace/trace_export.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index 4d9952d..07a22c3 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -40,7 +40,7 @@

#undef TRACE_FIELD_ZERO_CHAR
#define TRACE_FIELD_ZERO_CHAR(item) \
- ret = trace_seq_printf(s, "\tfield: char " #item ";\t" \
+ ret = trace_seq_printf(s, "\tfield:char " #item ";\t" \
"offset:%u;\tsize:0;\n", \
(unsigned int)offsetof(typeof(field), item)); \
if (!ret) \

2009-04-08 07:04:41

by Jiaying Zhang

[permalink] [raw]
Subject: Re: [PATCH 2/2] ftrace: Code cleanup for kernel/trace/trace_events_stage_*.h

Hi Steve,

I am looking at ftrace code more closely. It is also not clear to me why we
want to define the trace event macros in three stage header files. I wonder
whether it would be clearer if we merge them together. Then people don't
need to look at three files to understand what is going on in event tracing.

Jiaying

On Mon, Apr 6, 2009 at 6:54 PM, Steven Rostedt <[email protected]> wrote:
>
> On Fri, 3 Apr 2009, Zhaolei wrote:
>
> > Add TRACE_FORMAT's define for trace_events_stage_2.h.
> > Although it is already defined in trace_events_stage_1.h, we should make each
> > function independence.
> >
> > Move TP_fast_assign's define from trace_events_stage_2.h to
> > trace_events_stage_3.h because it is used there.
> >
> > Unify TRACE_EVENT's 5th argument's name to "assign"
> >
> > Impact: cleanup, no functionality changed
> >
> > Signed-off-by: Zhao Lei <[email protected]>
>
> I see what you are doing here, but I'm a little hesitant to apply it.
> I'm getting ready to travel, so I do not have the time to look deeper at
> this today. I'll try to do it while I'm traveling.
>
> Thanks,
>
> -- Steve
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/

2009-04-08 07:08:01

by Zhao Lei

[permalink] [raw]
Subject: Re: Re: [PATCH 2/2] ftrace: Code cleanup for kernel/trace/trace_events_stage_*.h

Jiaying Zhang wrote:
> Hi Steve,
>
> I am looking at ftrace code more closely. It is also not clear to me why we
Hello, Zhang

Different people may have different taste on codeing style, but IMHO, separate
those complex definition into several files make me easy to understand.
It is only my personal code liking...

B.R.
Zhaolei

> want to define the trace event macros in three stage header files. I wonder
> whether it would be clearer if we merge them together. Then people don't
> need to look at three files to understand what is going on in event tracing.
>
> Jiaying
>
> On Mon, Apr 6, 2009 at 6:54 PM, Steven Rostedt <[email protected]
> <mailto:[email protected]>> wrote:
>
>
> On Fri, 3 Apr 2009, Zhaolei wrote:
>
> > Add TRACE_FORMAT's define for trace_events_stage_2.h.
> > Although it is already defined in trace_events_stage_1.h, we
> should make each
> > function independence.
> >
> > Move TP_fast_assign's define from trace_events_stage_2.h to
> > trace_events_stage_3.h because it is used there.
> >
> > Unify TRACE_EVENT's 5th argument's name to "assign"
> >
> > Impact: cleanup, no functionality changed
> >
> > Signed-off-by: Zhao Lei <[email protected]
> <mailto:[email protected]>>
>
> I see what you are doing here, but I'm a little hesitant to apply it.
> I'm getting ready to travel, so I do not have the time to look deeper at
> this today. I'll try to do it while I'm traveling.
>
> Thanks,
>
> -- Steve
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to [email protected]
> <mailto:[email protected]>
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>

2009-04-08 07:16:24

by Jiaying Zhang

[permalink] [raw]
Subject: Re: Re: [PATCH 2/2] ftrace: Code cleanup for kernel/trace/trace_events_stage_*.h

Actually my real concern is that the current event tracing interface in ftrace
is still too complex. I am putting those macros together and see whether we
can merge some of them.

Jiaying

On Wed, Apr 8, 2009 at 12:04 AM, Zhaolei <[email protected]> wrote:
> Jiaying Zhang wrote:
>> Hi Steve,
>>
>> I am looking at ftrace code more closely. It is also not clear to me why we
> Hello, Zhang
>
> Different people may have different taste on codeing style, but IMHO, separate
> those complex definition into several files make me easy to understand.
> It is only my personal code liking...
>
> B.R.
> Zhaolei
>
>> want to define the trace event macros in three stage header files. I wonder
>> whether it would be clearer if we merge them together. Then people don't
>> need to look at three files to understand what is going on in event tracing.
>>
>> Jiaying
>>
>> On Mon, Apr 6, 2009 at 6:54 PM, Steven Rostedt <[email protected]
>> <mailto:[email protected]>> wrote:
>>
>>
>> ? ? On Fri, 3 Apr 2009, Zhaolei wrote:
>>
>> ? ? > Add TRACE_FORMAT's define for trace_events_stage_2.h.
>> ? ? > Although it is already defined in trace_events_stage_1.h, we
>> ? ? should make each
>> ? ? > function independence.
>> ? ? >
>> ? ? > Move TP_fast_assign's define from trace_events_stage_2.h to
>> ? ? > trace_events_stage_3.h because it is used there.
>> ? ? >
>> ? ? > Unify TRACE_EVENT's 5th argument's name to "assign"
>> ? ? >
>> ? ? > Impact: cleanup, no functionality changed
>> ? ? >
>> ? ? > Signed-off-by: Zhao Lei <[email protected]
>> ? ? <mailto:[email protected]>>
>>
>> ? ? I see what you are doing here, but I'm a little hesitant to apply it.
>> ? ? I'm getting ready to travel, so I do not have the time to look deeper at
>> ? ? this today. I'll try to do it while I'm traveling.
>>
>> ? ? Thanks,
>>
>> ? ? -- Steve
>> ? ? --
>> ? ? To unsubscribe from this list: send the line "unsubscribe
>> ? ? linux-kernel" in
>> ? ? the body of a message to [email protected]
>> ? ? <mailto:[email protected]>
>> ? ? More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>> ? ? Please read the FAQ at ?http://www.tux.org/lkml/
>>
>>
>
>
>

2009-04-16 14:55:21

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 2/2] ftrace: Code cleanup for kernel/trace/trace_events_stage_*.h


On Mon, 6 Apr 2009, Steven Rostedt wrote:

>
> On Fri, 3 Apr 2009, Zhaolei wrote:
>
> > Add TRACE_FORMAT's define for trace_events_stage_2.h.
> > Although it is already defined in trace_events_stage_1.h, we should make each
> > function independence.
> >
> > Move TP_fast_assign's define from trace_events_stage_2.h to
> > trace_events_stage_3.h because it is used there.
> >
> > Unify TRACE_EVENT's 5th argument's name to "assign"
> >
> > Impact: cleanup, no functionality changed
> >
> > Signed-off-by: Zhao Lei <[email protected]>
>
> I see what you are doing here, but I'm a little hesitant to apply it.
> I'm getting ready to travel, so I do not have the time to look deeper at
> this today. I'll try to do it while I'm traveling.

Hi Zhao,

As you probably noticed, the staging code has been combined into
include/trace/ftrace.h

Is this patch (or a variant) still needed?

-- Steve

2009-04-17 00:32:37

by Zhao Lei

[permalink] [raw]
Subject: Re: [PATCH 2/2] ftrace: Code cleanup for kernel/trace/trace_events_stage_*.h

* From: "Steven Rostedt" <[email protected]>
>
> On Mon, 6 Apr 2009, Steven Rostedt wrote:
>
>>
>> On Fri, 3 Apr 2009, Zhaolei wrote:
>>
>> > Add TRACE_FORMAT's define for trace_events_stage_2.h.
>> > Although it is already defined in trace_events_stage_1.h, we should make each
>> > function independence.
>> >
>> > Move TP_fast_assign's define from trace_events_stage_2.h to
>> > trace_events_stage_3.h because it is used there.
>> >
>> > Unify TRACE_EVENT's 5th argument's name to "assign"
>> >
>> > Impact: cleanup, no functionality changed
>> >
>> > Signed-off-by: Zhao Lei <[email protected]>
>>
>> I see what you are doing here, but I'm a little hesitant to apply it.
>> I'm getting ready to travel, so I do not have the time to look deeper at
>> this today. I'll try to do it while I'm traveling.
>
> Hi Zhao,
>
> As you probably noticed, the staging code has been combined into
> include/trace/ftrace.h
>
> Is this patch (or a variant) still needed?
Hello, Steven

I see your patchset, it is nice.
This patch is no use now, please ignore it.

Thanks
Zhaolei
>
> -- Steve
>
>
>????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?