2019-04-18 09:10:20

by Thomas Gleixner

Subject: [patch V2 20/29] tracing: Simplify stacktrace retrieval in histograms

The indirection through struct stack_trace is not necessary at all. Use the
storage array based interface.

Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Steven Rostedt <[email protected]>
---
kernel/trace/trace_events_hist.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5186,7 +5186,6 @@ static void event_hist_trigger(struct ev
u64 var_ref_vals[TRACING_MAP_VARS_MAX];
char compound_key[HIST_KEY_SIZE_MAX];
struct tracing_map_elt *elt = NULL;
- struct stack_trace stacktrace;
struct hist_field *key_field;
u64 field_contents;
void *key = NULL;
@@ -5198,14 +5197,9 @@ static void event_hist_trigger(struct ev
key_field = hist_data->fields[i];

if (key_field->flags & HIST_FIELD_FL_STACKTRACE) {
- stacktrace.max_entries = HIST_STACKTRACE_DEPTH;
- stacktrace.entries = entries;
- stacktrace.nr_entries = 0;
- stacktrace.skip = HIST_STACKTRACE_SKIP;
-
- memset(stacktrace.entries, 0, HIST_STACKTRACE_SIZE);
- save_stack_trace(&stacktrace);
-
+ memset(entries, 0, HIST_STACKTRACE_SIZE);
+ stack_trace_save(entries, HIST_STACKTRACE_DEPTH,
+ HIST_STACKTRACE_SKIP);
key = entries;
} else {
field_contents = key_field->fn(key_field, elt, rbe, rec);

2019-04-18 13:46:07

by Steven Rostedt

Subject: Re: [patch V2 20/29] tracing: Simplify stacktrace retrieval in histograms

[ Added Tom Zanussi ]

On Thu, 18 Apr 2019 10:41:39 +0200
Thomas Gleixner <[email protected]> wrote:

> The indirection through struct stack_trace is not necessary at all. Use the
> storage array based interface.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: Steven Rostedt <[email protected]>

Looks fine to me

Acked-by: Steven Rostedt (VMware) <[email protected]>

But...

Tom,

Can you review this too?

Patch series starts here:

http://lkml.kernel.org/r/[email protected]

Thanks,

-- Steve

> ---
> kernel/trace/trace_events_hist.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -5186,7 +5186,6 @@ static void event_hist_trigger(struct ev
> u64 var_ref_vals[TRACING_MAP_VARS_MAX];
> char compound_key[HIST_KEY_SIZE_MAX];
> struct tracing_map_elt *elt = NULL;
> - struct stack_trace stacktrace;
> struct hist_field *key_field;
> u64 field_contents;
> void *key = NULL;
> @@ -5198,14 +5197,9 @@ static void event_hist_trigger(struct ev
> key_field = hist_data->fields[i];
>
> if (key_field->flags & HIST_FIELD_FL_STACKTRACE) {
> - stacktrace.max_entries = HIST_STACKTRACE_DEPTH;
> - stacktrace.entries = entries;
> - stacktrace.nr_entries = 0;
> - stacktrace.skip = HIST_STACKTRACE_SKIP;
> -
> - memset(stacktrace.entries, 0, HIST_STACKTRACE_SIZE);
> - save_stack_trace(&stacktrace);
> -
> + memset(entries, 0, HIST_STACKTRACE_SIZE);
> + stack_trace_save(entries, HIST_STACKTRACE_DEPTH,
> + HIST_STACKTRACE_SKIP);
> key = entries;
> } else {
> field_contents = key_field->fn(key_field, elt, rbe, rec);
>

2019-04-18 20:02:21

by Tom Zanussi

Subject: Re: [patch V2 20/29] tracing: Simplify stacktrace retrieval in histograms

On Thu, 2019-04-18 at 09:40 -0400, Steven Rostedt wrote:
> [ Added Tom Zanussi ]
>
> On Thu, 18 Apr 2019 10:41:39 +0200
> Thomas Gleixner <[email protected]> wrote:
>
> > The indirection through struct stack_trace is not necessary at all.
> > Use the
> > storage array based interface.
> >
> > Signed-off-by: Thomas Gleixner <[email protected]>
> > Cc: Steven Rostedt <[email protected]>
>
> Looks fine to me
>
> Acked-by: Steven Rostedt (VMware) <[email protected]>
>
> But...
>
> Tom,
>
> Can you review this too?

Looks good to me too!

Acked-by: Tom Zanussi <[email protected]>


>
> Patch series starts here:
>
> http://lkml.kernel.org/r/[email protected]
>
> Thanks,
>
> -- Steve
>
> > ---
> > kernel/trace/trace_events_hist.c | 12 +++---------
> > 1 file changed, 3 insertions(+), 9 deletions(-)
> >
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -5186,7 +5186,6 @@ static void event_hist_trigger(struct ev
> > u64 var_ref_vals[TRACING_MAP_VARS_MAX];
> > char compound_key[HIST_KEY_SIZE_MAX];
> > struct tracing_map_elt *elt = NULL;
> > - struct stack_trace stacktrace;
> > struct hist_field *key_field;
> > u64 field_contents;
> > void *key = NULL;
> > @@ -5198,14 +5197,9 @@ static void event_hist_trigger(struct ev
> > key_field = hist_data->fields[i];
> >
> > if (key_field->flags & HIST_FIELD_FL_STACKTRACE) {
> > - stacktrace.max_entries = HIST_STACKTRACE_DEPTH;
> > - stacktrace.entries = entries;
> > - stacktrace.nr_entries = 0;
> > - stacktrace.skip = HIST_STACKTRACE_SKIP;
> > -
> > - memset(stacktrace.entries, 0,
> > HIST_STACKTRACE_SIZE);
> > - save_stack_trace(&stacktrace);
> > -
> > + memset(entries, 0, HIST_STACKTRACE_SIZE);
> > + stack_trace_save(entries,
> > HIST_STACKTRACE_DEPTH,
> > + HIST_STACKTRACE_SKIP);
> > key = entries;
> > } else {
> > field_contents = key_field->fn(key_field, elt,
> > rbe, rec);
> >
>
>

2019-04-18 20:18:24

by Steven Rostedt

Subject: Re: [patch V2 20/29] tracing: Simplify stacktrace retrieval in histograms

On Thu, 18 Apr 2019 14:58:55 -0500
Tom Zanussi <[email protected]> wrote:

> > Tom,
> >
> > Can you review this too?
>
> Looks good to me too!
>
> Acked-by: Tom Zanussi <[email protected]>
>

Would you be OK to upgrade this to a Reviewed-by tag?

Thanks!

-- Steve

2019-04-18 20:24:55

by Tom Zanussi

Subject: Re: [patch V2 20/29] tracing: Simplify stacktrace retrieval in histograms

Hi Steve,

On Thu, 2019-04-18 at 16:13 -0400, Steven Rostedt wrote:
> On Thu, 18 Apr 2019 14:58:55 -0500
> Tom Zanussi <[email protected]> wrote:
>
> > > Tom,
> > >
> > > Can you review this too?
> >
> > Looks good to me too!
> >
> > Acked-by: Tom Zanussi <[email protected]>
> >
>
> Would you be OK to upgrade this to a Reviewed-by tag?
>

Yeah, I did review it and even tested it, so:

Reviewed-by: Tom Zanussi <[email protected]>
Tested-by: Tom Zanussi <[email protected]>

Tom


> Thanks!
>
> -- Steve