Re: [PATCH 09/18] tracing: Add indexing of arguments for function based events

2018-02-08 Thread Steven Rostedt
On Fri, 9 Feb 2018 08:56:15 +0900
Namhyung Kim  wrote:

> On Thu, Feb 08, 2018 at 10:43:43AM -0500, Steven Rostedt wrote:
> > On Thu, 8 Feb 2018 19:59:24 +0900
> > Namhyung Kim  wrote:
> >   
> > > > @@ -347,6 +361,8 @@ static long long get_arg(struct func_arg *arg, 
> > > > unsigned long val)
> > > > char buf[8];
> > > > int ret;
> > > >  
> > > > +   val += arg->index;
> > > > +
> > > > if (!arg->indirect)
> > > > return val;
> > > 
> > > So this also works without the indirect, and just add the immediate to
> > > the value.  
> > 
> > Not sure what you are asking here. The immediate adds to the current
> > value, where as the indirect will then look what's at that location.  
> 
> I expected that the immediate offset is only meaningful with the
> indirect (dereference) as the doc says just about it.  So I asked it
> was intentional or not.
>

Yes it is intentional, but rather useless without an indirect. I mean,
you could just add to the value if you want :-)

The reason it doesn't need the indirect is because there's some types
(arrays and strings) that don't need the indirect. For example, with
the net_device with the perm_addr at 558 bytes away:

 echo 'ip_rcv(NULL, x8[6] perm_addr+558)' > function_events

produces:

  -0 [003] ..s3  1809.074329: 
__netif_receive_skb_core->ip_rcv(perm_addr=b4,b5,2f,ce,18,65)

-- Steve


Re: [PATCH 09/18] tracing: Add indexing of arguments for function based events

2018-02-08 Thread Namhyung Kim
On Thu, Feb 08, 2018 at 10:43:43AM -0500, Steven Rostedt wrote:
> On Thu, 8 Feb 2018 19:59:24 +0900
> Namhyung Kim  wrote:
> 
> > > @@ -347,6 +361,8 @@ static long long get_arg(struct func_arg *arg, 
> > > unsigned long val)
> > >   char buf[8];
> > >   int ret;
> > >  
> > > + val += arg->index;
> > > +
> > >   if (!arg->indirect)
> > >   return val;  
> > 
> > So this also works without the indirect, and just add the immediate to
> > the value.
> 
> Not sure what you are asking here. The immediate adds to the current
> value, where as the indirect will then look what's at that location.

I expected that the immediate offset is only meaningful with the
indirect (dereference) as the doc says just about it.  So I asked it
was intentional or not.

Thanks,
Namhyung


Re: [PATCH 09/18] tracing: Add indexing of arguments for function based events

2018-02-08 Thread Steven Rostedt
On Thu, 8 Feb 2018 19:59:24 +0900
Namhyung Kim  wrote:

> > @@ -347,6 +361,8 @@ static long long get_arg(struct func_arg *arg, unsigned 
> > long val)
> > char buf[8];
> > int ret;
> >  
> > +   val += arg->index;
> > +
> > if (!arg->indirect)
> > return val;  
> 
> So this also works without the indirect, and just add the immediate to
> the value.

Not sure what you are asking here. The immediate adds to the current
value, where as the indirect will then look what's at that location.

If the arg (val) is 0xabcd

u64 val+8

Will return: 0xabcd0008

u64 val[1]

will return what's at location 0xabcd0008

"u64 val+8[0]" is the same as "u64 val[1]"

Note: "u64 val[0]+8" will return what's at location 0xabcd
plus 8.

-- Steve


Re: [PATCH 09/18] tracing: Add indexing of arguments for function based events

2018-02-08 Thread Namhyung Kim
On Fri, Feb 02, 2018 at 06:05:07PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" 
> 
> Currently reading of 8 byte words can only happen 8 bytes aligned from the
> argument. But there may be cases that they are 4 bytes aligned. To make the
> capturing of arguments more flexible, add a plus '+' operator that can index
> the variable at arbitrary indexes to get any location.
> 
>  u64 arg+4[3]
> 
> Will get an 8 byte word at index 28 (3 * 8 + 4)
> 
> Signed-off-by: Steven Rostedt (VMware) 
> ---
>  Documentation/trace/function-based-events.rst | 24 +++-
>  kernel/trace/trace_event_ftrace.c | 18 ++
>  2 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/trace/function-based-events.rst 
> b/Documentation/trace/function-based-events.rst
> index 72e3e7730d63..bdb28f433bfb 100644
> --- a/Documentation/trace/function-based-events.rst
> +++ b/Documentation/trace/function-based-events.rst
> @@ -100,10 +100,12 @@ as follows:
>   'x8' | 'x16' | 'x32' | 'x64' |
>   'char' | 'short' | 'int' | 'long' | 'size_t'
>  
> - FIELD :=  |  INDEX
> + FIELD :=  |  INDEX |  OFFSET |  OFFSET INDEX
>  
>   INDEX := '['  ']'
>  
> + OFFSET := '+' 
> +
>   Where  is a unique string starting with an alphabetic character
>   and consists only of letters and numbers and underscores.
>  
> @@ -221,3 +223,23 @@ format:
>  print fmt: "%pS->%pS(skb=%u)", REC->__ip, REC->__parent_ip, REC->skb
>  
>  It is now printed with a "%u".
> +
> +
> +Offsets
> +===
> +
> +After the name of the variable, brackets '[' number ']' will index the value 
> of
> +the argument by the number given times the size of the field.
> +
> + int field[5] will dereference the value of the argument 20 bytes away (4 * 
> 5)
> +  as sizeof(int) is 4.
> +
> +If there's a case where the type is of 8 bytes in size but is not 8 bytes
> +alligned in the structure, an offset may be required.
> +
> +  For example: x64 param+4[2]
> +
> +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]".
> diff --git a/kernel/trace/trace_event_ftrace.c 
> b/kernel/trace/trace_event_ftrace.c
> index 9548b93eb8cd..4c23fa18453d 100644
> --- a/kernel/trace/trace_event_ftrace.c
> +++ b/kernel/trace/trace_event_ftrace.c
> @@ -19,6 +19,7 @@ struct func_arg {
>   char*type;
>   char*name;
>   longindirect;
> + longindex;
>   short   offset;
>   short   size;
>   s8  arg;
> @@ -62,6 +63,7 @@ enum func_states {
>   FUNC_STATE_INDIRECT,
>   FUNC_STATE_UNSIGNED,
>   FUNC_STATE_PIPE,
> + FUNC_STATE_PLUS,
>   FUNC_STATE_TYPE,
>   FUNC_STATE_VAR,
>   FUNC_STATE_COMMA,
> @@ -182,6 +184,7 @@ static char *next_token(char **ptr, char *last)
>   *str == ']' ||
>   *str == ',' ||
>   *str == '|' ||
> + *str == '+' ||
>   *str == ')')
>   break;
>   }
> @@ -323,6 +326,15 @@ process_event(struct func_event *fevent, const char 
> *token, enum func_states sta
>   }
>   break;
>  
> + case FUNC_STATE_PLUS:
> + if (WARN_ON(!fevent->last_arg))
> + break;
> + ret = kstrtol(token, 0, &val);
> + if (ret)
> + break;
> + fevent->last_arg->index += val;
> + return FUNC_STATE_VAR;
> +
>   case FUNC_STATE_VAR:
>   switch (token[0]) {
>   case ')':
> @@ -331,6 +343,8 @@ process_event(struct func_event *fevent, const char 
> *token, enum func_states sta
>   return FUNC_STATE_COMMA;
>   case '|':
>   return FUNC_STATE_PIPE;
> + case '+':
> + return FUNC_STATE_PLUS;
>   case '[':
>   return FUNC_STATE_BRACKET;
>   }
> @@ -347,6 +361,8 @@ static long long get_arg(struct func_arg *arg, unsigned 
> long val)
>   char buf[8];
>   int ret;
>  
> + val += arg->index;
> +
>   if (!arg->indirect)
>   return val;

So this also works without the indirect, and just add the immediate to
the value.

Thanks,
Namhyung


>  
> @@ -779,6 +795,8 @@ static int func_event_seq_show(struct seq_file *m, void 
> *v)
>   last_arg = arg->arg;
>   comma = true;
>   seq_printf(m, "%s %s", arg->type, arg->name);
> + if (arg->index)
> + seq_printf(m, "+%ld", arg->index);
>   if (arg->indirect && arg->size)
>   seq_printf(m, "[%ld]",
>   

[PATCH 09/18] tracing: Add indexing of arguments for function based events

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

Currently reading of 8 byte words can only happen 8 bytes aligned from the
argument. But there may be cases that they are 4 bytes aligned. To make the
capturing of arguments more flexible, add a plus '+' operator that can index
the variable at arbitrary indexes to get any location.

 u64 arg+4[3]

Will get an 8 byte word at index 28 (3 * 8 + 4)

Signed-off-by: Steven Rostedt (VMware) 
---
 Documentation/trace/function-based-events.rst | 24 +++-
 kernel/trace/trace_event_ftrace.c | 18 ++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/Documentation/trace/function-based-events.rst 
b/Documentation/trace/function-based-events.rst
index 72e3e7730d63..bdb28f433bfb 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -100,10 +100,12 @@ as follows:
  'x8' | 'x16' | 'x32' | 'x64' |
  'char' | 'short' | 'int' | 'long' | 'size_t'
 
- FIELD :=  |  INDEX
+ FIELD :=  |  INDEX |  OFFSET |  OFFSET INDEX
 
  INDEX := '['  ']'
 
+ OFFSET := '+' 
+
  Where  is a unique string starting with an alphabetic character
  and consists only of letters and numbers and underscores.
 
@@ -221,3 +223,23 @@ format:
 print fmt: "%pS->%pS(skb=%u)", REC->__ip, REC->__parent_ip, REC->skb
 
 It is now printed with a "%u".
+
+
+Offsets
+===
+
+After the name of the variable, brackets '[' number ']' will index the value of
+the argument by the number given times the size of the field.
+
+ int field[5] will dereference the value of the argument 20 bytes away (4 * 5)
+  as sizeof(int) is 4.
+
+If there's a case where the type is of 8 bytes in size but is not 8 bytes
+alligned in the structure, an offset may be required.
+
+  For example: x64 param+4[2]
+
+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]".
diff --git a/kernel/trace/trace_event_ftrace.c 
b/kernel/trace/trace_event_ftrace.c
index 9548b93eb8cd..4c23fa18453d 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -19,6 +19,7 @@ struct func_arg {
char*type;
char*name;
longindirect;
+   longindex;
short   offset;
short   size;
s8  arg;
@@ -62,6 +63,7 @@ enum func_states {
FUNC_STATE_INDIRECT,
FUNC_STATE_UNSIGNED,
FUNC_STATE_PIPE,
+   FUNC_STATE_PLUS,
FUNC_STATE_TYPE,
FUNC_STATE_VAR,
FUNC_STATE_COMMA,
@@ -182,6 +184,7 @@ static char *next_token(char **ptr, char *last)
*str == ']' ||
*str == ',' ||
*str == '|' ||
+   *str == '+' ||
*str == ')')
break;
}
@@ -323,6 +326,15 @@ process_event(struct func_event *fevent, const char 
*token, enum func_states sta
}
break;
 
+   case FUNC_STATE_PLUS:
+   if (WARN_ON(!fevent->last_arg))
+   break;
+   ret = kstrtol(token, 0, &val);
+   if (ret)
+   break;
+   fevent->last_arg->index += val;
+   return FUNC_STATE_VAR;
+
case FUNC_STATE_VAR:
switch (token[0]) {
case ')':
@@ -331,6 +343,8 @@ process_event(struct func_event *fevent, const char *token, 
enum func_states sta
return FUNC_STATE_COMMA;
case '|':
return FUNC_STATE_PIPE;
+   case '+':
+   return FUNC_STATE_PLUS;
case '[':
return FUNC_STATE_BRACKET;
}
@@ -347,6 +361,8 @@ static long long get_arg(struct func_arg *arg, unsigned 
long val)
char buf[8];
int ret;
 
+   val += arg->index;
+
if (!arg->indirect)
return val;
 
@@ -779,6 +795,8 @@ static int func_event_seq_show(struct seq_file *m, void *v)
last_arg = arg->arg;
comma = true;
seq_printf(m, "%s %s", arg->type, arg->name);
+   if (arg->index)
+   seq_printf(m, "+%ld", arg->index);
if (arg->indirect && arg->size)
seq_printf(m, "[%ld]",
   (arg->indirect ^ INDIRECT_FLAG) / arg->size);
-- 
2.15.1