Re: [RFC PATCH] tracing/kprobes: Avoid parsing symbol+offset when updating arguments

2018-11-03 Thread Masami Hiramatsu
On Sat, 3 Nov 2018 13:43:16 -0400
Steven Rostedt  wrote:

> On Sun,  4 Nov 2018 01:03:34 +0900
> Masami Hiramatsu  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.

OK, and original one saves lines too. Please drop it :)

Thank you,

> 
> -- 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 
> > ---
> >  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, );
> > +   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,
> > -);
> > -   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;
> >

Re: [RFC PATCH] tracing/kprobes: Avoid parsing symbol+offset when updating arguments

2018-11-03 Thread Masami Hiramatsu
On Sat, 3 Nov 2018 13:43:16 -0400
Steven Rostedt  wrote:

> On Sun,  4 Nov 2018 01:03:34 +0900
> Masami Hiramatsu  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.

OK, and original one saves lines too. Please drop it :)

Thank you,

> 
> -- 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 
> > ---
> >  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, );
> > +   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,
> > -);
> > -   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;
> >

Re: [RFC PATCH] tracing/kprobes: Avoid parsing symbol+offset when updating arguments

2018-11-03 Thread Steven Rostedt
On Sun,  4 Nov 2018 01:03:34 +0900
Masami Hiramatsu  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 
> ---
>  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, );
> + 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,
> -  );
> - 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 {

Re: [RFC PATCH] tracing/kprobes: Avoid parsing symbol+offset when updating arguments

2018-11-03 Thread Steven Rostedt
On Sun,  4 Nov 2018 01:03:34 +0900
Masami Hiramatsu  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 
> ---
>  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, );
> + 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,
> -  );
> - 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 {

[RFC PATCH] tracing/kprobes: Avoid parsing symbol+offset when updating arguments

2018-11-03 Thread Masami Hiramatsu
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.
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 
---
 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, );
+   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,
-);
-   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;
+   

[RFC PATCH] tracing/kprobes: Avoid parsing symbol+offset when updating arguments

2018-11-03 Thread Masami Hiramatsu
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.
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 
---
 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, );
+   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,
-);
-   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;
+