All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>,
	Zhaolei <zhaolei@cn.fujitsu.com>,
	Tom Zanussi <tzanussi@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h
Date: Tue, 14 Apr 2009 01:28:07 +0200	[thread overview]
Message-ID: <20090413232807.GE817@elte.hu> (raw)
In-Reply-To: <alpine.DEB.2.00.0904131907020.3041@gandalf.stny.rr.com>


* Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> On Tue, 14 Apr 2009, Ingo Molnar wrote:
> 
> > 
> > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > 
> > > On Mon, Apr 13, 2009 at 11:54:11AM +0800, Zhaolei wrote:
> > > > Impact: refactor code, no functionality changed
> > > > 
> > > > Files in include/trace/ should be definition of tracepoints, and header
> > > > file for boot trace should put to include/linux/.
> > > > 
> > > > Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > > ---
> > > 
> > > Until now I had the opinion that it's good to let every tracing 
> > > headers to be placed in include/trace/* because they are not 
> > > useful for anything else than the tracer itself so that we don't 
> > > encumber include/linux for private things.
> > > 
> > > So that we have both tracepoints/trace_events plus the low-level 
> > > tracers headers in include/trace/*
> > > 
> > > I'm not opposite to this change, but seeing this patch and the 
> > > recent divide of kmemtrace headers, I would like to know the 
> > > opinion of Ingo and Steven about the strict role of 
> > > include/trace/* Is it only for tracepoints-like bits, or oslo 
> > > intended for every private tracing purposes?
> > 
> > The header split itself is probably good to do - to keep the 'pure' 
> > portions of tracepoint definitions cleanly separated from more 
> > functional details like kmem tracer initialization.
> > 
> > The move to include/linux/ is indeed more debatable. I think if a 
> > header says 'footrace.h' in its name, it could easily be in 
> > include/trace/foo.h instead? Makes for a tidier structure - 
> > include/linux/ is massively over-crowded already.
> > 
> > Steve, what do you think?
> 
> We actually discussed this a little at the Linux Collaboration 
> Summit. The idea was to keep only the tracepoints aka TRACE_EVENT 
> code in include/trace/ and perhaps special headers that work with 
> the TRACE_EVENT macros. But the infrastructure of the tracers 
> would stay in include/linux.
> 
> The rational is that we have a separate directory reserved only 
> for trace points / trace events. Adding more headers into that 
> directory would make it a bit harder to see right away what trace 
> events where defined for a particular kernel source.

Hm, i have to say that is true committee design ;-)

The sane thing would be to put event headers into 
include/trace/events/ and put more generic/utility headers into 
include/trace/.

Reserving a full subdirectory for one singular purpose is a needless 
waste of a nice (and unique) name-space resource.

	Ingo

  reply	other threads:[~2009-04-13 23:28 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-13  3:54 [PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h Zhaolei
2009-04-13  3:55 ` [PATCH 2/2] tracing, syscalltrace: Move include/trace/syscall.h to include/linux/syscalltrace.h Zhaolei
2009-04-13 14:28   ` Frederic Weisbecker
2009-04-13 14:25 ` [PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h Frederic Weisbecker
2009-04-13 22:24   ` Ingo Molnar
2009-04-13 23:11     ` Steven Rostedt
2009-04-13 23:28       ` Ingo Molnar [this message]
2009-04-13 23:34         ` Steven Rostedt
2009-04-13 23:40           ` Ingo Molnar
2009-04-13 23:51             ` Steven Rostedt
2009-04-13 23:53               ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090413232807.GE817@elte.hu \
    --to=mingo@elte.hu \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tzanussi@gmail.com \
    --cc=zhaolei@cn.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.