[PATCH 0/2][RFC] tracing: Showing symbols for TRACE_EVENT

[PATCH 0/2][RFC] tracing: Showing symbols for TRACE_EVENT

[Steven Rostedt]

[Re-sending since my first try was missing a '>' from Frederic's
 email address, and it screwed up quilt mail ]

The __print_symbolic() macro is used by TRACE_EVENT TP_printk to convert
numbers into symbols. But if those numbers are defined as ENUMS, it works
fine for ftrace trace output, but a parser reading the binary trace
will not know how to translate the enum into a number.

The first patch creates a EXTRACT_TRACE_SYMBOL(sym) macro that
a <events>.h file can add to extract out all the enums that it uses
in TRACE_EVENTs. These symbols will then appear in the event_symbols
file in the events/ directory.

The second patch converts softirq symbols in include/trace/events/irq.h

This is an RFC patch set. I want to get your feedback before I push
this out of my git repo.

Thanks,

-- Steve

[PATCH 1/2][RFC] tracing: Add EXTRACT_TRACE_SYMBOL() macro

[Steven Rostedt]

[-- Attachment #1: extract-trace-syms.patch --]
[-- Type: text/plain, Size: 8465 bytes --]

The trace events that are created with the TRACE_EVENT family macros
can be read by binary readers, such as perf and trace-cmd. In order
to translate the binary data, the events also have a format file
associated with them. This format file explains the offset, size and
signess of elements in the raw binary data of an event.

At the end of the format file, there is a description on how to print
this data. A helper function is used to map numbers into strings
called __print_symbolic(). A problem arises when these numbers are
defined as enums in the kernel. The macros that export this data into
the format file does not translate the enums into numbers. Thus we
have in the irq.h file:

 #define softirq_name(sirq) { sirq##_SOFTIRQ, #sirq }
 #define show_softirq_name(val)				\
	__print_symbolic(val,				\
			 softirq_name(HI),		\
			 softirq_name(TIMER),		\
			 softirq_name(NET_TX),		\
			 softirq_name(NET_RX),		\
			 softirq_name(BLOCK),		\
			 softirq_name(BLOCK_IOPOLL),	\
			 softirq_name(TASKLET),		\
			 softirq_name(SCHED),		\
			 softirq_name(HRTIMER),		\
			 softirq_name(RCU))


	TP_printk("vec=%d [action=%s]", __entry->vec,
		  show_softirq_name(__entry->vec))


Which shows up in the event format file as:

  print fmt: "vec=%d [action=%s]", REC->vec, __print_symbolic(REC->vec, { HI_SOFTIRQ, "HI" }, { TIMER_SOFTIRQ, "TIMER" }, { NET_TX_SOFTIRQ, "NET_TX" }, { NET_RX_SOFTIRQ, "NET_RX" }, { BLOCK_SOFTIRQ, "BLOCK" }, { BLOCK_IOPOLL_SOFTIRQ, "BLOCK_IOPOLL" }, { TASKLET_SOFTIRQ, "TASKLET" }, { SCHED_SOFTIRQ, "SCHED" }, { HRTIMER_SOFTIRQ, "HRTIMER" }, { RCU_SOFTIRQ, "RCU" })


The parser has no idea of how to translate "HI_SOFTIRQ" into a number.

This patch adds an EXTRACT_TRACE_SYMBOL() macro that lets a trace header
define symbols that it uses, and will be converted into numbers.
These symbols will be shown in an "event_symbols" file in the event
directory.

By adding this macro with all the softirq names:

   EXTRACT_TRACE_SYMBOL(HI_SOFTIRQ);
   EXTRACT_TRACE_SYMBOL(TIMER_SOFTIRQ);
   EXTRACT_TRACE_SYMBOL(NET_TX_SOFTIRQ);
   EXTRACT_TRACE_SYMBOL(NET_RX_SOFTIRQ);
   EXTRACT_TRACE_SYMBOL(BLOCK_SOFTIRQ);
   EXTRACT_TRACE_SYMBOL(BLOCK_IOPOLL_SOFTIRQ);
   EXTRACT_TRACE_SYMBOL(TASKLET_SOFTIRQ);
   EXTRACT_TRACE_SYMBOL(SCHED_SOFTIRQ);
   EXTRACT_TRACE_SYMBOL(HRTIMER_SOFTIRQ);
   EXTRACT_TRACE_SYMBOL(RCU_SOFTIRQ);

in the events/irq.h file, this produces the output:


   [tracing/]$ cat events/event_symbols
   # Symbol:Size:Value
   # =================
   HI_SOFTIRQ:4:0
   TIMER_SOFTIRQ:4:1
   NET_TX_SOFTIRQ:4:2
   NET_RX_SOFTIRQ:4:3
   BLOCK_SOFTIRQ:4:4
   BLOCK_IOPOLL_SOFTIRQ:4:5
   TASKLET_SOFTIRQ:4:6
   SCHED_SOFTIRQ:4:7
   HRTIMER_SOFTIRQ:4:8
   RCU_SOFTIRQ:4:9

Now a parser can read this file and replace symbols it encounters in the
format files with an actual number.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

---
 include/asm-generic/vmlinux.lds.h |    4 +
 include/linux/tracepoint.h        |    2 
 include/trace/define_trace.h      |   16 ++++++
 kernel/trace/trace_events.c       |   92 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 114 insertions(+)

Index: linux-trace.git/include/asm-generic/vmlinux.lds.h
===================================================================
--- linux-trace.git.orig/include/asm-generic/vmlinux.lds.h	2010-02-11 14:30:03.000000000 -0500
+++ linux-trace.git/include/asm-generic/vmlinux.lds.h	2010-02-12 11:35:48.000000000 -0500
@@ -163,6 +163,10 @@
 	VMLINUX_SYMBOL(__start___verbose) = .;                          \
 	*(__verbose)                                                    \
 	VMLINUX_SYMBOL(__stop___verbose) = .;				\
+	. = ALIGN(32);							\
+	VMLINUX_SYMBOL(__start_trace_syms) = .;				\
+	*(_trace_symbols)						\
+	VMLINUX_SYMBOL(__stop_trace_syms) = .;				\
 	LIKELY_PROFILE()		       				\
 	BRANCH_PROFILE()						\
 	TRACE_PRINTKS()							\
Index: linux-trace.git/include/linux/tracepoint.h
===================================================================
--- linux-trace.git.orig/include/linux/tracepoint.h	2010-02-11 14:30:03.000000000 -0500
+++ linux-trace.git/include/linux/tracepoint.h	2010-02-12 11:35:48.000000000 -0500
@@ -292,4 +292,6 @@ static inline void tracepoint_synchroniz
 		assign, print, reg, unreg)			\
 	DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
 
+#define EXTRACT_TRACE_SYMBOL(symbol)
+
 #endif /* ifdef TRACE_EVENT (see note above) */
Index: linux-trace.git/include/trace/define_trace.h
===================================================================
--- linux-trace.git.orig/include/trace/define_trace.h	2010-02-11 14:30:03.000000000 -0500
+++ linux-trace.git/include/trace/define_trace.h	2010-02-12 11:35:48.000000000 -0500
@@ -43,6 +43,19 @@
 #define DECLARE_TRACE(name, proto, args)	\
 	DEFINE_TRACE(name)
 
+#undef EXTRACT_TRACE_SYMBOL
+#define EXTRACT_TRACE_SYMBOL(symbol)					\
+	struct {							\
+		u64			val;				\
+		const char		*name;				\
+		int			size;				\
+	}  ___trace_sym_##symbol					\
+	__attribute__((section("_trace_symbols"), aligned(32)))	= {	\
+		.name = #symbol,					\
+		.val = (u64)(symbol),					\
+		.size = sizeof(symbol)					\
+	}
+
 #undef TRACE_INCLUDE
 #undef __TRACE_INCLUDE
 
@@ -65,6 +78,9 @@
 
 #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
 
+#undef EXTRACT_TRACE_SYMBOL
+#define EXTRACT_TRACE_SYMBOL(symbol)
+
 #ifdef CONFIG_EVENT_TRACING
 #include <trace/ftrace.h>
 #endif
Index: linux-trace.git/kernel/trace/trace_events.c
===================================================================
--- linux-trace.git.orig/kernel/trace/trace_events.c	2010-02-11 14:30:03.000000000 -0500
+++ linux-trace.git/kernel/trace/trace_events.c	2010-02-12 13:00:15.000000000 -0500
@@ -28,6 +28,20 @@ DEFINE_MUTEX(event_mutex);
 
 LIST_HEAD(ftrace_events);
 
+/*
+ * Would have liked to put trace_syms in a header for define_trace.h to
+ * use, but found that it simply caused include hell.
+ */
+struct trace_syms {
+	u64			val;
+	const char		*name;
+	int			size;
+};
+
+extern struct trace_syms __start_trace_syms;
+extern struct trace_syms __stop_trace_syms;
+
+
 int trace_define_field(struct ftrace_event_call *call, const char *type,
 		       const char *name, int offset, int size, int is_signed,
 		       int filter_type)
@@ -287,6 +301,73 @@ ftrace_event_write(struct file *file, co
 }
 
 static void *
+sym_next(struct seq_file *m, void *v, loff_t *pos)
+{
+	struct trace_syms *sym;
+
+	/* The symbols are aligned at 32 bytes */
+	sym = (void *)&__start_trace_syms + *pos * 32;
+
+	if (sym >= &__stop_trace_syms)
+		return NULL;
+
+	(*pos)++;
+
+	return sym;
+}
+
+static void *sym_start(struct seq_file *m, loff_t *pos)
+{
+	return sym_next(m, NULL, pos);
+}
+
+static int sym_show(struct seq_file *m, void *v)
+{
+	struct trace_syms *sym = v;
+
+	/* Show header on first index */
+	if (sym == &__start_trace_syms)
+		seq_printf(m, "# Symbol:Size:Value\n"
+			   "# =================\n");
+
+	seq_printf(m, "%s:%d:", sym->name, sym->size);
+	switch (sym->size) {
+	case 1:
+		seq_printf(m, "%u\n", (unsigned char)sym->val);
+		break;
+	case 2:
+		seq_printf(m, "%u\n", (unsigned short)sym->val);
+		break;
+	case 4:
+		seq_printf(m, "%u\n", (unsigned int)sym->val);
+		break;
+	default:
+		seq_printf(m, "%llu\n", sym->val);
+		break;
+	}
+
+	return 0;
+}
+
+static void sym_stop(struct seq_file *m, void *p)
+{
+}
+
+static const struct seq_operations show_event_sym_seq_ops = {
+	.start = sym_start,
+	.next = sym_next,
+	.show = sym_show,
+	.stop = sym_stop,
+};
+
+static int
+ftrace_event_sym_seq_open(struct inode *inode, struct file *file)
+{
+	return seq_open(file, &show_event_sym_seq_ops);
+}
+
+
+static void *
 t_next(struct seq_file *m, void *v, loff_t *pos)
 {
 	struct ftrace_event_call *call = v;
@@ -815,6 +896,13 @@ static const struct file_operations ftra
 	.write = subsystem_filter_write,
 };
 
+static const struct file_operations ftrace_event_symbols_fops = {
+	.open = ftrace_event_sym_seq_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = seq_release,
+};
+
 static const struct file_operations ftrace_system_enable_fops = {
 	.open = tracing_open_generic,
 	.read = system_enable_read,
@@ -1280,6 +1368,10 @@ static __init int event_trace_init(void)
 			  ring_buffer_print_entry_header,
 			  &ftrace_show_header_fops);
 
+	/* Event symbols */
+	trace_create_file("event_symbols", 0444, d_events,
+			  NULL, &ftrace_event_symbols_fops);
+
 	trace_create_file("enable", 0644, d_events,
 			  NULL, &ftrace_system_enable_fops);
 

[PATCH 2/2][RFC] tracing: Add extract out softirq names used by irq trace events

[Steven Rostedt]

[-- Attachment #1: irq-events.patch --]
[-- Type: text/plain, Size: 1236 bytes --]

The irq trace events that map the softirq vectors to strings
shows up in the format files as names. To allow binary parsers to
be able to convert these names to their actual numbers, this patch
extracts those enums.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

---
 include/trace/events/irq.h |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: linux-trace.git/include/trace/events/irq.h
===================================================================
--- linux-trace.git.orig/include/trace/events/irq.h	2010-02-12 13:29:44.000000000 -0500
+++ linux-trace.git/include/trace/events/irq.h	2010-02-12 13:33:20.000000000 -0500
@@ -7,6 +7,17 @@
 #include <linux/tracepoint.h>
 #include <linux/interrupt.h>
 
+EXTRACT_TRACE_SYMBOL(HI_SOFTIRQ);
+EXTRACT_TRACE_SYMBOL(TIMER_SOFTIRQ);
+EXTRACT_TRACE_SYMBOL(NET_TX_SOFTIRQ);
+EXTRACT_TRACE_SYMBOL(NET_RX_SOFTIRQ);
+EXTRACT_TRACE_SYMBOL(BLOCK_SOFTIRQ);
+EXTRACT_TRACE_SYMBOL(BLOCK_IOPOLL_SOFTIRQ);
+EXTRACT_TRACE_SYMBOL(TASKLET_SOFTIRQ);
+EXTRACT_TRACE_SYMBOL(SCHED_SOFTIRQ);
+EXTRACT_TRACE_SYMBOL(HRTIMER_SOFTIRQ);
+EXTRACT_TRACE_SYMBOL(RCU_SOFTIRQ);
+
 #define softirq_name(sirq) { sirq##_SOFTIRQ, #sirq }
 #define show_softirq_name(val)				\
 	__print_symbolic(val,				\

Re: [PATCH 2/2][RFC] tracing: Add extract out softirq names used by irq trace events

[Peter Zijlstra]

On Fri, 2010-02-12 at 14:09 -0500, Steven Rostedt wrote:
> plain text document attachment (irq-events.patch)
> The irq trace events that map the softirq vectors to strings
> shows up in the format files as names. To allow binary parsers to
> be able to convert these names to their actual numbers, this patch
> extracts those enums.
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> 
> ---
>  include/trace/events/irq.h |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> Index: linux-trace.git/include/trace/events/irq.h
> ===================================================================
> --- linux-trace.git.orig/include/trace/events/irq.h	2010-02-12 13:29:44.000000000 -0500
> +++ linux-trace.git/include/trace/events/irq.h	2010-02-12 13:33:20.000000000 -0500
> @@ -7,6 +7,17 @@
>  #include <linux/tracepoint.h>
>  #include <linux/interrupt.h>
>  
> +EXTRACT_TRACE_SYMBOL(HI_SOFTIRQ);
> +EXTRACT_TRACE_SYMBOL(TIMER_SOFTIRQ);
> +EXTRACT_TRACE_SYMBOL(NET_TX_SOFTIRQ);
> +EXTRACT_TRACE_SYMBOL(NET_RX_SOFTIRQ);
> +EXTRACT_TRACE_SYMBOL(BLOCK_SOFTIRQ);
> +EXTRACT_TRACE_SYMBOL(BLOCK_IOPOLL_SOFTIRQ);
> +EXTRACT_TRACE_SYMBOL(TASKLET_SOFTIRQ);
> +EXTRACT_TRACE_SYMBOL(SCHED_SOFTIRQ);
> +EXTRACT_TRACE_SYMBOL(HRTIMER_SOFTIRQ);
> +EXTRACT_TRACE_SYMBOL(RCU_SOFTIRQ);
> +

Still sucks you have to explicitly iterate them all, and far away from
the actual definition site too, its just asking to get out of whack with
reality.

Re: [PATCH 2/2][RFC] tracing: Add extract out softirq names used by irq trace events

[Steven Rostedt]

On Sat, 2010-02-13 at 11:39 +0100, Peter Zijlstra wrote:
> On Fri, 2010-02-12 at 14:09 -0500, Steven Rostedt wrote:
> > plain text document attachment (irq-events.patch)
> > The irq trace events that map the softirq vectors to strings
> > shows up in the format files as names. To allow binary parsers to
> > be able to convert these names to their actual numbers, this patch
> > extracts those enums.
> > 
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> > 
> > ---
> >  include/trace/events/irq.h |   11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > Index: linux-trace.git/include/trace/events/irq.h
> > ===================================================================
> > --- linux-trace.git.orig/include/trace/events/irq.h	2010-02-12 13:29:44.000000000 -0500
> > +++ linux-trace.git/include/trace/events/irq.h	2010-02-12 13:33:20.000000000 -0500
> > @@ -7,6 +7,17 @@
> >  #include <linux/tracepoint.h>
> >  #include <linux/interrupt.h>
> >  
> > +EXTRACT_TRACE_SYMBOL(HI_SOFTIRQ);
> > +EXTRACT_TRACE_SYMBOL(TIMER_SOFTIRQ);
> > +EXTRACT_TRACE_SYMBOL(NET_TX_SOFTIRQ);
> > +EXTRACT_TRACE_SYMBOL(NET_RX_SOFTIRQ);
> > +EXTRACT_TRACE_SYMBOL(BLOCK_SOFTIRQ);
> > +EXTRACT_TRACE_SYMBOL(BLOCK_IOPOLL_SOFTIRQ);
> > +EXTRACT_TRACE_SYMBOL(TASKLET_SOFTIRQ);
> > +EXTRACT_TRACE_SYMBOL(SCHED_SOFTIRQ);
> > +EXTRACT_TRACE_SYMBOL(HRTIMER_SOFTIRQ);
> > +EXTRACT_TRACE_SYMBOL(RCU_SOFTIRQ);
> > +
> 
> Still sucks you have to explicitly iterate them all, and far away from
> the actual definition site too, its just asking to get out of whack with
> reality.

We don't care about adding them near the definition site. We care about
what is used. In this same file we have:

#define softirq_name(sirq) { sirq##_SOFTIRQ, #sirq }
#define show_softirq_name(val)                          \
        __print_symbolic(val,                           \
                         softirq_name(HI),              \
                         softirq_name(TIMER),           \
                         softirq_name(NET_TX),          \
                         softirq_name(NET_RX),          \
                         softirq_name(BLOCK),           \
                         softirq_name(BLOCK_IOPOLL),    \
                         softirq_name(TASKLET),         \
                         softirq_name(SCHED),           \
                         softirq_name(HRTIMER),         \
                         softirq_name(RCU))

Which uses the variables. If another softirq name is added, the
TRACE_EVENT already misses it. That's not the point of the EXTRACT
macro. The point is, if a TRACE_EVENT uses a name that is not a macro,
this gives a way for it to display that value.

-- Steve

Re: [PATCH 2/2][RFC] tracing: Add extract out softirq names used by irq trace events

[Frederic Weisbecker]

On Sat, Feb 13, 2010 at 06:11:09AM -0500, Steven Rostedt wrote:
> We don't care about adding them near the definition site. We care about
> what is used. In this same file we have:
> 
> #define softirq_name(sirq) { sirq##_SOFTIRQ, #sirq }
> #define show_softirq_name(val)                          \
>         __print_symbolic(val,                           \
>                          softirq_name(HI),              \
>                          softirq_name(TIMER),           \
>                          softirq_name(NET_TX),          \
>                          softirq_name(NET_RX),          \
>                          softirq_name(BLOCK),           \
>                          softirq_name(BLOCK_IOPOLL),    \
>                          softirq_name(TASKLET),         \
>                          softirq_name(SCHED),           \
>                          softirq_name(HRTIMER),         \
>                          softirq_name(RCU))
> 
> Which uses the variables. If another softirq name is added, the
> TRACE_EVENT already misses it. That's not the point of the EXTRACT
> macro. The point is, if a TRACE_EVENT uses a name that is not a macro,
> this gives a way for it to display that value.


And what about a kind of macro that could have two effects:

- define the enum
- define the nr -> name pairs for resolution

This could be something like:

define_trace_enum(softirq)
	enum_entry(TASKLET, 4), //don't know if it's 4, just an example
	enum_entry(HRTIMER, 5),
end_trace_enum()

(My naming sucks, as usual).

In normal headers, it would define an enum:

enum softirq {
	TASKLET = 4,
	HRTIMER = 5,
};

And in the file that has DEFINE_TRACEPOINT:

/* can be in ftrace_event.h */
struct resolve_enum {
	const char *name;
	int val;
};

struct resolve_enum softirq = { //come from define_trace_enum()
	{"TASKLET", 4}, //come from enum_entry()
	{"HRTIMER", 5},
	{ NULL, 0}, /* end */
};

/* This can be used from the trace_event macro */
const char *softirq_name(int nr)
{
	return resolve_enum[nr];
}


And then you can get back the struct resolve_enum softirq
to export the values to debugfs, may be by storing such
structures in a section (and adding the name of the enum)

This has the advantage of beeing sync with core header
changes, but this has the drawback of beeing less readable
to define an enum usable from a TRACE_EVENT (especially
if I define the naming personally).

Re: [PATCH 2/2][RFC] tracing: Add extract out softirq names used by irq trace events

[Steven Rostedt]

On Fri, 2010-02-26 at 00:48 +0100, Frederic Weisbecker wrote:

> 
> And what about a kind of macro that could have two effects:
> 
> - define the enum
> - define the nr -> name pairs for resolution
> 
> This could be something like:
> 
> define_trace_enum(softirq)
> 	enum_entry(TASKLET, 4), //don't know if it's 4, just an example
> 	enum_entry(HRTIMER, 5),
> end_trace_enum()

That wont work. Unless it is in a separate file that can be included
over and over again. That is we would need:

** include/linux/softirq_names.h:

define_trace_enum(softirq)
	enum_entry(HI, 0),
	enum_entry(TIMER, 1),
	[...]
end_trace_enum();



** include/trace/define_enum.h:

#define define_trace_enum(name)  enum name {
#define enum_entry(a, b)  a = b
#define end_trace_enum()  }


** include/linux/interrupt.h

/* instead of declaring an enum we have */

#include <trace/define_enum.h>
#include <linux/softirq_names.h>

Here we could do something special with that file.

> 
> (My naming sucks, as usual).
> 
> In normal headers, it would define an enum:
> 
> enum softirq {
> 	TASKLET = 4,
> 	HRTIMER = 5,
> };
> 
> And in the file that has DEFINE_TRACEPOINT:
> 
> /* can be in ftrace_event.h */
> struct resolve_enum {
> 	const char *name;
> 	int val;
> };
> 
> struct resolve_enum softirq = { //come from define_trace_enum()
> 	{"TASKLET", 4}, //come from enum_entry()
> 	{"HRTIMER", 5},
> 	{ NULL, 0}, /* end */
> };
> 
> /* This can be used from the trace_event macro */
> const char *softirq_name(int nr)
> {
> 	return resolve_enum[nr];
> }
> 
> 
> And then you can get back the struct resolve_enum softirq
> to export the values to debugfs, may be by storing such
> structures in a section (and adding the name of the enum)
> 
> This has the advantage of beeing sync with core header
> changes, but this has the drawback of beeing less readable
> to define an enum usable from a TRACE_EVENT (especially
> if I define the naming personally).
> 


This just gets ugly and I'm not sure people would like doing this. It
also requires that you number the enums specifically.

A better way could be this:

** include/linux/interrupt.h

#define SOFTIRQ_NAMES 	\
	C(HI),		\
	C(TIMER),	\
	[...]		\
	C(RCU)

#define C(name)		name##_SOFTIRQ

enum { SOFTIRQ_NAMES, NR_SOFTIRQS };

#undef C

And in the trace file we could do:

#define C(name)  { #name, name##_SOFTIRQ, sizeof(name##_SOFTIRQ) }

struct {
	char *name;
	int val;
	int size;
} softirq_names[] __attribute__((section("trace_syms"))) =
  { SOFTIRQ_NAMES };

#undef C


This way we don't need to number every enum, or have to move the enum
into another file. My question is...

Would the following be acceptable in normal headers?

#define ENUM_NAMES  	\
	C(a),		\
	C(b),		\
	...

#define C(name) name

enum { ENUM_NAMES };

#undef C

-- Steve

Re: [PATCH 2/2][RFC] tracing: Add extract out softirq names used by irq trace events

[Frederic Weisbecker]

On Thu, Feb 25, 2010 at 09:26:19PM -0500, Steven Rostedt wrote:
> On Fri, 2010-02-26 at 00:48 +0100, Frederic Weisbecker wrote:
> 
> > 
> > And what about a kind of macro that could have two effects:
> > 
> > - define the enum
> > - define the nr -> name pairs for resolution
> > 
> > This could be something like:
> > 
> > define_trace_enum(softirq)
> > 	enum_entry(TASKLET, 4), //don't know if it's 4, just an example
> > 	enum_entry(HRTIMER, 5),
> > end_trace_enum()
> 
> That wont work. Unless it is in a separate file that can be included
> over and over again. That is we would need:
> 
> ** include/linux/softirq_names.h:
> 
> define_trace_enum(softirq)
> 	enum_entry(HI, 0),
> 	enum_entry(TIMER, 1),
> 	[...]
> end_trace_enum();
> 
> 
> 
> ** include/trace/define_enum.h:
> 
> #define define_trace_enum(name)  enum name {
> #define enum_entry(a, b)  a = b
> #define end_trace_enum()  }
> 
> 
> ** include/linux/interrupt.h
> 
> /* instead of declaring an enum we have */
> 
> #include <trace/define_enum.h>
> #include <linux/softirq_names.h>
> 
> Here we could do something special with that file.



Right.



> 
> > 
> > (My naming sucks, as usual).
> > 
> > In normal headers, it would define an enum:
> > 
> > enum softirq {
> > 	TASKLET = 4,
> > 	HRTIMER = 5,
> > };
> > 
> > And in the file that has DEFINE_TRACEPOINT:
> > 
> > /* can be in ftrace_event.h */
> > struct resolve_enum {
> > 	const char *name;
> > 	int val;
> > };
> > 
> > struct resolve_enum softirq = { //come from define_trace_enum()
> > 	{"TASKLET", 4}, //come from enum_entry()
> > 	{"HRTIMER", 5},
> > 	{ NULL, 0}, /* end */
> > };
> > 
> > /* This can be used from the trace_event macro */
> > const char *softirq_name(int nr)
> > {
> > 	return resolve_enum[nr];
> > }
> > 
> > 
> > And then you can get back the struct resolve_enum softirq
> > to export the values to debugfs, may be by storing such
> > structures in a section (and adding the name of the enum)
> > 
> > This has the advantage of beeing sync with core header
> > changes, but this has the drawback of beeing less readable
> > to define an enum usable from a TRACE_EVENT (especially
> > if I define the naming personally).
> > 
> 
> 
> This just gets ugly and I'm not sure people would like doing this. It
> also requires that you number the enums specifically.


Yeah :-(



> 
> A better way could be this:
> 
> ** include/linux/interrupt.h
> 
> #define SOFTIRQ_NAMES 	\
> 	C(HI),		\
> 	C(TIMER),	\
> 	[...]		\
> 	C(RCU)
> 
> #define C(name)		name##_SOFTIRQ
> 
> enum { SOFTIRQ_NAMES, NR_SOFTIRQS };
> 
> #undef C
> 
> And in the trace file we could do:
> 
> #define C(name)  { #name, name##_SOFTIRQ, sizeof(name##_SOFTIRQ) }
> 
> struct {
> 	char *name;
> 	int val;
> 	int size;
> } softirq_names[] __attribute__((section("trace_syms"))) =
>   { SOFTIRQ_NAMES };
> 
> #undef C
> 
> 
> This way we don't need to number every enum, or have to move the enum
> into another file. My question is...
> 
> Would the following be acceptable in normal headers?
> 
> #define ENUM_NAMES  	\
> 	C(a),		\
> 	C(b),		\
> 	...
> 
> #define C(name) name
> 
> enum { ENUM_NAMES };
> 
> #undef C



Could be acceptable, this should only concern rare enums I
guess.



> -- Steve
> 
>