Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1412669 > unrolled thread

[PATCH v2 0/3] tracing: expose current->comm to [ku]probe events

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/3] tracing: expose current->comm to [ku]probe events Omar Sandoval <osandov@osandov.com> - 2016-06-03 03:20 +0200
    [PATCH v2 1/3] tracing: document "string_size" [ku]probe type Omar Sandoval <osandov@osandov.com> - 2016-06-03 03:20 +0200
      Re: [PATCH v2 1/3] tracing: document "string_size" [ku]probe type Masami Hiramatsu <mhiramat@kernel.org> - 2016-06-07 08:30 +0200
    [PATCH v2 2/3] tracing: expose current->comm to [ku]probe events Omar Sandoval <osandov@osandov.com> - 2016-06-03 03:20 +0200
      Re: [PATCH v2 2/3] tracing: expose current->comm to [ku]probe  events Masami Hiramatsu <mhiramat@kernel.org> - 2016-06-07 09:30 +0200
        Re: [PATCH v2 2/3] tracing: expose current->comm to [ku]probe events Namhyung Kim <namhyung@kernel.org> - 2016-06-07 15:50 +0200
    [PATCH v2 3/3] tracing: make "string" the default type for [ku]probe event $comm Omar Sandoval <osandov@osandov.com> - 2016-06-03 03:20 +0200
      Re: [PATCH v2 3/3] tracing: make "string" the default type for  [ku]probe event $comm Masami Hiramatsu <mhiramat@kernel.org> - 2016-06-07 10:10 +0200
        Re: [PATCH v2 3/3] tracing: make "string" the default type for  [ku]probe event $comm Namhyung Kim <namhyung@kernel.org> - 2016-06-07 15:50 +0200
          Re: [PATCH v2 3/3] tracing: make "string" the default type for  [ku]probe event $comm Steven Rostedt <rostedt@goodmis.org> - 2016-06-08 15:50 +0200

#1412669 — [PATCH v2 0/3] tracing: expose current->comm to [ku]probe events

From: Omar Sandoval <osandov@fb.com>

Hi,

Respinning with a few changes:

- Added patches 1 and 3 (document "string_size" type and make "string"
  default for $comm)
- Also support uprobe events
- Only allow "string" and "string_size" types

Patch 2, which actually introduces $comm, is pretty simple. Patch 3 is
more intrusive, but without it, the default type for $comm is invalid,
which is stupid.

This series applies to v4.7-rc1.

Thanks!

Omar Sandoval (3):
  tracing: document "string_size" [ku]probe type
  tracing: expose current->comm to [ku]probe events
  tracing: make "string" the default type for [ku]probe event $comm

 Documentation/trace/kprobetrace.txt  |  15 +++--
 Documentation/trace/uprobetracer.txt |  13 ++--
 kernel/trace/trace_kprobe.c          |   1 +
 kernel/trace/trace_probe.c           | 123 ++++++++++++++++++++++++++---------
 kernel/trace/trace_probe.h           |  10 +++
 5 files changed, 123 insertions(+), 39 deletions(-)

-- 
2.8.3

[toc] | [next] | [standalone]

#1412670 — [PATCH v2 1/3] tracing: document "string_size" [ku]probe type

From: Omar Sandoval <osandov@fb.com>

It has been undocumented since it was introduced in commit e09c8614b329
("tracing/kprobes: Support "string" type").

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 Documentation/trace/kprobetrace.txt  | 11 ++++++-----
 Documentation/trace/uprobetracer.txt |  9 +++++----
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
index d68ea5fc812b..4b3f5293b7bf 100644
--- a/Documentation/trace/kprobetrace.txt
+++ b/Documentation/trace/kprobetrace.txt
@@ -43,8 +43,8 @@ Synopsis of kprobe_events
   +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
   NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
   FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
-		  (u8/u16/u32/u64/s8/s16/s32/s64), "string" and bitfield
-		  are supported.
+		  (u8/u16/u32/u64/s8/s16/s32/s64), "string", "string_size", and
+		  bitfield are supported.
 
   (*) only for return probe.
   (**) this is useful for fetching a field of data structures.
@@ -54,9 +54,10 @@ Types
 Several types are supported for fetch-args. Kprobe tracer will access memory
 by given type. Prefix 's' and 'u' means those types are signed and unsigned
 respectively. Traced arguments are shown in decimal (signed) or hex (unsigned).
-String type is a special type, which fetches a "null-terminated" string from
-kernel space. This means it will fail and store NULL if the string container
-has been paged out.
+"string" is a special type which fetches a null-terminated string from kernel
+space. This means it will fail and store NULL if the string container has been
+paged out. "string_size" is the length of a null-terminated string including
+the NUL byte.
 Bitfield is another special type, which takes 3 parameters, bit-width, bit-
 offset, and container-size (usually 32). The syntax is;
 
diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
index f1cf9a34ad9d..7e0480263c2f 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -39,8 +39,8 @@ Synopsis of uprobe_tracer
    +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
    NAME=FETCHARG     : Set NAME as the argument name of FETCHARG.
    FETCHARG:TYPE     : Set TYPE as the type of FETCHARG. Currently, basic types
-		       (u8/u16/u32/u64/s8/s16/s32/s64), "string" and bitfield
-		       are supported.
+		       (u8/u16/u32/u64/s8/s16/s32/s64), "string",
+		       "string_size", and bitfield are supported.
 
   (*) only for return probe.
   (**) this is useful for fetching a field of data structures.
@@ -50,8 +50,9 @@ Types
 Several types are supported for fetch-args. Uprobe tracer will access memory
 by given type. Prefix 's' and 'u' means those types are signed and unsigned
 respectively. Traced arguments are shown in decimal (signed) or hex (unsigned).
-String type is a special type, which fetches a "null-terminated" string from
-user space.
+"string" is a special type which fetches a null-terminated string from user
+space. "string_size" is the length of a null-terminated string including the
+NUL byte.
 Bitfield is another special type, which takes 3 parameters, bit-width, bit-
 offset, and container-size (usually 32). The syntax is;
 
-- 
2.8.3

[toc] | [prev] | [next] | [standalone]

#1415739 — Re: [PATCH v2 1/3] tracing: document "string_size" [ku]probe type

On Thu,  2 Jun 2016 18:11:01 -0700
Omar Sandoval <osandov@osandov.com> wrote:

> From: Omar Sandoval <osandov@fb.com>
> 
> It has been undocumented since it was introduced in commit e09c8614b329
> ("tracing/kprobes: Support "string" type").

Ah, actually, string_size is a support fetch function for string.
So please ignore it. Or would you have any idea to use it?

Thank you,

> 
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
>  Documentation/trace/kprobetrace.txt  | 11 ++++++-----
>  Documentation/trace/uprobetracer.txt |  9 +++++----
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
> index d68ea5fc812b..4b3f5293b7bf 100644
> --- a/Documentation/trace/kprobetrace.txt
> +++ b/Documentation/trace/kprobetrace.txt
> @@ -43,8 +43,8 @@ Synopsis of kprobe_events
>    +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
>    NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
>    FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
> -		  (u8/u16/u32/u64/s8/s16/s32/s64), "string" and bitfield
> -		  are supported.
> +		  (u8/u16/u32/u64/s8/s16/s32/s64), "string", "string_size", and
> +		  bitfield are supported.
>  
>    (*) only for return probe.
>    (**) this is useful for fetching a field of data structures.
> @@ -54,9 +54,10 @@ Types
>  Several types are supported for fetch-args. Kprobe tracer will access memory
>  by given type. Prefix 's' and 'u' means those types are signed and unsigned
>  respectively. Traced arguments are shown in decimal (signed) or hex (unsigned).
> -String type is a special type, which fetches a "null-terminated" string from
> -kernel space. This means it will fail and store NULL if the string container
> -has been paged out.
> +"string" is a special type which fetches a null-terminated string from kernel
> +space. This means it will fail and store NULL if the string container has been
> +paged out. "string_size" is the length of a null-terminated string including
> +the NUL byte.
>  Bitfield is another special type, which takes 3 parameters, bit-width, bit-
>  offset, and container-size (usually 32). The syntax is;
>  
> diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
> index f1cf9a34ad9d..7e0480263c2f 100644
> --- a/Documentation/trace/uprobetracer.txt
> +++ b/Documentation/trace/uprobetracer.txt
> @@ -39,8 +39,8 @@ Synopsis of uprobe_tracer
>     +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
>     NAME=FETCHARG     : Set NAME as the argument name of FETCHARG.
>     FETCHARG:TYPE     : Set TYPE as the type of FETCHARG. Currently, basic types
> -		       (u8/u16/u32/u64/s8/s16/s32/s64), "string" and bitfield
> -		       are supported.
> +		       (u8/u16/u32/u64/s8/s16/s32/s64), "string",
> +		       "string_size", and bitfield are supported.
>  
>    (*) only for return probe.
>    (**) this is useful for fetching a field of data structures.
> @@ -50,8 +50,9 @@ Types
>  Several types are supported for fetch-args. Uprobe tracer will access memory
>  by given type. Prefix 's' and 'u' means those types are signed and unsigned
>  respectively. Traced arguments are shown in decimal (signed) or hex (unsigned).
> -String type is a special type, which fetches a "null-terminated" string from
> -user space.
> +"string" is a special type which fetches a null-terminated string from user
> +space. "string_size" is the length of a null-terminated string including the
> +NUL byte.
>  Bitfield is another special type, which takes 3 parameters, bit-width, bit-
>  offset, and container-size (usually 32). The syntax is;
>  
> -- 
> 2.8.3
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1412673 — [PATCH v2 2/3] tracing: expose current->comm to [ku]probe events

From: Omar Sandoval <osandov@fb.com>

ftrace is very quick to give up on saving the task command line (see
`trace_save_cmdline()`). The workaround for events which really care
about the command line is to explicitly assign it as part of the entry.
However, this doesn't work for kprobe events, as there's no
straightforward way to get access to current->comm. Add a kprobe/uprobe
event variable $comm which provides exactly that.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 Documentation/trace/kprobetrace.txt  |  4 ++++
 Documentation/trace/uprobetracer.txt |  4 ++++
 kernel/trace/trace_kprobe.c          |  1 +
 kernel/trace/trace_probe.c           | 27 +++++++++++++++++++++++++++
 kernel/trace/trace_probe.h           | 10 ++++++++++
 5 files changed, 46 insertions(+)

diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
index 4b3f5293b7bf..0bf746656d06 100644
--- a/Documentation/trace/kprobetrace.txt
+++ b/Documentation/trace/kprobetrace.txt
@@ -40,6 +40,7 @@ Synopsis of kprobe_events
   $stackN	: Fetch Nth entry of stack (N >= 0)
   $stack	: Fetch stack address.
   $retval	: Fetch return value.(*)
+  $comm		: Fetch current task comm.(***)
   +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
   NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
   FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
@@ -48,6 +49,7 @@ Synopsis of kprobe_events
 
   (*) only for return probe.
   (**) this is useful for fetching a field of data structures.
+  (***) you probably want this as a string, i.e., $comm:string
 
 Types
 -----
@@ -63,6 +65,8 @@ offset, and container-size (usually 32). The syntax is;
 
  b<bit-width>@<bit-offset>/<container-size>
 
+For $comm, the type must be either "string" or "string_size".
+
 
 Per-Probe Event Filtering
 -------------------------
diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
index 7e0480263c2f..34754da46860 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -36,6 +36,7 @@ Synopsis of uprobe_tracer
    $stackN	: Fetch Nth entry of stack (N >= 0)
    $stack	: Fetch stack address.
    $retval	: Fetch return value.(*)
+   $comm	: Fetch current task comm.(***)
    +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
    NAME=FETCHARG     : Set NAME as the argument name of FETCHARG.
    FETCHARG:TYPE     : Set TYPE as the type of FETCHARG. Currently, basic types
@@ -44,6 +45,7 @@ Synopsis of uprobe_tracer
 
   (*) only for return probe.
   (**) this is useful for fetching a field of data structures.
+  (***) you probably want this as a string, i.e., $comm:string
 
 Types
 -----
@@ -58,6 +60,8 @@ offset, and container-size (usually 32). The syntax is;
 
  b<bit-width>@<bit-offset>/<container-size>
 
+For $comm, the type must be either "string" or "string_size".
+
 
 Event Profiling
 ---------------
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 5546eec0505f..9aedb0b06683 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -587,6 +587,7 @@ static int create_trace_kprobe(int argc, char **argv)
 	 *  $retval	: fetch return value
 	 *  $stack	: fetch stack address
 	 *  $stackN	: fetch Nth of stack (N:0-)
+	 *  $comm       : fetch current task comm
 	 *  @ADDR	: fetch memory at ADDR (ADDR should be in kernel)
 	 *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
 	 *  %REG	: fetch register REG
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 1d372fa6fefb..3900b6e4a05d 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -218,6 +218,28 @@ free_bitfield_fetch_param(struct bitfield_fetch_param *data)
 	kfree(data);
 }
 
+void FETCH_FUNC_NAME(comm, string)(struct pt_regs *regs,
+					  void *data, void *dest)
+{
+	int maxlen = get_rloc_len(*(u32 *)dest);
+	u8 *dst = get_rloc_data(dest);
+	long ret;
+
+	if (!maxlen)
+		return;
+
+	ret = strlcpy(dst, current->comm, maxlen);
+	*(u32 *)dest = make_data_rloc(ret, get_rloc_offs(*(u32 *)dest));
+}
+NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, string));
+
+void FETCH_FUNC_NAME(comm, string_size)(struct pt_regs *regs,
+					       void *data, void *dest)
+{
+	*(u32 *)dest = strlen(current->comm) + 1;
+}
+NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, string_size));
+
 static const struct fetch_type *find_fetch_type(const char *type,
 						const struct fetch_type *ftbl)
 {
@@ -348,6 +370,11 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
 			}
 		} else
 			ret = -EINVAL;
+	} else if (strcmp(arg, "comm") == 0) {
+		if (strcmp(t->name, "string") != 0 &&
+		    strcmp(t->name, "string_size") != 0)
+			return -EINVAL;
+		f->fn = t->fetch[FETCH_MTD_comm];
 	} else
 		ret = -EINVAL;
 
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index f6398db09114..45400ca5ded1 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -102,6 +102,7 @@ enum {
 	FETCH_MTD_reg = 0,
 	FETCH_MTD_stack,
 	FETCH_MTD_retval,
+	FETCH_MTD_comm,
 	FETCH_MTD_memory,
 	FETCH_MTD_symbol,
 	FETCH_MTD_deref,
@@ -183,6 +184,14 @@ DECLARE_BASIC_FETCH_FUNCS(bitfield);
 #define fetch_bitfield_string			NULL
 #define fetch_bitfield_string_size		NULL
 
+/* comm only makes sense as a string */
+#define fetch_comm_u8		NULL
+#define fetch_comm_u16		NULL
+#define fetch_comm_u32		NULL
+#define fetch_comm_u64		NULL
+DECLARE_FETCH_FUNC(comm, string);
+DECLARE_FETCH_FUNC(comm, string_size);
+
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -213,6 +222,7 @@ DEFINE_FETCH_##method(u64)
 ASSIGN_FETCH_FUNC(reg, ftype),				\
 ASSIGN_FETCH_FUNC(stack, ftype),			\
 ASSIGN_FETCH_FUNC(retval, ftype),			\
+ASSIGN_FETCH_FUNC(comm, ftype),				\
 ASSIGN_FETCH_FUNC(memory, ftype),			\
 ASSIGN_FETCH_FUNC(symbol, ftype),			\
 ASSIGN_FETCH_FUNC(deref, ftype),			\
-- 
2.8.3

[toc] | [prev] | [next] | [standalone]

#1415784 — Re: [PATCH v2 2/3] tracing: expose current->comm to [ku]probe events

On Thu,  2 Jun 2016 18:11:02 -0700
Omar Sandoval <osandov@osandov.com> wrote:

> From: Omar Sandoval <osandov@fb.com>
> 
> ftrace is very quick to give up on saving the task command line (see
> `trace_save_cmdline()`). The workaround for events which really care
> about the command line is to explicitly assign it as part of the entry.
> However, this doesn't work for kprobe events, as there's no
> straightforward way to get access to current->comm. Add a kprobe/uprobe
> event variable $comm which provides exactly that.
> 
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
>  Documentation/trace/kprobetrace.txt  |  4 ++++
>  Documentation/trace/uprobetracer.txt |  4 ++++
>  kernel/trace/trace_kprobe.c          |  1 +
>  kernel/trace/trace_probe.c           | 27 +++++++++++++++++++++++++++
>  kernel/trace/trace_probe.h           | 10 ++++++++++
>  5 files changed, 46 insertions(+)
> 
> diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
> index 4b3f5293b7bf..0bf746656d06 100644
> --- a/Documentation/trace/kprobetrace.txt
> +++ b/Documentation/trace/kprobetrace.txt
> @@ -40,6 +40,7 @@ Synopsis of kprobe_events
>    $stackN	: Fetch Nth entry of stack (N >= 0)
>    $stack	: Fetch stack address.
>    $retval	: Fetch return value.(*)
> +  $comm		: Fetch current task comm.(***)
>    +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
>    NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
>    FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
> @@ -48,6 +49,7 @@ Synopsis of kprobe_events
>  
>    (*) only for return probe.
>    (**) this is useful for fetching a field of data structures.
> +  (***) you probably want this as a string, i.e., $comm:string
>  
>  Types
>  -----
> @@ -63,6 +65,8 @@ offset, and container-size (usually 32). The syntax is;
>  
>   b<bit-width>@<bit-offset>/<container-size>
>  
> +For $comm, the type must be either "string" or "string_size".

Please remove string_size, which is an internal type.

> +
>  
>  Per-Probe Event Filtering
>  -------------------------
> diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
> index 7e0480263c2f..34754da46860 100644
> --- a/Documentation/trace/uprobetracer.txt
> +++ b/Documentation/trace/uprobetracer.txt
> @@ -36,6 +36,7 @@ Synopsis of uprobe_tracer
>     $stackN	: Fetch Nth entry of stack (N >= 0)
>     $stack	: Fetch stack address.
>     $retval	: Fetch return value.(*)
> +   $comm	: Fetch current task comm.(***)
>     +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
>     NAME=FETCHARG     : Set NAME as the argument name of FETCHARG.
>     FETCHARG:TYPE     : Set TYPE as the type of FETCHARG. Currently, basic types
> @@ -44,6 +45,7 @@ Synopsis of uprobe_tracer
>  
>    (*) only for return probe.
>    (**) this is useful for fetching a field of data structures.
> +  (***) you probably want this as a string, i.e., $comm:string

Ditto.

Other part looks OK for me :)

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,

>  
>  Types
>  -----
> @@ -58,6 +60,8 @@ offset, and container-size (usually 32). The syntax is;
>  
>   b<bit-width>@<bit-offset>/<container-size>
>  
> +For $comm, the type must be either "string" or "string_size".
> +
>  
>  Event Profiling
>  ---------------
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5546eec0505f..9aedb0b06683 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -587,6 +587,7 @@ static int create_trace_kprobe(int argc, char **argv)
>  	 *  $retval	: fetch return value
>  	 *  $stack	: fetch stack address
>  	 *  $stackN	: fetch Nth of stack (N:0-)
> +	 *  $comm       : fetch current task comm
>  	 *  @ADDR	: fetch memory at ADDR (ADDR should be in kernel)
>  	 *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
>  	 *  %REG	: fetch register REG
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 1d372fa6fefb..3900b6e4a05d 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -218,6 +218,28 @@ free_bitfield_fetch_param(struct bitfield_fetch_param *data)
>  	kfree(data);
>  }
>  
> +void FETCH_FUNC_NAME(comm, string)(struct pt_regs *regs,
> +					  void *data, void *dest)
> +{
> +	int maxlen = get_rloc_len(*(u32 *)dest);
> +	u8 *dst = get_rloc_data(dest);
> +	long ret;
> +
> +	if (!maxlen)
> +		return;
> +
> +	ret = strlcpy(dst, current->comm, maxlen);
> +	*(u32 *)dest = make_data_rloc(ret, get_rloc_offs(*(u32 *)dest));
> +}
> +NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, string));
> +
> +void FETCH_FUNC_NAME(comm, string_size)(struct pt_regs *regs,
> +					       void *data, void *dest)
> +{
> +	*(u32 *)dest = strlen(current->comm) + 1;
> +}
> +NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, string_size));
> +
>  static const struct fetch_type *find_fetch_type(const char *type,
>  						const struct fetch_type *ftbl)
>  {
> @@ -348,6 +370,11 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
>  			}
>  		} else
>  			ret = -EINVAL;
> +	} else if (strcmp(arg, "comm") == 0) {
> +		if (strcmp(t->name, "string") != 0 &&
> +		    strcmp(t->name, "string_size") != 0)
> +			return -EINVAL;
> +		f->fn = t->fetch[FETCH_MTD_comm];
>  	} else
>  		ret = -EINVAL;
>  
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index f6398db09114..45400ca5ded1 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -102,6 +102,7 @@ enum {
>  	FETCH_MTD_reg = 0,
>  	FETCH_MTD_stack,
>  	FETCH_MTD_retval,
> +	FETCH_MTD_comm,
>  	FETCH_MTD_memory,
>  	FETCH_MTD_symbol,
>  	FETCH_MTD_deref,
> @@ -183,6 +184,14 @@ DECLARE_BASIC_FETCH_FUNCS(bitfield);
>  #define fetch_bitfield_string			NULL
>  #define fetch_bitfield_string_size		NULL
>  
> +/* comm only makes sense as a string */
> +#define fetch_comm_u8		NULL
> +#define fetch_comm_u16		NULL
> +#define fetch_comm_u32		NULL
> +#define fetch_comm_u64		NULL
> +DECLARE_FETCH_FUNC(comm, string);
> +DECLARE_FETCH_FUNC(comm, string_size);
> +
>  /*
>   * Define macro for basic types - we don't need to define s* types, because
>   * we have to care only about bitwidth at recording time.
> @@ -213,6 +222,7 @@ DEFINE_FETCH_##method(u64)
>  ASSIGN_FETCH_FUNC(reg, ftype),				\
>  ASSIGN_FETCH_FUNC(stack, ftype),			\
>  ASSIGN_FETCH_FUNC(retval, ftype),			\
> +ASSIGN_FETCH_FUNC(comm, ftype),				\
>  ASSIGN_FETCH_FUNC(memory, ftype),			\
>  ASSIGN_FETCH_FUNC(symbol, ftype),			\
>  ASSIGN_FETCH_FUNC(deref, ftype),			\
> -- 
> 2.8.3
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1416202 — Re: [PATCH v2 2/3] tracing: expose current->comm to [ku]probe events

Hello,

On Tue, Jun 07, 2016 at 04:24:23PM +0900, Masami Hiramatsu wrote:
> On Thu,  2 Jun 2016 18:11:02 -0700
> Omar Sandoval <osandov@osandov.com> wrote:
> 
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > ftrace is very quick to give up on saving the task command line (see
> > `trace_save_cmdline()`). The workaround for events which really care
> > about the command line is to explicitly assign it as part of the entry.
> > However, this doesn't work for kprobe events, as there's no
> > straightforward way to get access to current->comm. Add a kprobe/uprobe
> > event variable $comm which provides exactly that.
> > 

Same opinion with Masami; The string_size itself seems not useful IMHO.
Also you can increase 'saved_cmdlines_size' in the tracefs.

Thanks,
Namhyung


> > Signed-off-by: Omar Sandoval <osandov@fb.com>
> > ---
> >  Documentation/trace/kprobetrace.txt  |  4 ++++
> >  Documentation/trace/uprobetracer.txt |  4 ++++
> >  kernel/trace/trace_kprobe.c          |  1 +
> >  kernel/trace/trace_probe.c           | 27 +++++++++++++++++++++++++++
> >  kernel/trace/trace_probe.h           | 10 ++++++++++
> >  5 files changed, 46 insertions(+)
> > 
> > diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
> > index 4b3f5293b7bf..0bf746656d06 100644
> > --- a/Documentation/trace/kprobetrace.txt
> > +++ b/Documentation/trace/kprobetrace.txt
> > @@ -40,6 +40,7 @@ Synopsis of kprobe_events
> >    $stackN	: Fetch Nth entry of stack (N >= 0)
> >    $stack	: Fetch stack address.
> >    $retval	: Fetch return value.(*)
> > +  $comm		: Fetch current task comm.(***)
> >    +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
> >    NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
> >    FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
> > @@ -48,6 +49,7 @@ Synopsis of kprobe_events
> >  
> >    (*) only for return probe.
> >    (**) this is useful for fetching a field of data structures.
> > +  (***) you probably want this as a string, i.e., $comm:string
> >  
> >  Types
> >  -----
> > @@ -63,6 +65,8 @@ offset, and container-size (usually 32). The syntax is;
> >  
> >   b<bit-width>@<bit-offset>/<container-size>
> >  
> > +For $comm, the type must be either "string" or "string_size".
> 
> Please remove string_size, which is an internal type.

> 
> > +
> >  
> >  Per-Probe Event Filtering
> >  -------------------------
> > diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
> > index 7e0480263c2f..34754da46860 100644
> > --- a/Documentation/trace/uprobetracer.txt
> > +++ b/Documentation/trace/uprobetracer.txt
> > @@ -36,6 +36,7 @@ Synopsis of uprobe_tracer
> >     $stackN	: Fetch Nth entry of stack (N >= 0)
> >     $stack	: Fetch stack address.
> >     $retval	: Fetch return value.(*)
> > +   $comm	: Fetch current task comm.(***)
> >     +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
> >     NAME=FETCHARG     : Set NAME as the argument name of FETCHARG.
> >     FETCHARG:TYPE     : Set TYPE as the type of FETCHARG. Currently, basic types
> > @@ -44,6 +45,7 @@ Synopsis of uprobe_tracer
> >  
> >    (*) only for return probe.
> >    (**) this is useful for fetching a field of data structures.
> > +  (***) you probably want this as a string, i.e., $comm:string
> 
> Ditto.
> 
> Other part looks OK for me :)
> 
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> 
> >  
> >  Types
> >  -----
> > @@ -58,6 +60,8 @@ offset, and container-size (usually 32). The syntax is;
> >  
> >   b<bit-width>@<bit-offset>/<container-size>
> >  
> > +For $comm, the type must be either "string" or "string_size".
> > +
> >  
> >  Event Profiling
> >  ---------------
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 5546eec0505f..9aedb0b06683 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -587,6 +587,7 @@ static int create_trace_kprobe(int argc, char **argv)
> >  	 *  $retval	: fetch return value
> >  	 *  $stack	: fetch stack address
> >  	 *  $stackN	: fetch Nth of stack (N:0-)
> > +	 *  $comm       : fetch current task comm
> >  	 *  @ADDR	: fetch memory at ADDR (ADDR should be in kernel)
> >  	 *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
> >  	 *  %REG	: fetch register REG
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index 1d372fa6fefb..3900b6e4a05d 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -218,6 +218,28 @@ free_bitfield_fetch_param(struct bitfield_fetch_param *data)
> >  	kfree(data);
> >  }
> >  
> > +void FETCH_FUNC_NAME(comm, string)(struct pt_regs *regs,
> > +					  void *data, void *dest)
> > +{
> > +	int maxlen = get_rloc_len(*(u32 *)dest);
> > +	u8 *dst = get_rloc_data(dest);
> > +	long ret;
> > +
> > +	if (!maxlen)
> > +		return;
> > +
> > +	ret = strlcpy(dst, current->comm, maxlen);
> > +	*(u32 *)dest = make_data_rloc(ret, get_rloc_offs(*(u32 *)dest));
> > +}
> > +NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, string));
> > +
> > +void FETCH_FUNC_NAME(comm, string_size)(struct pt_regs *regs,
> > +					       void *data, void *dest)
> > +{
> > +	*(u32 *)dest = strlen(current->comm) + 1;
> > +}
> > +NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, string_size));
> > +
> >  static const struct fetch_type *find_fetch_type(const char *type,
> >  						const struct fetch_type *ftbl)
> >  {
> > @@ -348,6 +370,11 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
> >  			}
> >  		} else
> >  			ret = -EINVAL;
> > +	} else if (strcmp(arg, "comm") == 0) {
> > +		if (strcmp(t->name, "string") != 0 &&
> > +		    strcmp(t->name, "string_size") != 0)
> > +			return -EINVAL;
> > +		f->fn = t->fetch[FETCH_MTD_comm];
> >  	} else
> >  		ret = -EINVAL;
> >  
> > diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> > index f6398db09114..45400ca5ded1 100644
> > --- a/kernel/trace/trace_probe.h
> > +++ b/kernel/trace/trace_probe.h
> > @@ -102,6 +102,7 @@ enum {
> >  	FETCH_MTD_reg = 0,
> >  	FETCH_MTD_stack,
> >  	FETCH_MTD_retval,
> > +	FETCH_MTD_comm,
> >  	FETCH_MTD_memory,
> >  	FETCH_MTD_symbol,
> >  	FETCH_MTD_deref,
> > @@ -183,6 +184,14 @@ DECLARE_BASIC_FETCH_FUNCS(bitfield);
> >  #define fetch_bitfield_string			NULL
> >  #define fetch_bitfield_string_size		NULL
> >  
> > +/* comm only makes sense as a string */
> > +#define fetch_comm_u8		NULL
> > +#define fetch_comm_u16		NULL
> > +#define fetch_comm_u32		NULL
> > +#define fetch_comm_u64		NULL
> > +DECLARE_FETCH_FUNC(comm, string);
> > +DECLARE_FETCH_FUNC(comm, string_size);
> > +
> >  /*
> >   * Define macro for basic types - we don't need to define s* types, because
> >   * we have to care only about bitwidth at recording time.
> > @@ -213,6 +222,7 @@ DEFINE_FETCH_##method(u64)
> >  ASSIGN_FETCH_FUNC(reg, ftype),				\
> >  ASSIGN_FETCH_FUNC(stack, ftype),			\
> >  ASSIGN_FETCH_FUNC(retval, ftype),			\
> > +ASSIGN_FETCH_FUNC(comm, ftype),				\
> >  ASSIGN_FETCH_FUNC(memory, ftype),			\
> >  ASSIGN_FETCH_FUNC(symbol, ftype),			\
> >  ASSIGN_FETCH_FUNC(deref, ftype),			\
> > -- 
> > 2.8.3
> > 
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1412674 — [PATCH v2 3/3] tracing: make "string" the default type for [ku]probe event $comm

From: Omar Sandoval <osandov@fb.com>

You'd only ever want $comm as a string, but the default is still u64.
Push the type parsing later so we can decide based on the actual
fetcharg and make "string" the default for $comm.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 Documentation/trace/kprobetrace.txt  |   6 +--
 Documentation/trace/uprobetracer.txt |   6 +--
 kernel/trace/trace_probe.c           | 102 +++++++++++++++++++++++------------
 3 files changed, 75 insertions(+), 39 deletions(-)

diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
index 0bf746656d06..da3b437d4e5e 100644
--- a/Documentation/trace/kprobetrace.txt
+++ b/Documentation/trace/kprobetrace.txt
@@ -40,7 +40,7 @@ Synopsis of kprobe_events
   $stackN	: Fetch Nth entry of stack (N >= 0)
   $stack	: Fetch stack address.
   $retval	: Fetch return value.(*)
-  $comm		: Fetch current task comm.(***)
+  $comm		: Fetch current task comm.
   +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
   NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
   FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
@@ -49,7 +49,6 @@ Synopsis of kprobe_events
 
   (*) only for return probe.
   (**) this is useful for fetching a field of data structures.
-  (***) you probably want this as a string, i.e., $comm:string
 
 Types
 -----
@@ -65,7 +64,8 @@ offset, and container-size (usually 32). The syntax is;
 
  b<bit-width>@<bit-offset>/<container-size>
 
-For $comm, the type must be either "string" or "string_size".
+For $comm, the type must be either "string" or "string_size". The default is
+"string".
 
 
 Per-Probe Event Filtering
diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
index 34754da46860..5daa61cf431a 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -36,7 +36,7 @@ Synopsis of uprobe_tracer
    $stackN	: Fetch Nth entry of stack (N >= 0)
    $stack	: Fetch stack address.
    $retval	: Fetch return value.(*)
-   $comm	: Fetch current task comm.(***)
+   $comm	: Fetch current task comm.
    +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
    NAME=FETCHARG     : Set NAME as the argument name of FETCHARG.
    FETCHARG:TYPE     : Set TYPE as the type of FETCHARG. Currently, basic types
@@ -45,7 +45,6 @@ Synopsis of uprobe_tracer
 
   (*) only for return probe.
   (**) this is useful for fetching a field of data structures.
-  (***) you probably want this as a string, i.e., $comm:string
 
 Types
 -----
@@ -60,7 +59,8 @@ offset, and container-size (usually 32). The syntax is;
 
  b<bit-width>@<bit-offset>/<container-size>
 
-For $comm, the type must be either "string" or "string_size".
+For $comm, the type must be either "string" or "string_size". The default is
+"string".
 
 
 Event Profiling
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 3900b6e4a05d..6b0a553308f9 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -296,6 +296,26 @@ static void fetch_user_stack_address(struct pt_regs *regs, void *dummy, void *de
 }
 NOKPROBE_SYMBOL(fetch_user_stack_address);
 
+static int parse_fetch_type(const char *t, ssize_t *size,
+			    const struct fetch_type **tp,
+			    const struct fetch_type *ftbl,
+			    const char *default_t)
+{
+	if (!t)
+		t = default_t;
+
+	*tp = find_fetch_type(t, ftbl);
+	if (!*tp) {
+		pr_info("Unsupported type: %s\n", t);
+		return -EINVAL;
+	}
+
+	if (size)
+		*size += (*tp)->size;
+
+	return 0;
+}
+
 static fetch_func_t get_fetch_size_function(const struct fetch_type *type,
 					    fetch_func_t orig_fn,
 					    const struct fetch_type *ftbl)
@@ -339,21 +359,28 @@ int traceprobe_split_symbol_offset(char *symbol, unsigned long *offset)
 
 #define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long))
 
-static int parse_probe_vars(char *arg, const struct fetch_type *t,
-			    struct fetch_param *f, bool is_return,
-			    bool is_kprobe)
+static int parse_probe_vars(char *arg, const char *t, ssize_t *size,
+			    const struct fetch_type **tp, struct fetch_param *f,
+			    bool is_return, bool is_kprobe,
+			    const struct fetch_type *ftbl)
 {
 	int ret = 0;
 	unsigned long param;
 
 	if (strcmp(arg, "retval") == 0) {
+		ret = parse_fetch_type(t, size, tp, ftbl, NULL);
+		if (ret)
+			return ret;
 		if (is_return)
-			f->fn = t->fetch[FETCH_MTD_retval];
+			f->fn = (*tp)->fetch[FETCH_MTD_retval];
 		else
 			ret = -EINVAL;
 	} else if (strncmp(arg, "stack", 5) == 0) {
+		ret = parse_fetch_type(t, size, tp, ftbl, NULL);
+		if (ret)
+			return ret;
 		if (arg[5] == '\0') {
-			if (strcmp(t->name, DEFAULT_FETCH_TYPE_STR))
+			if (strcmp((*tp)->name, DEFAULT_FETCH_TYPE_STR))
 				return -EINVAL;
 
 			if (is_kprobe)
@@ -365,16 +392,19 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
 			if (ret || (is_kprobe && param > PARAM_MAX_STACK))
 				ret = -EINVAL;
 			else {
-				f->fn = t->fetch[FETCH_MTD_stack];
+				f->fn = (*tp)->fetch[FETCH_MTD_stack];
 				f->data = (void *)param;
 			}
 		} else
 			ret = -EINVAL;
 	} else if (strcmp(arg, "comm") == 0) {
-		if (strcmp(t->name, "string") != 0 &&
-		    strcmp(t->name, "string_size") != 0)
+		ret = parse_fetch_type(t, size, tp, ftbl, "string");
+		if (ret)
+			return ret;
+		if (strcmp((*tp)->name, "string") != 0 &&
+		    strcmp((*tp)->name, "string_size") != 0)
 			return -EINVAL;
-		f->fn = t->fetch[FETCH_MTD_comm];
+		f->fn = (*tp)->fetch[FETCH_MTD_comm];
 	} else
 		ret = -EINVAL;
 
@@ -382,9 +412,10 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
 }
 
 /* Recursive argument parser */
-static int parse_probe_arg(char *arg, const struct fetch_type *t,
-		     struct fetch_param *f, bool is_return, bool is_kprobe,
-		     const struct fetch_type *ftbl)
+static int parse_probe_arg(char *arg, const char *t, ssize_t *size,
+			   const struct fetch_type **tp,
+			   struct fetch_param *f, bool is_return,
+			   bool is_kprobe, const struct fetch_type *ftbl)
 {
 	unsigned long param;
 	long offset;
@@ -393,25 +424,32 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
 
 	switch (arg[0]) {
 	case '$':
-		ret = parse_probe_vars(arg + 1, t, f, is_return, is_kprobe);
+		ret = parse_probe_vars(arg + 1, t, size, tp, f, is_return,
+				       is_kprobe, ftbl);
 		break;
 
 	case '%':	/* named register */
+		ret = parse_fetch_type(t, size, tp, ftbl, NULL);
+		if (ret)
+			break;
 		ret = regs_query_register_offset(arg + 1);
 		if (ret >= 0) {
-			f->fn = t->fetch[FETCH_MTD_reg];
+			f->fn = (*tp)->fetch[FETCH_MTD_reg];
 			f->data = (void *)(unsigned long)ret;
 			ret = 0;
 		}
 		break;
 
 	case '@':	/* memory, file-offset or symbol */
+		ret = parse_fetch_type(t, size, tp, ftbl, NULL);
+		if (ret)
+			break;
 		if (isdigit(arg[1])) {
 			ret = kstrtoul(arg + 1, 0, &param);
 			if (ret)
 				break;
 
-			f->fn = t->fetch[FETCH_MTD_memory];
+			f->fn = (*tp)->fetch[FETCH_MTD_memory];
 			f->data = (void *)param;
 		} else if (arg[1] == '+') {
 			/* kprobes don't support file offsets */
@@ -422,7 +460,7 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
 			if (ret)
 				break;
 
-			f->fn = t->fetch[FETCH_MTD_file_offset];
+			f->fn = (*tp)->fetch[FETCH_MTD_file_offset];
 			f->data = (void *)offset;
 		} else {
 			/* uprobes don't support symbols */
@@ -435,13 +473,16 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
 
 			f->data = alloc_symbol_cache(arg + 1, offset);
 			if (f->data)
-				f->fn = t->fetch[FETCH_MTD_symbol];
+				f->fn = (*tp)->fetch[FETCH_MTD_symbol];
 		}
 		break;
 
 	case '+':	/* deref memory */
 		arg++;	/* Skip '+', because kstrtol() rejects it. */
 	case '-':
+		ret = parse_fetch_type(t, size, tp, ftbl, NULL);
+		if (ret)
+			break;
 		tmp = strchr(arg, '(');
 		if (!tmp)
 			break;
@@ -459,7 +500,6 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
 			struct deref_fetch_param	*dprm;
 			const struct fetch_type		*t2;
 
-			t2 = find_fetch_type(NULL, ftbl);
 			*tmp = '\0';
 			dprm = kzalloc(sizeof(struct deref_fetch_param), GFP_KERNEL);
 
@@ -467,22 +507,24 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
 				return -ENOMEM;
 
 			dprm->offset = offset;
-			dprm->fetch = t->fetch[FETCH_MTD_memory];
-			dprm->fetch_size = get_fetch_size_function(t,
-							dprm->fetch, ftbl);
-			ret = parse_probe_arg(arg, t2, &dprm->orig, is_return,
-							is_kprobe, ftbl);
+			dprm->fetch = (*tp)->fetch[FETCH_MTD_memory];
+			dprm->fetch_size = get_fetch_size_function((*tp),
+								   dprm->fetch,
+								   ftbl);
+			ret = parse_probe_arg(arg, NULL, NULL, &t2, &dprm->orig,
+					      is_return, is_kprobe, ftbl);
 			if (ret)
 				kfree(dprm);
 			else {
-				f->fn = t->fetch[FETCH_MTD_deref];
+				f->fn = (*tp)->fetch[FETCH_MTD_deref];
 				f->data = (void *)dprm;
 			}
 		}
 		break;
 	}
 	if (!ret && !f->fn) {	/* Parsed, but do not find fetch method */
-		pr_info("%s type has no corresponding fetch method.\n", t->name);
+		pr_info("%s type has no corresponding fetch method.\n",
+			(*tp)->name);
 		ret = -EINVAL;
 	}
 
@@ -549,15 +591,9 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
 		arg[t - parg->comm] = '\0';
 		t++;
 	}
-	parg->type = find_fetch_type(t, ftbl);
-	if (!parg->type) {
-		pr_info("Unsupported type: %s\n", t);
-		return -EINVAL;
-	}
 	parg->offset = *size;
-	*size += parg->type->size;
-	ret = parse_probe_arg(arg, parg->type, &parg->fetch, is_return,
-			      is_kprobe, ftbl);
+	ret = parse_probe_arg(arg, t, size, &parg->type, &parg->fetch,
+			      is_return, is_kprobe, ftbl);
 
 	if (ret >= 0 && t != NULL)
 		ret = __parse_bitfield_probe_arg(t, parg->type, &parg->fetch);
-- 
2.8.3

[toc] | [prev] | [next] | [standalone]

#1415821 — Re: [PATCH v2 3/3] tracing: make "string" the default type for [ku]probe event $comm

On Thu,  2 Jun 2016 18:11:03 -0700
Omar Sandoval <osandov@osandov.com> wrote:

> From: Omar Sandoval <osandov@fb.com>
> 
> You'd only ever want $comm as a string, but the default is still u64.
> Push the type parsing later so we can decide based on the actual
> fetcharg and make "string" the default for $comm.

Hmm, at this moment, I'd rather like a small hack in 
traceprobe_parse_probe_arg() as like below:

--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -522,6 +522,12 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
                arg[t - parg->comm] = '\0';
                t++;
        }
+       /*
+        * The default type of "$comm" should be a string, and it must not
+        * be dereferred
+        */
+       if (!t && strcmp("$comm", arg))
+               t = "string";
        parg->type = find_fetch_type(t, ftbl);
        if (!parg->type) {
                pr_info("Unsupported type: %s\n", t);

This is not general, but easy to review :)

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1416206 — Re: [PATCH v2 3/3] tracing: make "string" the default type for [ku]probe event $comm

On Tue, Jun 07, 2016 at 05:02:32PM +0900, Masami Hiramatsu wrote:
> On Thu,  2 Jun 2016 18:11:03 -0700
> Omar Sandoval <osandov@osandov.com> wrote:
> 
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > You'd only ever want $comm as a string, but the default is still u64.
> > Push the type parsing later so we can decide based on the actual
> > fetcharg and make "string" the default for $comm.
> 
> Hmm, at this moment, I'd rather like a small hack in 
> traceprobe_parse_probe_arg() as like below:
> 
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -522,6 +522,12 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
>                 arg[t - parg->comm] = '\0';
>                 t++;
>         }
> +       /*
> +        * The default type of "$comm" should be a string, and it must not
> +        * be dereferred
> +        */
> +       if (!t && strcmp("$comm", arg))
> +               t = "string";
>         parg->type = find_fetch_type(t, ftbl);
>         if (!parg->type) {
>                 pr_info("Unsupported type: %s\n", t);
> 
> This is not general, but easy to review :)

+1


Thanks,
Namhyung

[toc] | [prev] | [next] | [standalone]

#1417457 — Re: [PATCH v2 3/3] tracing: make "string" the default type for [ku]probe event $comm

On Tue, 7 Jun 2016 22:41:33 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> > This is not general, but easy to review :)  
> 
> +1

I'll wait for v3 then :-)

-- Steve

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web