Message-ID: <20110503140717.GA29736@Krystal>
Date:	Tue, 3 May 2011 10:07:17 -0400
From:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:	LKML <linux-kernel@...r.kernel.org>,
	Steven Rostedt <rostedt@...dmis.org>
Cc:	Ingo Molnar <mingo@...e.hu>, Thomas Gleixner <tglx@...utronix.de>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Subject: Re: [RFC patch 01/32] TRACE_EVENT: gradual semicolon removal

Hi,

And here is a self-reply to patch 01/32, due to LKML refusing messages
with more than 20 recipients.

Thanks,

Mathieu

* Mathieu Desnoyers (mathieu.desnoyers@...icios.com) wrote:
> Initiate the removal of the extra semicolons at the end of:
> 
> TRACE_EVENT(
>   ...
> );     <---- here,
> 
> TRACE_EVENT_FN(
>   ...
> );     <---- here,
> 
> DEFINE_EVENT(
>   ...
> );     <---- here,
> 
> DEFINE_EVENT_PRINT(
>   ...
> );     <---- and here.
> 
> by removing the semicolon from the comment in tracepoint.h explaining the
> TRACE_EVENT() usage. It is now also mandated that all declarations done in the
> trace event headers should be surrounded by ifdefs checks: it is already
> necessary, but some users get away from this requirement for structure
> forward-declarations.
> 
> I am proposing to merge this patchset through the "tracing" tree for better
> coordination.
> 
> Adding the missing semicolon within the DEFINE_EVENT(), DEFINE_EVENT_PRINT()
> and DEFINE_TRACE_FN() macros is required as a preliminary step to remove extra
> semicolons.  We currently are not seeing any impact of this missing semicolon
> because extra they appear all over the place in the code generated from
> TRACE_EVENT within ftrace stages. The side-effect of these extra semicolons are:
> 
> a) to pollute the preprocessor output, leaving extra ";" between trace event
> instances in trace points creation.
> 
> b) to render impossible creation of an array of events. Extra semicolons are
> not a matter as long as TRACE_EVENT creates statements, structure elements or
> functions, because extra semicolons are discarded by the compiler. However,
> when creating an array, the separator is the comma, and the semicolon causes a
> parse error.
> 
> 
> * So what is the motivation for removing these semicolons ?
> 
> Currently, Ftrace is creating a "ftrace_define_fields_##call" function for each
> event to define the event fields, which consumes space. It also has the
> ".fields" list head to keep a dynamically generated list of event fields.
> 
> By allowing creation of arrays of events, we can do the following: turn the
> dynamically created list of event fields into a first-level const array listing
> event fields. We can use ARRAY_SIZE() on this array to know its size,
> statically. Then, in a following trace event stage, we can create another const
> array containing tuples of (pointers to the event-specific arrays, array size).
> 
> So we get all the same information Ftrace currently gets with much less code
> overall, much less read-write data, and less dynamic initialization code.
> 
> 
> * Why do this incrementally ?
> 
> After this preliminary patch, the semicolon removal can be done gradually
> without breaking the build: we can be in an intermediate state with some files
> having semicolons and others without. This is therefore good for
> bissectability, and seems like a nice way to bring in an internal API change
> without making developers suffer too much. Then, once things are stabilized, we
> can start modifying the Ftrace code to generate the more space-efficient arrays
> (possibly in a release-cycle or so), knowing that this enforces a strict
> requirement on semicolon removal.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
> Acked-by: Frederic Weisbecker <fweisbec@...il.com>
> CC: Alexander Graf <agraf@...e.de>
> CC: Alex Elder <aelder@....com>
> CC: Anton Blanchard <anton@...ba.org>
> CC: Arjan van de Ven <arjan@...ux.intel.com>
> CC: Avi Kivity <avi@...hat.com>
> CC: Benjamin Herrenschmidt <benh@...nel.crashing.org>
> CC: Christoph Hellwig <hch@....de>
> CC: Chris Wilson <chris@...is-wilson.co.uk>
> CC: Dave Airlie <airlied@...hat.com>
> CC: Dave Chinner <david@...morbit.com>
> CC: Dave Chinner <dchinner@...hat.com>
> CC: David S. Miller <davem@...emloft.net>
> CC: Gleb Natapov <gleb@...hat.com>
> CC: Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>
> CC: Ingo Molnar <mingo@...e.hu>
> CC: James Bottomley <James.Bottomley@...e.de>
> CC: Jean Pihet <j-pihet@...com>
> CC: Jeff Moyer <jmoyer@...hat.com>
> CC: Jens Axboe <axboe@...nel.dk>
> CC: Jeremy Kerr <jk@...abs.org>
> CC: Jesse Barnes <jbarnes@...tuousgeek.org>
> CC: Joerg Roedel <joerg.roedel@....com>
> CC: Johannes Berg <johannes@...solutions.net>
> CC: John W. Linville <linville@...driver.com>
> CC: Josh Stone <jistone@...hat.com>
> CC: Kei Tokunaga <tokunaga.keiich@...fujitsu.com>
> CC: Koki Sanagi <sanagi.koki@...fujitsu.com>
> CC: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
> CC: Larry Woodman <lwoodman@...hat.com>
> CC: Len Brown <len.brown@...el.com>
> CC: Li Zefan <lizf@...fujitsu.com>
> CC: Marcelo Tosatti <mtosatti@...hat.com>
> CC: Martin K. Petersen <martin.petersen@...cle.com>
> CC: Masami Hiramatsu <mhiramat@...hat.com>
> CC: Mel Gorman <mel@....ul.ie>
> CC: Neil Horman <nhorman@...driver.com>
> CC: Oleg Nesterov <oleg@...hat.com>
> CC: Paul Mackerras <paulus@...ba.org>
> CC: Peter Zijlstra <peterz@...radead.org>
> CC: Reinette Chatre <reinette.chatre@...el.com>
> CC: Rik van Riel <riel@...hat.com>
> CC: Roland McGrath <roland@...hat.com>
> CC: Rusty Russell <rusty@...tcorp.com.au>
> CC: Steven Rostedt <rostedt@...dmis.org>
> CC: Steven Whitehouse <swhiteho@...hat.com>
> CC: Tejun Heo <tj@...nel.org>
> CC: Theodore Ts'o <tytso@....edu>
> CC: Thomas Gleixner <tglx@...utronix.de>
> CC: Thomas Renninger <trenn@...e.de>
> CC: Tomohiro Kusumi <kusumi.tomohiro@...fujitsu.com>
> CC: Vadim Rozenfeld <vrozenfe@...hat.com>
> CC: Xiao Guangrong <xiaoguangrong@...fujitsu.com>
> CC: Zhu Yi <yi.zhu@...el.com>
> ---
>  include/linux/tracepoint.h |    4 ++--
>  include/trace/ftrace.h     |   10 +++++-----
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> Index: linux-2.6-lttng/include/trace/ftrace.h
> ===================================================================
> --- linux-2.6-lttng.orig/include/trace/ftrace.h
> +++ linux-2.6-lttng/include/trace/ftrace.h
> @@ -34,8 +34,8 @@
>  			     PARAMS(args),		       \
>  			     PARAMS(tstruct),		       \
>  			     PARAMS(assign),		       \
> -			     PARAMS(print));		       \
> -	DEFINE_EVENT(name, name, PARAMS(proto), PARAMS(args));
> +			     PARAMS(print))		       \
> +	DEFINE_EVENT(name, name, PARAMS(proto), PARAMS(args))
>  
>  
>  #undef __field
> @@ -69,7 +69,7 @@
>  #undef DEFINE_EVENT
>  #define DEFINE_EVENT(template, name, proto, args)	\
>  	static struct ftrace_event_call	__used		\
> -	__attribute__((__aligned__(4))) event_##name
> +	__attribute__((__aligned__(4))) event_##name;
>  
>  #undef DEFINE_EVENT_PRINT
>  #define DEFINE_EVENT_PRINT(template, name, proto, args, print)	\
> @@ -588,7 +588,7 @@ static struct ftrace_event_call __used e
>  	.print_fmt		= print_fmt_##template,			\
>  };									\
>  static struct ftrace_event_call __used					\
> -__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
> +__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call;
>  
>  #undef DEFINE_EVENT_PRINT
>  #define DEFINE_EVENT_PRINT(template, call, proto, args, print)		\
> @@ -602,7 +602,7 @@ static struct ftrace_event_call __used e
>  	.print_fmt		= print_fmt_##call,			\
>  };									\
>  static struct ftrace_event_call __used					\
> -__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
> +__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call;
>  
>  #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
>  
> Index: linux-2.6-lttng/include/linux/tracepoint.h
> ===================================================================
> --- linux-2.6-lttng.orig/include/linux/tracepoint.h
> +++ linux-2.6-lttng/include/linux/tracepoint.h
> @@ -187,7 +187,7 @@ do_trace:								\
>  		&__tracepoint_##name;
>  
>  #define DEFINE_TRACE(name)						\
> -	DEFINE_TRACE_FN(name, NULL, NULL);
> +	DEFINE_TRACE_FN(name, NULL, NULL)
>  
>  #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)				\
>  	EXPORT_SYMBOL_GPL(__tracepoint_##name)
> @@ -345,7 +345,7 @@ do_trace:								\
>   *		__entry->prev_comm, __entry->prev_pid, __entry->prev_prio,
>   *		__entry->next_comm, __entry->next_pid, __entry->next_prio),
>   *
> - * );
> + * )
>   *
>   * This macro construct is thus used for the regular printk format
>   * tracing setup, it is used to construct a function pointer based
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
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/

Powered by blists - more mailing lists