From: Steven Rostedt <rostedt@goodmis.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] tracing/kprobes: Avoid parsing symbol+offset when updating arguments
Date: Sat, 3 Nov 2018 13:43:16 -0400 [thread overview]
Message-ID: <20181103134317.68873bd8@vmware.local.home> (raw)
In-Reply-To: <154126101356.14912.1379688281618914741.stgit@devbox>
On Sun, 4 Nov 2018 01:03:34 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Introduce symbol_offset data structure for avoiding symbol+offset
> parsing when updating arguments.
>
> For kprobe events, "@symbol+offset" is supported, that requires
> to be updated when new module is loaded because @symbol address
> can be solved at that point. Currently kprobe events saves
> the "symbol+offset" string and parse it repeatedly, but that
> is inefficient.
Do we care if it's inefficient? It's only done on module load and at
the beginning. That's far from a fast path.
If anything, the original solution saves memory, which is a bigger
concern than speed in this case.
-- Steve
> This introduces symbol_offset data structure which can save the
> offset and symbol separated, so that we don't need to parse it
> anymore.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
> kernel/trace/trace_probe.c | 49 +++++++++++++++++++++++++-------------------
> kernel/trace/trace_probe.h | 7 +++++-
> 2 files changed, 34 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index bd30e9398d2a..0a3af7d6e133 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -201,6 +201,26 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
> return ret;
> }
>
> +static struct symbol_offset *allocate_symbol_offset(char *sym_offs_str)
> +{
> + int ret;
> + long offset = 0;
> + struct symbol_offset *sof;
> +
> + ret = traceprobe_split_symbol_offset(sym_offs_str, &offset);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + sof = kzalloc(sizeof(struct symbol_offset) + strlen(sym_offs_str) + 1,
> + GFP_KERNEL);
> + if (!sof)
> + return ERR_PTR(-ENOMEM);
> +
> + sof->offset = offset;
> + strcpy(sof->symbol, sym_offs_str);
> + return sof;
> +}
> +
> /* Recursive argument parser */
> static int
> parse_probe_arg(char *arg, const struct fetch_type *type,
> @@ -253,9 +273,9 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
>
> /* Preserve symbol for updating */
> code->op = FETCH_NOP_SYMBOL;
> - code->data = kstrdup(arg + 1, GFP_KERNEL);
> - if (!code->data)
> - return -ENOMEM;
> + code->symoffs = allocate_symbol_offset(arg + 1);
> + if (IS_ERR(code->symoffs))
> + return PTR_ERR(code->symoffs);
> if (++code == end)
> return -E2BIG;
>
> @@ -483,7 +503,7 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
> if (ret) {
> for (code = tmp; code < tmp + FETCH_INSN_MAX; code++)
> if (code->op == FETCH_NOP_SYMBOL)
> - kfree(code->data);
> + kfree(code->symoffs);
> }
> kfree(tmp);
>
> @@ -513,7 +533,7 @@ void traceprobe_free_probe_arg(struct probe_arg *arg)
>
> while (code && code->op != FETCH_OP_END) {
> if (code->op == FETCH_NOP_SYMBOL)
> - kfree(code->data);
> + kfree(code->symoffs);
> code++;
> }
> kfree(arg->code);
> @@ -525,31 +545,18 @@ void traceprobe_free_probe_arg(struct probe_arg *arg)
> int traceprobe_update_arg(struct probe_arg *arg)
> {
> struct fetch_insn *code = arg->code;
> - long offset;
> - char *tmp;
> - char c;
> - int ret = 0;
>
> while (code && code->op != FETCH_OP_END) {
> if (code->op == FETCH_NOP_SYMBOL) {
> if (code[1].op != FETCH_OP_IMM)
> return -EINVAL;
>
> - tmp = strpbrk(code->data, "+-");
> - if (tmp)
> - c = *tmp;
> - ret = traceprobe_split_symbol_offset(code->data,
> - &offset);
> - if (ret)
> - return ret;
> -
> code[1].immediate =
> - (unsigned long)kallsyms_lookup_name(code->data);
> - if (tmp)
> - *tmp = c;
> + (unsigned long)kallsyms_lookup_name(
> + code->symoffs->symbol);
> if (!code[1].immediate)
> return -ENOENT;
> - code[1].immediate += offset;
> + code[1].immediate += code->symoffs->offset;
> }
> code++;
> }
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 974afc1a3e73..942424d05427 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -103,6 +103,11 @@ enum fetch_op {
> FETCH_NOP_SYMBOL, /* Unresolved Symbol holder */
> };
>
> +struct symbol_offset {
> + long offset;
> + char symbol[];
> +};
> +
> struct fetch_insn {
> enum fetch_op op;
> union {
> @@ -117,7 +122,7 @@ struct fetch_insn {
> unsigned char rshift;
> };
> unsigned long immediate;
> - void *data;
> + struct symbol_offset *symoffs;
> };
> };
>
next prev parent reply other threads:[~2018-11-03 17:43 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-01 14:29 [BUGFIX PATCH] tracing/kprobes: Fix strpbrk() argument order Masami Hiramatsu
2018-11-01 19:10 ` Steven Rostedt
2018-11-02 7:14 ` Masami Hiramatsu
2018-11-02 13:54 ` Steven Rostedt
2018-11-03 11:54 ` Masami Hiramatsu
2018-11-03 13:17 ` Steven Rostedt
2018-11-03 16:21 ` Masami Hiramatsu
2018-11-03 16:03 ` [RFC PATCH] tracing/kprobes: Avoid parsing symbol+offset when updating arguments Masami Hiramatsu
2018-11-03 17:43 ` Steven Rostedt [this message]
2018-11-04 2:13 ` Masami Hiramatsu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181103134317.68873bd8@vmware.local.home \
--to=rostedt@goodmis.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).