On Wed, 5 Nov 2014 16:18:45 +0900
byungchul.p...@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
> }
>
> /*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
