Re: [RFC PATCH 2/3] Generic Trace Setup and Control (GTSC) code

From: Christoph Hellwig
Date: Sat Jul 14 2007 - 12:48:32 EST


On Fri, Jun 29, 2007 at 10:24:32PM -0500, Tom Zanussi wrote:
> The Generic Tracing and Control Interface (GTSC) code.

The code looks fine to me from a first look, but the the name is just dumb.
Given that all your internal identifiers are trace_* or TRACE_ go all
the way and make it CONFIG_TRACE and lib/trace.c aswell.

Long term we probably want more complex tracing based on lttng, but I'm
a big fan of starting out simple and doing incremental changes.

So my 1c for merging this in 2.6.23 with the exports ripped out until
we actually grow modular users (which will probably come with the blktrace
markers conversion later on)

> +static inline struct trace_info *trace_setup(const char *root,
> + const char *name,
> + u32 buf_size,
> + u32 buf_nr,
> + u32 flags)

static inline struct trace_info *trace_setup(const char *root,
const char *name, u32 buf_size, u32 buf_nr, u32 flags)

> + * Based on blktrace code, Copyright (C) 2006 Jens Axboe <axboe@xxxxxxx>
> + * Moved to utt.c by Tom Zanussi <zanussi@xxxxxxxxxx>, 2006

It's gstc.c now and should be trace.c, nevermind that filename doesn't
really belong here anyway :)

> +unsigned long long trace_timestamp(void)
> +{
> + return sched_clock();
> +}
> +EXPORT_SYMBOL_GPL(trace_timestamp);

Using sched_clock is quite dangerous. And just adding a wrapper to get
it exported is not nice either :)

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/