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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20070714164819.GA14489@infradead.org>
Date:	Sat, 14 Jul 2007 17:48:19 +0100
From:	Christoph Hellwig <hch@...radead.org>
To:	Tom Zanussi <zanussi@...ibm.com>
Cc:	linux-kernel@...r.kernel.org, adobriyan@...il.com,
	dwilder@...ibm.com, hunt@...hat.com
Subject: Re: [RFC PATCH 2/3] Generic Trace Setup and Control (GTSC) code

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@...e.de>
> + * Moved to utt.c by Tom Zanussi <zanussi@...ibm.com>, 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/