Messages in this thread Patch in this message | | | Date | Thu, 13 Nov 2014 22:32:06 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH v2 2/3] tracing, function_graph: add additional marks to signal very large function execution time |
| |
On Wed, 5 Nov 2014 16:18:45 +0900 byungchul.park@lge.com wrote:
> +/* trace overhead mark */ > +struct trace_mark { > + unsigned long long val; /* unit: nsec */ > + char sym; > +};
Please format the above to:
struct trace_mark { unsigned long long val; char sym; };
> + > +#define DEFINE_MARK_STRUCT static const struct trace_mark mark[]
Why is this a macro?
> +#define DEFINE_MARK(v, s) {.val = v, .sym = s} > +#define MARK(d) \ > +({ \ > + int i = ARRAY_SIZE(mark); \ > + while ((unsigned long long)(d) <= mark[--i].val && i > 0); \ > + mark[i].sym; \ > +})
Why is this a macro and not a static inline?
static inline char find_trace_mark(unsigned long long duration) { int i = ARRAY_SIZE(mark);
while (duration <= mark[--i].val && i > 0) ;
return make[i]. sym; }
That's much more readable.
> > /** > * struct tracer - a specific tracer and its callbacks to interact with debugfs > diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c > index cb33f46..8a62907 100644 > --- a/kernel/trace/trace_functions_graph.c > +++ b/kernel/trace/trace_functions_graph.c > @@ -797,6 +797,14 @@ trace_print_graph_duration(unsigned long long duration, struct trace_seq *s) > return TRACE_TYPE_HANDLED; > } > > +DEFINE_MARK_STRUCT = { > + DEFINE_MARK(0ULL , ' '), /* 0 usecs */ > + DEFINE_MARK(10000ULL , '+'), /* 10 usecs */ > + DEFINE_MARK(100000ULL , '!'), /* 100 usecs */ > + DEFINE_MARK(1000000ULL , '#'), /* 1000 usecs */ > + DEFINE_MARK(1000000000ULL , '$'), /* 1 sec */ > +};
Don't need to use a name as big as DEFINE_MARK. Just have:
#undef MARK #define MARK(v, s) { .val = v, .sym = s }
struct trace_mark mark[] = { MARK(0ULL , ' '), /* 0 usecs */ [...] };
With the define directly above the call, we don't need need to be that descriptive.
> + > static enum print_line_t > print_graph_duration(unsigned long long duration, struct trace_seq *s, > u32 flags) > @@ -822,12 +830,7 @@ print_graph_duration(unsigned long long duration, struct trace_seq *s, > > /* Signal a overhead of time execution to the output */ > if (flags & TRACE_GRAPH_PRINT_OVERHEAD) { > - /* Duration exceeded 100 usecs */ > - if (duration > 100000ULL) > - ret = trace_seq_puts(s, "! "); > - /* Duration exceeded 10 usecs */ > - else if (duration > 10000ULL) > - ret = trace_seq_puts(s, "+ "); > + ret = trace_seq_printf(s, "%c ", MARK(duration));
Then this could just be:
ret = trace_seq_printf(s, "%c ", find_trace_mark(duration));
-- Steve
> } > > /*
| |