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

[Frederic Weisbecker <fweisbec@gmail.com>]

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
> 
>