Re: [PATCH 11/18] tracing: Add symbol type to function based events

2018-02-08 Thread Steven Rostedt 
On Thu, 8 Feb 2018 20:03:41 +0900
Namhyung Kim  wrote:

> > @@ -76,6 +76,7 @@ typedef u64 x64;
> >  typedef u32 x32;
> >  typedef u16 x16;
> >  typedef u8 x8;
> > +typedef void * symbol;
> >  
> >  #define TYPE_TUPLE(type)   \
> > { #type, sizeof(type), is_signed_type(type) }
> > @@ -97,7 +98,8 @@ typedef u8 x8;
> > TYPE_TUPLE(x16),\
> > TYPE_TUPLE(u8), \
> > TYPE_TUPLE(s8), \
> > -   TYPE_TUPLE(x8)
> > +   TYPE_TUPLE(x8), \
> > +   TYPE_TUPLE(symbol)
> >  
> >  static struct func_type {
> > char*name;
> > @@ -262,7 +264,7 @@ process_event(struct func_event *fevent, const char 
> > *token, enum func_states sta
> > switch (state) {
> > case FUNC_STATE_INIT:
> > unsign = 0;
> > -   if (!isalpha(token[0]))
> > +   if (!isalpha(token[0]) && token[0] != '_')
> > break;  
> 
> Hmm.. it seems that it needs to be moved to the patch 1?

Agreed.

> 
> 
> > /* Do not allow wild cards */
> > if (strstr(token, "*") || strstr(token, "?"))
> > @@ -305,7 +307,7 @@ process_event(struct func_event *fevent, const char 
> > *token, enum func_states sta
> > return FUNC_STATE_TYPE;
> >  
> > case FUNC_STATE_TYPE:
> > -   if (!isalpha(token[0]))
> > +   if (!isalpha(token[0]) || token[0] == '_')
> > break;  
> 
> Why is different that the above?  Anyway, '_' is not an alphabet..

That's a bug. I thought I fixed that. Ah, in v2 I fixed it in a later
patch. I'll restructure to fix it in the proper place.

Thanks!

-- Steve

Re: [PATCH 11/18] tracing: Add symbol type to function based events

2018-02-08 Thread Steven Rostedt 
On Thu, 8 Feb 2018 20:03:41 +0900
Namhyung Kim  wrote:

> > @@ -76,6 +76,7 @@ typedef u64 x64;
> >  typedef u32 x32;
> >  typedef u16 x16;
> >  typedef u8 x8;
> > +typedef void * symbol;
> >  
> >  #define TYPE_TUPLE(type)   \
> > { #type, sizeof(type), is_signed_type(type) }
> > @@ -97,7 +98,8 @@ typedef u8 x8;
> > TYPE_TUPLE(x16),\
> > TYPE_TUPLE(u8), \
> > TYPE_TUPLE(s8), \
> > -   TYPE_TUPLE(x8)
> > +   TYPE_TUPLE(x8), \
> > +   TYPE_TUPLE(symbol)
> >  
> >  static struct func_type {
> > char*name;
> > @@ -262,7 +264,7 @@ process_event(struct func_event *fevent, const char 
> > *token, enum func_states sta
> > switch (state) {
> > case FUNC_STATE_INIT:
> > unsign = 0;
> > -   if (!isalpha(token[0]))
> > +   if (!isalpha(token[0]) && token[0] != '_')
> > break;  
> 
> Hmm.. it seems that it needs to be moved to the patch 1?

Agreed.

> 
> 
> > /* Do not allow wild cards */
> > if (strstr(token, "*") || strstr(token, "?"))
> > @@ -305,7 +307,7 @@ process_event(struct func_event *fevent, const char 
> > *token, enum func_states sta
> > return FUNC_STATE_TYPE;
> >  
> > case FUNC_STATE_TYPE:
> > -   if (!isalpha(token[0]))
> > +   if (!isalpha(token[0]) || token[0] == '_')
> > break;  
> 
> Why is different that the above?  Anyway, '_' is not an alphabet..

That's a bug. I thought I fixed that. Ah, in v2 I fixed it in a later
patch. I'll restructure to fix it in the proper place.

Thanks!

-- Steve

Re: [PATCH 11/18] tracing: Add symbol type to function based events

2018-02-08 Thread Namhyung Kim 
On Fri, Feb 02, 2018 at 06:05:09PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" 
> 
> Add a special type "symbol" that will use %pS to display the field of a
> function based event.
> 
> Signed-off-by: Steven Rostedt (VMware) 
> ---
>  Documentation/trace/function-based-events.rst | 26 +-
>  kernel/trace/trace_event_ftrace.c | 13 ++---
>  2 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/trace/function-based-events.rst 
> b/Documentation/trace/function-based-events.rst
> index bdb28f433bfb..f18c8f3ef330 100644
> --- a/Documentation/trace/function-based-events.rst
> +++ b/Documentation/trace/function-based-events.rst
> @@ -98,7 +98,8 @@ as follows:
>   ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
>   's8' | 's16' | 's32' | 's64' |
>   'x8' | 'x16' | 'x32' | 'x64' |
> - 'char' | 'short' | 'int' | 'long' | 'size_t'
> + 'char' | 'short' | 'int' | 'long' | 'size_t' |
> +  'symbol'
>  
>   FIELD :=  |  INDEX |  OFFSET |  OFFSET INDEX
>  
> @@ -243,3 +244,26 @@ The above will take the parameter value, add it by 4, 
> then index it by two
>  8 byte words. It's the same in C as: (u64 *)((void *)param + 4)[2]
>  
>   Note: "int skb[32]" is the same as "int skb+4[31]".
> +
> +
> +Symbols (function names)
> +
> +
> +To display kallsyms "%pS" type of output, use the special type "symbol".
> +
> +Again, using gdb to find the offset of the "func" field of struct work_struct
> +
> +(gdb) printf "%d\n", &((struct work_struct *)0)->func
> +24
> +
> + Both "symbol func[3]" and "symbol func+24[0]" will work.
> +
> + # echo '__queue_work(int cpu, x64 wq, symbol func[3])' > function_events
> +
> + # echo 1 > events/functions/__queue_work/enable
> + # cat trace
> +   bash-1641  [007] d..2  6241.171332: 
> queue_work_on->__queue_work(cpu=128, wq=88011a010e00, 
> func=flush_to_ldisc+0x0/0xa0)
> +   bash-1641  [007] d..2  6241.171460: 
> queue_work_on->__queue_work(cpu=128, wq=88011a010e00, 
> func=flush_to_ldisc+0x0/0xa0)
> + -0 [000] dNs3  6241.172004: 
> delayed_work_timer_fn->__queue_work(cpu=128, wq=88011a010800, 
> func=vmstat_shepherd+0x0/0xb0)
> + worker/0:2-1689  [000] d..2  6241.172026: 
> __queue_delayed_work->__queue_work(cpu=7, wq=88011a11da00, 
> func=vmstat_update+0x0/0x70)
> + -0 [005] d.s3  6241.347996: 
> queue_work_on->__queue_work(cpu=128, wq=88011a011200, 
> func=fb_flashcursor+0x0/0x110 [fb])
> diff --git a/kernel/trace/trace_event_ftrace.c 
> b/kernel/trace/trace_event_ftrace.c
> index 0f2650e97e49..ba10177b9bd6 100644
> --- a/kernel/trace/trace_event_ftrace.c
> +++ b/kernel/trace/trace_event_ftrace.c
> @@ -76,6 +76,7 @@ typedef u64 x64;
>  typedef u32 x32;
>  typedef u16 x16;
>  typedef u8 x8;
> +typedef void * symbol;
>  
>  #define TYPE_TUPLE(type) \
>   { #type, sizeof(type), is_signed_type(type) }
> @@ -97,7 +98,8 @@ typedef u8 x8;
>   TYPE_TUPLE(x16),\
>   TYPE_TUPLE(u8), \
>   TYPE_TUPLE(s8), \
> - TYPE_TUPLE(x8)
> + TYPE_TUPLE(x8), \
> + TYPE_TUPLE(symbol)
>  
>  static struct func_type {
>   char*name;
> @@ -262,7 +264,7 @@ process_event(struct func_event *fevent, const char 
> *token, enum func_states sta
>   switch (state) {
>   case FUNC_STATE_INIT:
>   unsign = 0;
> - if (!isalpha(token[0]))
> + if (!isalpha(token[0]) && token[0] != '_')
>   break;

Hmm.. it seems that it needs to be moved to the patch 1?


>   /* Do not allow wild cards */
>   if (strstr(token, "*") || strstr(token, "?"))
> @@ -305,7 +307,7 @@ process_event(struct func_event *fevent, const char 
> *token, enum func_states sta
>   return FUNC_STATE_TYPE;
>  
>   case FUNC_STATE_TYPE:
> - if (!isalpha(token[0]))
> + if (!isalpha(token[0]) || token[0] == '_')
>   break;

Why is different that the above?  Anyway, '_' is not an alphabet..

Thanks,
Namhyung


>   if (WARN_ON(!fevent->last_arg))
>   break;
> @@ -472,6 +474,11 @@ static void make_fmt(struct func_arg *arg, char *fmt)
>  {
>   int c = 0;
>  
> + if (arg->func_type == FUNC_TYPE_symbol) {
> + strcpy(fmt, "%pS");
> + return;
> + }
> +
>   fmt[c++] = '%';
>  
>   if (arg->size == 8) {
> -- 
> 2.15.1
> 
>

Re: [PATCH 11/18] tracing: Add symbol type to function based events

2018-02-08 Thread Namhyung Kim 
On Fri, Feb 02, 2018 at 06:05:09PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" 
> 
> Add a special type "symbol" that will use %pS to display the field of a
> function based event.
> 
> Signed-off-by: Steven Rostedt (VMware) 
> ---
>  Documentation/trace/function-based-events.rst | 26 +-
>  kernel/trace/trace_event_ftrace.c | 13 ++---
>  2 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/trace/function-based-events.rst 
> b/Documentation/trace/function-based-events.rst
> index bdb28f433bfb..f18c8f3ef330 100644
> --- a/Documentation/trace/function-based-events.rst
> +++ b/Documentation/trace/function-based-events.rst
> @@ -98,7 +98,8 @@ as follows:
>   ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
>   's8' | 's16' | 's32' | 's64' |
>   'x8' | 'x16' | 'x32' | 'x64' |
> - 'char' | 'short' | 'int' | 'long' | 'size_t'
> + 'char' | 'short' | 'int' | 'long' | 'size_t' |
> +  'symbol'
>  
>   FIELD :=  |  INDEX |  OFFSET |  OFFSET INDEX
>  
> @@ -243,3 +244,26 @@ The above will take the parameter value, add it by 4, 
> then index it by two
>  8 byte words. It's the same in C as: (u64 *)((void *)param + 4)[2]
>  
>   Note: "int skb[32]" is the same as "int skb+4[31]".
> +
> +
> +Symbols (function names)
> +
> +
> +To display kallsyms "%pS" type of output, use the special type "symbol".
> +
> +Again, using gdb to find the offset of the "func" field of struct work_struct
> +
> +(gdb) printf "%d\n", &((struct work_struct *)0)->func
> +24
> +
> + Both "symbol func[3]" and "symbol func+24[0]" will work.
> +
> + # echo '__queue_work(int cpu, x64 wq, symbol func[3])' > function_events
> +
> + # echo 1 > events/functions/__queue_work/enable
> + # cat trace
> +   bash-1641  [007] d..2  6241.171332: 
> queue_work_on->__queue_work(cpu=128, wq=88011a010e00, 
> func=flush_to_ldisc+0x0/0xa0)
> +   bash-1641  [007] d..2  6241.171460: 
> queue_work_on->__queue_work(cpu=128, wq=88011a010e00, 
> func=flush_to_ldisc+0x0/0xa0)
> + -0 [000] dNs3  6241.172004: 
> delayed_work_timer_fn->__queue_work(cpu=128, wq=88011a010800, 
> func=vmstat_shepherd+0x0/0xb0)
> + worker/0:2-1689  [000] d..2  6241.172026: 
> __queue_delayed_work->__queue_work(cpu=7, wq=88011a11da00, 
> func=vmstat_update+0x0/0x70)
> + -0 [005] d.s3  6241.347996: 
> queue_work_on->__queue_work(cpu=128, wq=88011a011200, 
> func=fb_flashcursor+0x0/0x110 [fb])
> diff --git a/kernel/trace/trace_event_ftrace.c 
> b/kernel/trace/trace_event_ftrace.c
> index 0f2650e97e49..ba10177b9bd6 100644
> --- a/kernel/trace/trace_event_ftrace.c
> +++ b/kernel/trace/trace_event_ftrace.c
> @@ -76,6 +76,7 @@ typedef u64 x64;
>  typedef u32 x32;
>  typedef u16 x16;
>  typedef u8 x8;
> +typedef void * symbol;
>  
>  #define TYPE_TUPLE(type) \
>   { #type, sizeof(type), is_signed_type(type) }
> @@ -97,7 +98,8 @@ typedef u8 x8;
>   TYPE_TUPLE(x16),\
>   TYPE_TUPLE(u8), \
>   TYPE_TUPLE(s8), \
> - TYPE_TUPLE(x8)
> + TYPE_TUPLE(x8), \
> + TYPE_TUPLE(symbol)
>  
>  static struct func_type {
>   char*name;
> @@ -262,7 +264,7 @@ process_event(struct func_event *fevent, const char 
> *token, enum func_states sta
>   switch (state) {
>   case FUNC_STATE_INIT:
>   unsign = 0;
> - if (!isalpha(token[0]))
> + if (!isalpha(token[0]) && token[0] != '_')
>   break;

Hmm.. it seems that it needs to be moved to the patch 1?


>   /* Do not allow wild cards */
>   if (strstr(token, "*") || strstr(token, "?"))
> @@ -305,7 +307,7 @@ process_event(struct func_event *fevent, const char 
> *token, enum func_states sta
>   return FUNC_STATE_TYPE;
>  
>   case FUNC_STATE_TYPE:
> - if (!isalpha(token[0]))
> + if (!isalpha(token[0]) || token[0] == '_')
>   break;

Why is different that the above?  Anyway, '_' is not an alphabet..

Thanks,
Namhyung


>   if (WARN_ON(!fevent->last_arg))
>   break;
> @@ -472,6 +474,11 @@ static void make_fmt(struct func_arg *arg, char *fmt)
>  {
>   int c = 0;
>  
> + if (arg->func_type == FUNC_TYPE_symbol) {
> + strcpy(fmt, "%pS");
> + return;
> + }
> +
>   fmt[c++] = '%';
>  
>   if (arg->size == 8) {
> -- 
> 2.15.1
> 
>

[PATCH 11/18] tracing: Add symbol type to function based events

2018-02-02 Thread Steven Rostedt 
From: "Steven Rostedt (VMware)" 

Add a special type "symbol" that will use %pS to display the field of a
function based event.

Signed-off-by: Steven Rostedt (VMware) 
---
 Documentation/trace/function-based-events.rst | 26 +-
 kernel/trace/trace_event_ftrace.c | 13 ++---
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst 
b/Documentation/trace/function-based-events.rst
index bdb28f433bfb..f18c8f3ef330 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -98,7 +98,8 @@ as follows:
  ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
  's8' | 's16' | 's32' | 's64' |
  'x8' | 'x16' | 'x32' | 'x64' |
- 'char' | 'short' | 'int' | 'long' | 'size_t'
+ 'char' | 'short' | 'int' | 'long' | 'size_t' |
+'symbol'
 
  FIELD :=  |  INDEX |  OFFSET |  OFFSET INDEX
 
@@ -243,3 +244,26 @@ The above will take the parameter value, add it by 4, then 
index it by two
 8 byte words. It's the same in C as: (u64 *)((void *)param + 4)[2]
 
  Note: "int skb[32]" is the same as "int skb+4[31]".
+
+
+Symbols (function names)
+
+
+To display kallsyms "%pS" type of output, use the special type "symbol".
+
+Again, using gdb to find the offset of the "func" field of struct work_struct
+
+(gdb) printf "%d\n", &((struct work_struct *)0)->func
+24
+
+ Both "symbol func[3]" and "symbol func+24[0]" will work.
+
+ # echo '__queue_work(int cpu, x64 wq, symbol func[3])' > function_events
+
+ # echo 1 > events/functions/__queue_work/enable
+ # cat trace
+   bash-1641  [007] d..2  6241.171332: 
queue_work_on->__queue_work(cpu=128, wq=88011a010e00, 
func=flush_to_ldisc+0x0/0xa0)
+   bash-1641  [007] d..2  6241.171460: 
queue_work_on->__queue_work(cpu=128, wq=88011a010e00, 
func=flush_to_ldisc+0x0/0xa0)
+ -0 [000] dNs3  6241.172004: 
delayed_work_timer_fn->__queue_work(cpu=128, wq=88011a010800, 
func=vmstat_shepherd+0x0/0xb0)
+ worker/0:2-1689  [000] d..2  6241.172026: 
__queue_delayed_work->__queue_work(cpu=7, wq=88011a11da00, 
func=vmstat_update+0x0/0x70)
+ -0 [005] d.s3  6241.347996: 
queue_work_on->__queue_work(cpu=128, wq=88011a011200, 
func=fb_flashcursor+0x0/0x110 [fb])
diff --git a/kernel/trace/trace_event_ftrace.c 
b/kernel/trace/trace_event_ftrace.c
index 0f2650e97e49..ba10177b9bd6 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -76,6 +76,7 @@ typedef u64 x64;
 typedef u32 x32;
 typedef u16 x16;
 typedef u8 x8;
+typedef void * symbol;
 
 #define TYPE_TUPLE(type)   \
{ #type, sizeof(type), is_signed_type(type) }
@@ -97,7 +98,8 @@ typedef u8 x8;
TYPE_TUPLE(x16),\
TYPE_TUPLE(u8), \
TYPE_TUPLE(s8), \
-   TYPE_TUPLE(x8)
+   TYPE_TUPLE(x8), \
+   TYPE_TUPLE(symbol)
 
 static struct func_type {
char*name;
@@ -262,7 +264,7 @@ process_event(struct func_event *fevent, const char *token, 
enum func_states sta
switch (state) {
case FUNC_STATE_INIT:
unsign = 0;
-   if (!isalpha(token[0]))
+   if (!isalpha(token[0]) && token[0] != '_')
break;
/* Do not allow wild cards */
if (strstr(token, "*") || strstr(token, "?"))
@@ -305,7 +307,7 @@ process_event(struct func_event *fevent, const char *token, 
enum func_states sta
return FUNC_STATE_TYPE;
 
case FUNC_STATE_TYPE:
-   if (!isalpha(token[0]))
+   if (!isalpha(token[0]) || token[0] == '_')
break;
if (WARN_ON(!fevent->last_arg))
break;
@@ -472,6 +474,11 @@ static void make_fmt(struct func_arg *arg, char *fmt)
 {
int c = 0;
 
+   if (arg->func_type == FUNC_TYPE_symbol) {
+   strcpy(fmt, "%pS");
+   return;
+   }
+
fmt[c++] = '%';
 
if (arg->size == 8) {
-- 
2.15.1

[PATCH 11/18] tracing: Add symbol type to function based events

2018-02-02 Thread Steven Rostedt 
From: "Steven Rostedt (VMware)" 

Add a special type "symbol" that will use %pS to display the field of a
function based event.

Signed-off-by: Steven Rostedt (VMware) 
---
 Documentation/trace/function-based-events.rst | 26 +-
 kernel/trace/trace_event_ftrace.c | 13 ++---
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst 
b/Documentation/trace/function-based-events.rst
index bdb28f433bfb..f18c8f3ef330 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -98,7 +98,8 @@ as follows:
  ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
  's8' | 's16' | 's32' | 's64' |
  'x8' | 'x16' | 'x32' | 'x64' |
- 'char' | 'short' | 'int' | 'long' | 'size_t'
+ 'char' | 'short' | 'int' | 'long' | 'size_t' |
+'symbol'
 
  FIELD :=  |  INDEX |  OFFSET |  OFFSET INDEX
 
@@ -243,3 +244,26 @@ The above will take the parameter value, add it by 4, then 
index it by two
 8 byte words. It's the same in C as: (u64 *)((void *)param + 4)[2]
 
  Note: "int skb[32]" is the same as "int skb+4[31]".
+
+
+Symbols (function names)
+
+
+To display kallsyms "%pS" type of output, use the special type "symbol".
+
+Again, using gdb to find the offset of the "func" field of struct work_struct
+
+(gdb) printf "%d\n", &((struct work_struct *)0)->func
+24
+
+ Both "symbol func[3]" and "symbol func+24[0]" will work.
+
+ # echo '__queue_work(int cpu, x64 wq, symbol func[3])' > function_events
+
+ # echo 1 > events/functions/__queue_work/enable
+ # cat trace
+   bash-1641  [007] d..2  6241.171332: 
queue_work_on->__queue_work(cpu=128, wq=88011a010e00, 
func=flush_to_ldisc+0x0/0xa0)
+   bash-1641  [007] d..2  6241.171460: 
queue_work_on->__queue_work(cpu=128, wq=88011a010e00, 
func=flush_to_ldisc+0x0/0xa0)
+ -0 [000] dNs3  6241.172004: 
delayed_work_timer_fn->__queue_work(cpu=128, wq=88011a010800, 
func=vmstat_shepherd+0x0/0xb0)
+ worker/0:2-1689  [000] d..2  6241.172026: 
__queue_delayed_work->__queue_work(cpu=7, wq=88011a11da00, 
func=vmstat_update+0x0/0x70)
+ -0 [005] d.s3  6241.347996: 
queue_work_on->__queue_work(cpu=128, wq=88011a011200, 
func=fb_flashcursor+0x0/0x110 [fb])
diff --git a/kernel/trace/trace_event_ftrace.c 
b/kernel/trace/trace_event_ftrace.c
index 0f2650e97e49..ba10177b9bd6 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -76,6 +76,7 @@ typedef u64 x64;
 typedef u32 x32;
 typedef u16 x16;
 typedef u8 x8;
+typedef void * symbol;
 
 #define TYPE_TUPLE(type)   \
{ #type, sizeof(type), is_signed_type(type) }
@@ -97,7 +98,8 @@ typedef u8 x8;
TYPE_TUPLE(x16),\
TYPE_TUPLE(u8), \
TYPE_TUPLE(s8), \
-   TYPE_TUPLE(x8)
+   TYPE_TUPLE(x8), \
+   TYPE_TUPLE(symbol)
 
 static struct func_type {
char*name;
@@ -262,7 +264,7 @@ process_event(struct func_event *fevent, const char *token, 
enum func_states sta
switch (state) {
case FUNC_STATE_INIT:
unsign = 0;
-   if (!isalpha(token[0]))
+   if (!isalpha(token[0]) && token[0] != '_')
break;
/* Do not allow wild cards */
if (strstr(token, "*") || strstr(token, "?"))
@@ -305,7 +307,7 @@ process_event(struct func_event *fevent, const char *token, 
enum func_states sta
return FUNC_STATE_TYPE;
 
case FUNC_STATE_TYPE:
-   if (!isalpha(token[0]))
+   if (!isalpha(token[0]) || token[0] == '_')
break;
if (WARN_ON(!fevent->last_arg))
break;
@@ -472,6 +474,11 @@ static void make_fmt(struct func_arg *arg, char *fmt)
 {
int c = 0;
 
+   if (arg->func_type == FUNC_TYPE_symbol) {
+   strcpy(fmt, "%pS");
+   return;
+   }
+
fmt[c++] = '%';
 
if (arg->size == 8) {
-- 
2.15.1