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

> From: Omar Sandoval <osan...@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 <osan...@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 <mhira...@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 <mhira...@kernel.org>

Reply via email to