[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20160609115534.087b15a22dbd97535c8a0939@kernel.org>
Date: Thu, 9 Jun 2016 11:55:34 +0900
From: Masami Hiramatsu <mhiramat@...nel.org>
To: Omar Sandoval <osandov@...ndov.com>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Ingo Molnar <mingo@...hat.com>,
Namhyung Kim <namhyung@...il.com>,
linux-kernel@...r.kernel.org, kernel-team@...com,
Alexei Starovoitov <ast@...com>
Subject: Re: [PATCH v3] tracing: expose current->comm to [ku]probe events
On Wed, 8 Jun 2016 18:38:02 -0700
Omar Sandoval <osandov@...ndov.com> wrote:
> From: Omar Sandoval <osandov@...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@...com>
Looks good to me:)
Acked-by: Masami Hiramatsu <mhiramat@...nel.org>
> ---
> Alexei pointed out to me that this same functionality can be acheived
> with BPF and the bpf_get_current_comm() helper, so strictly speaking,
> this patch isn't necessary. It's still convenient for ad-hoc stuff,
> though, so I figured I'd throw this up and see what everyone thought.
Anyway, I'd like to pull this patch since this still be useful
in handy way to get comm on ftrace :)
Thank you!
>
> Documentation/trace/kprobetrace.txt | 3 +++
> Documentation/trace/uprobetracer.txt | 3 +++
> kernel/trace/trace_kprobe.c | 1 +
> kernel/trace/trace_probe.c | 33 +++++++++++++++++++++++++++++++++
> kernel/trace/trace_probe.h | 10 ++++++++++
> 5 files changed, 50 insertions(+)
>
> diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
> index d68ea5fc812b..ea52ec1f8484 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
> @@ -62,6 +63,8 @@ offset, and container-size (usually 32). The syntax is;
>
> b<bit-width>@<bit-offset>/<container-size>
>
> +For $comm, the default type is "string"; any other type is invalid.
> +
>
> Per-Probe Event Filtering
> -------------------------
> diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
> index f1cf9a34ad9d..72d1cd4f7bf3 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
> @@ -57,6 +58,8 @@ offset, and container-size (usually 32). The syntax is;
>
> b<bit-width>@<bit-offset>/<container-size>
>
> +For $comm, the default type is "string"; any other type is invalid.
> +
>
> 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..74e80a582c28 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;
>
> @@ -522,6 +549,12 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
> arg[t - parg->comm] = '\0';
> t++;
> }
> + /*
> + * The default type of $comm should be "string", and it can't be
> + * dereferenced.
> + */
> + if (!t && strcmp(arg, "$comm") == 0)
> + t = "string";
> parg->type = find_fetch_type(t, ftbl);
> if (!parg->type) {
> pr_info("Unsupported type: %s\n", t);
> 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@...nel.org>
Powered by blists - more mailing lists