mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@redhat.com>,
	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, 3 Dec 2009 09:09:42 -0500	[thread overview]
Message-ID: <20091203140942.GB24905@Krystal> (raw)
In-Reply-To: <1259848493.12870.114.camel@gandalf.stny.rr.com>

* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Thu, 2009-12-03 at 08:51 -0500, Masami Hiramatsu wrote:
> 
> > > 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).
> 
> I'm fine with a kernel/events dir.

Yep, sounds fine.

Maybe we could have separate files for:

a) event definitions
b) class definitions

?

> 
> > 
> > >> 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.
> 
> Why? Modules usually do have their own headers in their sub system.
> 
> OK, if a module keeps their headers global (include/linux) then sure
> they can keep their tracepoint header in include/trace/events. But I
> still think that the module CREATE_TRACE_POINTS code should be kept with
> the module code itself (but in a small separate file).

I agree with Steven here: modules should come with their own trace event
definitions, and if the trace classes they use are not available in the
standard kernel, they should come with these trace classes definitions
too.

A small *_trace.c file linked along with the module looks fine by me.

And please, try to re-used the already existing symbol dependency
management already present in the kernel to deal with module dependency
on class and dependency such as:

  module-a.ko
    defines trace class
  module-b.ko
  module-c.ko

    module-b and module-c define events which depend on the trace class.

If you make the event definition depend on a symbol defined in module-a,
everything should work flawlessly. It also works if the class is defined
in the core kernel.

Thanks,

Mathieu

> 
> -- Steve
> 
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

  reply	other threads:[~2009-12-03 14:14 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
2009-12-03 13:54                                     ` Steven Rostedt
2009-12-03 14:09                                       ` Mathieu Desnoyers [this message]
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=20091203140942.GB24905@Krystal \
    --to=mathieu.desnoyers@polymtl.ca \
    --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=mhiramat@redhat.com \
    --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

all inboxes | Powered by JetHome®