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

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

[Zhaolei]

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 <zhaolei@cn.fujitsu.com>
---
 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

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

[Zhaolei]

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 <zhaolei@cn.fujitsu.com>
---
 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

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

[Steven Rostedt]

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 <zhaolei@cn.fujitsu.com>

Thanks, applied.

-- Steve

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

[Steven Rostedt]

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 <zhaolei@cn.fujitsu.com>

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

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

[Zhaolei]

Commit-ID:  1bbe2a83ab68e5cf8c66c372c7cb3b51910c2cfe
Gitweb:     http://git.kernel.org/tip/1bbe2a83ab68e5cf8c66c372c7cb3b51910c2cfe
Author:     Zhaolei <zhaolei@cn.fujitsu.com>
AuthorDate: Fri, 3 Apr 2009 18:24:46 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
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 <zhaolei@cn.fujitsu.com>
LKML-Reference: <49D5E3EE.70201@cn.fujitsu.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 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)							\

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

[Jiaying Zhang]

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 <rostedt@goodmis.org> 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 <zhaolei@cn.fujitsu.com>
>
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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

[Zhaolei]

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 <rostedt@goodmis.org
> <mailto:rostedt@goodmis.org>> 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 <zhaolei@cn.fujitsu.com
>     <mailto:zhaolei@cn.fujitsu.com>>
> 
>     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 majordomo@vger.kernel.org
>     <mailto:majordomo@vger.kernel.org>
>     More majordomo info at  http://vger.kernel.org/majordomo-info.html
>     Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

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

[Jiaying Zhang]

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 <zhaolei@cn.fujitsu.com> 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 <rostedt@goodmis.org
>> <mailto:rostedt@goodmis.org>> 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 <zhaolei@cn.fujitsu.com
>>     <mailto:zhaolei@cn.fujitsu.com>>
>>
>>     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 majordomo@vger.kernel.org
>>     <mailto:majordomo@vger.kernel.org>
>>     More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>     Please read the FAQ at  http://www.tux.org/lkml/
>>
>>
>
>
>

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

[Steven Rostedt]

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 <zhaolei@cn.fujitsu.com>
> 
> 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

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

[Zhaolei]

* From: "Steven Rostedt" <rostedt@goodmis.org>
> 
> 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 <zhaolei@cn.fujitsu.com>
>> 
>> 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Ç+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥