All of lore.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: Wed, 02 Dec 2009 17:36:54 -0500	[thread overview]
Message-ID: <4B16EC06.6010706@redhat.com> (raw)
In-Reply-To: <1259781578.12870.78.camel@gandalf.stny.rr.com>

Steven Rostedt wrote:
>>> I think the kernel developers are smart enough to figure out that these
>>> macros are not a typical DECLARE/DEFINE that is elsewhere. But I think
>>> using the DECLARE/DEFINE names will give them a better idea of what is
>>> happening than to make up something completely new.
>>
>> In my opinion, re-using a well-known keyword (e.g. DECLARE/DEFINE) but
>> applying a different semantic to what is generally agreed upon is a
>> recipe for confusing developers and users, who will skip the review of
>> some pieces of code assuming they already know what "DECLARE" and
>> "DEFINE" stands for.
>>
>> I argue here that the content of trace/events/ headers are _not_ per se
>> declarations nor definitions, and hence they should not confuse people
>> by using inappropriately well-known keywords. They are actually more
>> evolved macros that can be turned in either a declaration or definition,
>> depending if CREATE_TRACE_POINTS is declared.
> 
> And I argue that the semantics here are not too far off to what those
> are. Yes, these macros behave differently if CREATE_TRACE_POINTS is
> declared or not, but I argue that the average (and below average) kernel
> developer is smart enough to understand this difference.
> 
> 
>>
>> When I created the markers/tracepoints, Andrew Morton explained to me
>> the importance of distinguishing DECLARE vs DEFINE macros. I would
>> really like to hear his point of view on the current question.
> 
> I would like to hear Andrew's comments too, as well as anyone else.
> Randy Dunlap seemed to already approve of these naming conventions, and
> he's a pretty picky person too.
> 
> Randy, do you agree that the use of DECLARE/DEFINE here is fine, or do
> you think that we should come up with a better naming. I do not want to
> add any needless abstraction layer for the sake of naming. These macros
> are confusing enough without that.
> 
> Or do you (or anyone else) have a better name?

How about renaming DEFINE_EVENT to TRACE_EVENT_CLASS?

DECLARE_EVENT_CLASS(y, ...)	declare an event-class y
TRACE_EVENT_CLASS(x, y, ...)	define/declare a trace event x from event-class y
TRACE_EVENT(x, ...)		define/declare a trace event x

Thus TRACE_EVENT_* implies that this macro will be expanded
to both of definition and declaration.
I don't think separating it is good idea from the viewpoint
of maintaining code.

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com


  parent reply	other threads:[~2009-12-02 22:37 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 [this message]
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
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=4B16EC06.6010706@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 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.