[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20180222173548.8beb16060de396491078a3eb@kernel.org>
Date: Thu, 22 Feb 2018 17:35:48 +0900
From: Masami Hiramatsu <mhiramat@...nel.org>
To: Masami Hiramatsu <mhiramat@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>, linux-kernel@...r.kernel.org,
Ingo Molnar <mingo@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Tom Zanussi <tom.zanussi@...ux.intel.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
linux-trace-users@...r.kernel.org, linux-kselftest@...r.kernel.org,
shuah@...nel.org
Subject: Re: [PATCH v2 13/17] tracing: probeevent: Add $argN for accessing
function args
On Thu, 22 Feb 2018 00:00:31 +0900
Masami Hiramatsu <mhiramat@...nel.org> wrote:
> Add $argN special fetch variable for accessing function
> arguments. This allows user to trace the Nth argument easily
> at the function entry.
>
> Note that this returns most probably assignment of registers
> and stacks. In some case, it may not work well. If you need
> to access correct registers or stacks you should use perf-probe.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@...nel.org>
> ---
> Changes in v2:
> - Add $argN in README file
> - Make N start from 1 as same as auto-generate event argument
> names.
> ---
> Documentation/trace/kprobetrace.txt | 10 ++++++----
> kernel/trace/trace.c | 2 +-
> kernel/trace/trace_kprobe.c | 18 +++++++++++++-----
> kernel/trace/trace_probe.c | 36 ++++++++++++++++++++++-------------
> kernel/trace/trace_probe.h | 9 ++++++++-
> kernel/trace/trace_uprobe.c | 2 +-
> 6 files changed, 52 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
> index d49381f2e411..1d082f8ffeee 100644
> --- a/Documentation/trace/kprobetrace.txt
> +++ b/Documentation/trace/kprobetrace.txt
> @@ -43,16 +43,18 @@ Synopsis of kprobe_events
> @SYM[+|-offs] : Fetch memory at SYM +|- offs (SYM should be a data symbol)
> $stackN : Fetch Nth entry of stack (N >= 0)
> $stack : Fetch stack address.
> - $retval : Fetch return value.(*)
> + $argN : Fetch the Nth function argument. (N >= 1) (*1)
> + $retval : Fetch return value.(*2)
> $comm : Fetch current task comm.
> - +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
> + +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(*3)
> 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), hexadecimal types
> (x8/x16/x32/x64), "string" and bitfield are supported.
>
> - (*) only for return probe.
> - (**) this is useful for fetching a field of data structures.
> + (*1) only for the probe on function entry (offs == 0).
> + (*2) only for return probe.
> + (*3) this is useful for fetching a field of data structures.
>
> Types
> -----
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8f08811d15b8..94423529b986 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -4608,7 +4608,7 @@ static const char readme_msg[] =
> #endif
> "\t args: <name>=fetcharg[:type]\n"
> "\t fetcharg: %<register>, @<address>, @<symbol>[+|-<offset>],\n"
> - "\t $stack<index>, $stack, $retval, $comm\n"
> + "\t $stack<index>, $stack, $retval, $comm, $arg<N>\n"
Oops, this should depend on CONFIG_HAVE_FUNCTION_ARG_ACCESS_API, or
testcase can not detect the kernel supports argN or not.
Thanks,
> "\t type: s8/16/32/64, u8/16/32/64, x8/16/32/64, string, symbol,\n"
> "\t b<bit-width>@<bit-offset>/<container-size>\n"
> #endif
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5025907e0c99..df39c7d5dd4c 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -490,13 +490,15 @@ static int create_trace_kprobe(int argc, char **argv)
> unsigned long offset = 0;
> void *addr = NULL;
> char buf[MAX_EVENT_NAME_LEN];
> + unsigned int flags = TPARG_FL_KERNEL;
>
> /* argc must be >= 1 */
> if (argv[0][0] == 'p')
> is_return = false;
> - else if (argv[0][0] == 'r')
> + else if (argv[0][0] == 'r') {
> is_return = true;
> - else if (argv[0][0] == '-')
> + flags |= TPARG_FL_RETURN;
> + } else if (argv[0][0] == '-')
> is_delete = true;
> else {
> pr_info("Probe definition must be started with 'p', 'r' or"
> @@ -579,8 +581,9 @@ static int create_trace_kprobe(int argc, char **argv)
> pr_info("Failed to parse either an address or a symbol.\n");
> return ret;
> }
> - if (offset && is_return &&
> - !kprobe_on_func_entry(NULL, symbol, offset)) {
> + if (kprobe_on_func_entry(NULL, symbol, offset))
> + flags |= TPARG_FL_FENTRY;
> + if (offset && is_return && !(flags & TPARG_FL_FENTRY)) {
> pr_info("Given offset is not valid for return probe.\n");
> return -EINVAL;
> }
> @@ -650,7 +653,7 @@ static int create_trace_kprobe(int argc, char **argv)
>
> /* Parse fetch argument */
> ret = traceprobe_parse_probe_arg(arg, &tk->tp.size, parg,
> - is_return, true);
> + flags);
> if (ret) {
> pr_info("Parse error at argument[%d]. (%d)\n", i, ret);
> goto error;
> @@ -890,6 +893,11 @@ process_fetch_insn(struct fetch_insn *code, struct pt_regs *regs, void *dest,
> case FETCH_OP_COMM:
> val = (unsigned long)current->comm;
> break;
> +#ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
> + case FETCH_OP_ARG:
> + val = regs_get_kernel_argument(regs, code->param);
> + break;
> +#endif
> default:
> return -EILSEQ;
> }
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index f41a2989130c..f4b83927d6c0 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -171,14 +171,13 @@ int traceprobe_split_symbol_offset(char *symbol, 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_insn *code, bool is_return,
> - bool is_kprobe)
> + struct fetch_insn *code, unsigned int flags)
> {
> int ret = 0;
> unsigned long param;
>
> if (strcmp(arg, "retval") == 0) {
> - if (is_return)
> + if (flags & TPARG_FL_RETURN)
> code->op = FETCH_OP_RETVAL;
> else
> ret = -EINVAL;
> @@ -187,7 +186,8 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
> code->op = FETCH_OP_STACKP;
> } else if (isdigit(arg[5])) {
> ret = kstrtoul(arg + 5, 10, ¶m);
> - if (ret || (is_kprobe && param > PARAM_MAX_STACK))
> + if (ret || ((flags & TPARG_FL_KERNEL) &&
> + param > PARAM_MAX_STACK))
> ret = -EINVAL;
> else {
> code->op = FETCH_OP_STACK;
> @@ -197,6 +197,18 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
> ret = -EINVAL;
> } else if (strcmp(arg, "comm") == 0) {
> code->op = FETCH_OP_COMM;
> +#ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
> + } else if (((flags & TPARG_FL_MASK) ==
> + (TPARG_FL_KERNEL | TPARG_FL_FENTRY)) &&
> + strncmp(arg, "arg", 3) == 0) {
> + if (!isdigit(arg[3]))
> + return -EINVAL;
> + ret = kstrtoul(arg + 3, 10, ¶m);
> + if (ret || !param || param > PARAM_MAX_STACK)
> + return -EINVAL;
> + code->op = FETCH_OP_ARG;
> + code->param = (unsigned int)param - 1;
> +#endif
> } else
> ret = -EINVAL;
>
> @@ -207,7 +219,7 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
> static int
> parse_probe_arg(char *arg, const struct fetch_type *type,
> struct fetch_insn **pcode, struct fetch_insn *end,
> - bool is_return, bool is_kprobe)
> + unsigned int flags)
> {
> struct fetch_insn *code = *pcode;
> unsigned long param;
> @@ -217,8 +229,7 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
>
> switch (arg[0]) {
> case '$':
> - ret = parse_probe_vars(arg + 1, type, code,
> - is_return, is_kprobe);
> + ret = parse_probe_vars(arg + 1, type, code, flags);
> break;
>
> case '%': /* named register */
> @@ -240,7 +251,7 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
> code->immediate = param;
> } else if (arg[1] == '+') {
> /* kprobes don't support file offsets */
> - if (is_kprobe)
> + if (flags & TPARG_FL_KERNEL)
> return -EINVAL;
>
> ret = kstrtol(arg + 2, 0, &offset);
> @@ -251,7 +262,7 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
> code->immediate = (unsigned long)offset; // imm64?
> } else {
> /* uprobes don't support symbols */
> - if (!is_kprobe)
> + if (!(flags & TPARG_FL_KERNEL))
> return -EINVAL;
>
> ret = traceprobe_split_symbol_offset(arg + 1, &offset);
> @@ -292,8 +303,7 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
> const struct fetch_type *t2 = find_fetch_type(NULL);
>
> *tmp = '\0';
> - ret = parse_probe_arg(arg, t2, &code, end, is_return,
> - is_kprobe);
> + ret = parse_probe_arg(arg, t2, &code, end, flags);
> if (ret)
> break;
> if (code->op == FETCH_OP_COMM)
> @@ -353,7 +363,7 @@ static int __parse_bitfield_probe_arg(const char *bf,
>
> /* String length checking wrapper */
> int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
> - struct probe_arg *parg, bool is_return, bool is_kprobe)
> + struct probe_arg *parg, unsigned int flags)
> {
> struct fetch_insn *code, *tmp = NULL;
> const char *t;
> @@ -393,7 +403,7 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
> code[FETCH_INSN_MAX - 1].op = FETCH_OP_END;
>
> ret = parse_probe_arg(arg, parg->type, &code, &code[FETCH_INSN_MAX - 1],
> - is_return, is_kprobe);
> + flags);
> if (ret)
> goto fail;
>
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index ef477bd8468a..ff91faf70887 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -35,6 +35,7 @@
> #include <linux/stringify.h>
> #include <linux/limits.h>
> #include <linux/uaccess.h>
> +#include <linux/bitops.h>
> #include <asm/bitsperlong.h>
>
> #include "trace.h"
> @@ -89,6 +90,7 @@ enum fetch_op {
> FETCH_OP_RETVAL, /* Return value */
> FETCH_OP_IMM, /* Immediate : .immediate */
> FETCH_OP_COMM, /* Current comm */
> + FETCH_OP_ARG, /* Function argument : .param */
> FETCH_OP_FOFFS, /* File offset: .immediate */
> // Stage 2 (dereference) op
> FETCH_OP_DEREF, /* Dereference: .offset */
> @@ -261,8 +263,13 @@ find_event_file_link(struct trace_probe *tp, struct trace_event_file *file)
> return NULL;
> }
>
> +#define TPARG_FL_RETURN BIT(0)
> +#define TPARG_FL_KERNEL BIT(1)
> +#define TPARG_FL_FENTRY BIT(2)
> +#define TPARG_FL_MASK GENMASK(2, 0)
> +
> extern int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
> - struct probe_arg *parg, bool is_return, bool is_kprobe);
> + struct probe_arg *parg, unsigned int flags);
>
> extern int traceprobe_conflict_field_name(const char *name,
> struct probe_arg *args, int narg);
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 64c1fbe087a1..e15da2281855 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -519,7 +519,7 @@ static int create_trace_uprobe(int argc, char **argv)
>
> /* Parse fetch argument */
> ret = traceprobe_parse_probe_arg(arg, &tu->tp.size, parg,
> - is_return, false);
> + is_return ? TPARG_FL_RETURN : 0);
> if (ret) {
> pr_info("Parse error at argument[%d]. (%d)\n", i, ret);
> goto error;
>
--
Masami Hiramatsu <mhiramat@...nel.org>
Powered by blists - more mailing lists