public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@redhat.com>
To: rostedt@goodmis.org
Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>,
	akpm@linux-foundation.org, Ingo Molnar <mingo@elte.hu>,
	mingo@redhat.com, hpa@zytor.com, linux-kernel@vger.kernel.org,
	randy.dunlap@oracle.com, wcohen@redhat.com, fweisbec@gmail.com,
	tglx@linutronix.de, jbaron@redhat.com,
	linux-tip-commits@vger.kernel.org, Christoph Hellwig <hch@lst.de>
Subject: Re: trace/events: DECLARE vs DEFINE semantic
Date: Thu, 03 Dec 2009 08:51:22 -0500	[thread overview]
Message-ID: <4B17C25A.8090703@redhat.com> (raw)
In-Reply-To: <1259813228.12870.108.camel@gandalf.stny.rr.com>

Steven Rostedt wrote:
> On Wed, 2009-12-02 at 23:00 -0500, Masami Hiramatsu wrote:
>> Mathieu Desnoyers wrote:
>>> In addition, I wonder if we should rename "CREATE_TRACE_POINTS" to
>>> something more suitable while we are here ? Basically, it will affect
>>> all TRACE_CLASS/TRACE_CLASS_EVENT/TRACE_EVENT from headers included
>>> after it's defined.
>>
>> Agreed, CREATE_TRACE_POINTS is a bit irritating thing :-(
> 
> Well, I think the name could use some help, but I don't think it is the
> name that irritates you.

Yes :-)

>> For example, if we call tracepoints defined in same-header on
>> several different files, we need to check other people have
>> already defined CREATE_TRACE_POINTS on another file, because
>> CREATE_TRACE_POINTS must be used once for each header...
>>
>> So, how about introducing a c file which is only for defining
>> tracepoints for kernel parts ? or defining tracepoints in
>> kernel at the beginning of kernel/tracepoint.c ? (and don't
>> touch tracepoints in modules)
> 
> I think the proper fix is to have each tracepoint header have its own C
> file. I believe Christoph does this with xfs.
> 
> Basically, we should have a:
> 
> kernel/sched_trace.c that includes the include/trace/events/sched.h and
> does the define.
> 
> And the same goes for other trace points.

Hmm, I'd rather like to move it into kernel/events/ or something new
sub directory, since those files will have just two lines (define and
include).

>> e.g.
>>
>> @kernel/tracepoint.c
>>  ...
>>  #define CREATE_TRACE_POINTS
>>  #include <trace/events/sched.h>
>>  #include <trace/events/...>
> 
> We could do this for all that is defined in the include/trace/events.
> 
>>  ...
>>
>> @kernel/sched.c
>>  ...
>>  #include <trace/events/sched.h>	/* Just include events header */
>>  ...
>>
>> @fs/ext4/super.c (no change, since it can be module)
>>  ...
>>  #define CREATE_TRACE_POINTS
>>  #include <trace/events/ext4.h>
> 
> Perhaps we should move out anything in include/trace/events that is also
> a module into its sub system?

Would you mean putting those headers in sub-system's directory?
(e.g. fs/ext4/)
In that case, a problem will happen when user want to hook those
tracepoint from their module, because it is hard to find those
local headers.

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com


  reply	other threads:[~2009-12-03 13:49 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-01 17:18 [PATCH v2] tracing: add DEFINE_EVENT(), DEFINE_SINGLE_EVENT() support to docbook Jason Baron
2009-12-01 17:20 ` Randy Dunlap
2009-12-02 10:42 ` [tip:perf/core] tracing: Add " tip-bot for Jason Baron
2009-12-02 13:52   ` Steven Rostedt
2009-12-02 14:01     ` Ingo Molnar
2009-12-02 14:28       ` Steven Rostedt
2009-12-02 14:43         ` Ingo Molnar
2009-12-02 14:55           ` Steven Rostedt
2009-12-02 16:15             ` Randy Dunlap
2009-12-02 16:27             ` Mathieu Desnoyers
2009-12-02 17:11               ` Steven Rostedt
2009-12-02 18:06                 ` Mathieu Desnoyers
2009-12-02 18:19                   ` Steven Rostedt
2009-12-02 19:01                     ` trace/events: DECLARE vs DEFINE semantic Mathieu Desnoyers
2009-12-02 19:19                       ` Steven Rostedt
2009-12-02 19:34                         ` Randy Dunlap
2009-12-02 22:36                         ` Masami Hiramatsu
2009-12-02 22:46                           ` Steven Rostedt
2009-12-02 22:57                             ` Mathieu Desnoyers
2009-12-02 23:08                               ` H. Peter Anvin
2009-12-02 23:13                                 ` Steven Rostedt
2009-12-02 23:18                                   ` H. Peter Anvin
2009-12-02 23:15                                 ` Mathieu Desnoyers
2009-12-03  3:24                                   ` Masami Hiramatsu
2009-12-02 23:10                             ` Mathieu Desnoyers
2009-12-03  4:00                               ` Masami Hiramatsu
2009-12-03  4:07                                 ` Steven Rostedt
2009-12-03 13:51                                   ` Masami Hiramatsu [this message]
2009-12-03 13:54                                     ` Steven Rostedt
2009-12-03 14:09                                       ` Mathieu Desnoyers
2009-12-03 14:24                                         ` Steven Rostedt
2009-12-03 14:42                                           ` Mathieu Desnoyers
2009-12-03 15:31                                       ` Masami Hiramatsu
2009-12-03 15:56                                         ` Steven Rostedt
2009-12-03 16:11                                           ` Masami Hiramatsu
2009-12-02 20:11                       ` [PATCH][tip/perf/core] tracing: Rename TRACE_EVENT and others to something resonable Steven Rostedt
2009-12-02 20:16                         ` Frederic Weisbecker

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=4B17C25A.8090703@redhat.com \
    --to=mhiramat@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=fweisbec@gmail.com \
    --cc=hch@lst.de \
    --cc=hpa@zytor.com \
    --cc=jbaron@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mathieu.desnoyers@polymtl.ca \
    --cc=mingo@elte.hu \
    --cc=mingo@redhat.com \
    --cc=randy.dunlap@oracle.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=wcohen@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox