Re: [RFC patch 01/32] TRACE_EVENT: gradual semicolon removal

Mathieu Desnoyers
2011-05-03T14:07:21Z

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@efficios.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@efficios.com>
> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> CC: Alexander Graf <agraf@suse.de>
> CC: Alex Elder <aelder@sgi.com>
> CC: Anton Blanchard <anton@samba.org>
> CC: Arjan van de Ven <arjan@linux.intel.com>
> CC: Avi Kivity <avi@redhat.com>
> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: Christoph Hellwig <hch@lst.de>
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Dave Airlie <airlied@redhat.com>
> CC: Dave Chinner <david@fromorbit.com>
> CC: Dave Chinner <dchinner@redhat.com>
> CC: David S. Miller <davem@davemloft.net>
> CC: Gleb Natapov <gleb@redhat.com>
> CC: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
> CC: Ingo Molnar <mingo@elte.hu>
> CC: James Bottomley <James.Bottomley@suse.de>
> CC: Jean Pihet <j-pihet@ti.com>
> CC: Jeff Moyer <jmoyer@redhat.com>
> CC: Jens Axboe <axboe@kernel.dk>
> CC: Jeremy Kerr <jk@ozlabs.org>
> CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> CC: Joerg Roedel <joerg.roedel@amd.com>
> CC: Johannes Berg <johannes@sipsolutions.net>
> CC: John W. Linville <linville@tuxdriver.com>
> CC: Josh Stone <jistone@redhat.com>
> CC: Kei Tokunaga <tokunaga.keiich@jp.fujitsu.com>
> CC: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
> CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> CC: Larry Woodman <lwoodman@redhat.com>
> CC: Len Brown <len.brown@intel.com>
> CC: Li Zefan <lizf@cn.fujitsu.com>
> CC: Marcelo Tosatti <mtosatti@redhat.com>
> CC: Martin K. Petersen <martin.petersen@oracle.com>
> CC: Masami Hiramatsu <mhiramat@redhat.com>
> CC: Mel Gorman <mel@csn.ul.ie>
> CC: Neil Horman <nhorman@tuxdriver.com>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: Paul Mackerras <paulus@samba.org>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Reinette Chatre <reinette.chatre@intel.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Roland McGrath <roland@redhat.com>
> CC: Rusty Russell <rusty@rustcorp.com.au>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Steven Whitehouse <swhiteho@redhat.com>
> CC: Tejun Heo <tj@kernel.org>
> CC: Theodore Ts'o <tytso@mit.edu>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Thomas Renninger <trenn@suse.de>
> CC: Tomohiro Kusumi <kusumi.tomohiro@jp.fujitsu.com>
> CC: Vadim Rozenfeld <vrozenfe@redhat.com>
> CC: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> CC: Zhu Yi <yi.zhu@intel.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.