Re: [PATCH] sched: Provide iowait counters

[Peter Zijlstra <peterz@infradead.org>]

On Mon, 2009-07-20 at 16:26 -0400, Steven Rostedt wrote:
> > +#define _TRACE_PROFILE(call, proto, args, assign)                    \
> >  static void ftrace_profile_##call(proto)                             \
> >  {                                                                    \
> > -     extern void perf_tpcounter_event(int);                          \
> > -     perf_tpcounter_event(event_##call.id);                          \
> > +     extern void perf_tpcounter_event(int, u64, u64);                \
> > +     u64 addr = 0, count = 1;                                        \
> > +     { assign; }                                                     \
> > +     perf_tpcounter_event(event_##call.id, addr, count);             \
> 
> The problem here is that the assign also will do the :
> 
>        TP_fast_assign(
>                __entry->pid    = p->pid;
>                __entry->time   = time;
> 
> part, thus you will probably get errors in processing the __entry part.

Oh, right, no macros wrapping that..

> What about doing instead:
> 
>  TRACE_EVENT(sched_iowait,
>  
>         TP_PROTO(struct task_struct *p, u64 time),
>  
>         TP_ARGS(p, time),
>  
>         TP_STRUCT__entry(
>                 __field(pid_t, pid )
>                 __field(u64,   time)
>         ),
>  
>         TP_fast_assign(
>                 __entry->pid    = p->pid;
>                 __entry->time   = time;
>  
>         ) __perf_count(time),
> 
>         TP_printk("task %d waited for IO for %Lu ns",
>                 __entry->pid, __entry->time)
>  );
>  
> Then we could simply do:
> 
> #undef __perf_count
> #define __perf_count(a)
> 
> [ do all the ftrace event work ]
> 
> #undef TP_fast_assign
> #define TP_fast_assign(a...)
> 
> #undef __perf_count
> #define __perf_count(c) count = (c);

Hmm, should we maybe do something like:

	TP_fast_assign(
		...
	)
	TP_perf_assign(
		addr = 0;
		count = time;
	),

So we can extend it over time?

But otherwise, yes, looks nice. Lets make it happen ;-)