Messages in this thread | | | Date | Thu, 19 Nov 2015 14:39:35 +0000 | From | Javi Merino <> | Subject | Re: [PATCH 4/4] sched: add trace event for idle injection |
| |
[ CCing Steve Rostedt for the tracing bits ]
On Fri, Nov 13, 2015 at 11:53:07AM -0800, Jacob Pan wrote: > Trace events for idle injection can be used to determine > timer activities for checking synchronization. In addition they > also helps to determine when the runqueue is throttled. > > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > --- > include/linux/sched.h | 5 +++++ > include/trace/events/sched.h | 25 +++++++++++++++++++++++++ > kernel/sched/fair.c | 3 +++ > 3 files changed, 33 insertions(+) > > diff --git a/include/linux/sched.h b/include/linux/sched.h > index ff551a3..99c79bf 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -3189,6 +3189,11 @@ extern int proc_sched_cfs_idle_inject_duration_handler(struct ctl_table *table, > int write, > void __user *buffer, > size_t *length, loff_t *ppos); > +enum cfs_idle_inject_action { > + CFS_IDLE_INJECT_TIMER, /* timer sync point */ > + CFS_IDLE_INJECT_FORCED, /* idle forced in rq */ > + CFS_IDLE_INJECT_YIELD_SOFTIRQ /* yield to pending softirq */ > +}; > #endif > > #endif > diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h > index 539d6bc..52c11c1 100644 > --- a/include/trace/events/sched.h > +++ b/include/trace/events/sched.h > @@ -566,6 +566,31 @@ TRACE_EVENT(sched_wake_idle_without_ipi, > > TP_printk("cpu=%d", __entry->cpu) > ); > + > +#ifdef CONFIG_CFS_IDLE_INJECT > +/* > + * Tracepoint for idle injection > + */ > +TRACE_EVENT(sched_cfs_idle_inject, > + > + TP_PROTO(enum cfs_idle_inject_action action, int throttled), > + > + TP_ARGS(action, throttled), > + > + TP_STRUCT__entry( > + __field(enum cfs_idle_inject_action, action) > + __field(int, throttled) > + ), > + > + TP_fast_assign( > + __entry->action = action; > + __entry->throttled = throttled; > + ), > + > + TP_printk("action:%d throttled:%d", __entry->action, __entry->throttled) > +); > +#endif > +
One minor nit: can you use key=value (i.e. "throttled=%d") instead for consistency with the rest of this file?
Other than that, I know that Peter suggested an enum for the action, but wouldn't it be better to create an EVENT_CLASS and subclass the three actions from it? Something like:
DECLARE_EVENT_CLASS(sched_cfs_idle_inject_template,
TP_PROTO(int throttled),
TP_ARGS(throttled),
TP_STRUCT__entry( __field(int, throttled) ),
TP_fast_assign( __entry->throttled = throttled; ),
TP_printk("throttled=%d", __entry->throttled) );
DEFINE_EVENT(sched_cfs_idle_inject_template, sched_cfs_idle_inject_timer, TP_PROTO(int throttled), TP_ARGS(throttled));
DEFINE_EVENT(sched_cfs_idle_inject_template, sched_cfs_idle_inject_forced, TP_PROTO(int throttled), TP_ARGS(throttled));
DEFINE_EVENT(sched_cfs_idle_inject_template, sched_cfs_idle_inject_yield_softirq, TP_PROTO(int throttled), TP_ARGS(throttled));
> #endif /* _TRACE_SCHED_H */ > > /* This part must be outside protection */ > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index a0cd777..20027eb 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -5248,8 +5248,10 @@ idle: > if (in_forced_idle(cfs_rq)) { > if (unlikely(local_softirq_pending())) { > __unthrottle_cfs_rq(cfs_rq); > + trace_sched_cfs_idle_inject(CFS_IDLE_INJECT_YIELD_SOFTIRQ, 1);
This becomes:
trace_sched_cfs_idle_inject_yield_softirq(1);
> goto again; > } > + trace_sched_cfs_idle_inject(CFS_IDLE_INJECT_FORCED, 1);
This becomes: trace_sched_cfs_idle_inject_forced(1);
> return NULL; > } > /* > @@ -8432,6 +8434,7 @@ static enum hrtimer_restart idle_inject_timer_fn(struct hrtimer *hrtimer) > throttle_rq(cpu); > } > raw_cpu_write(idle_injected, !status); > + trace_sched_cfs_idle_inject(CFS_IDLE_INJECT_TIMER, !status);
and this would be:
trace_sched_cfs_idle_inject_timer(!status);
> > return HRTIMER_RESTART; > }
Cheers, Javi
| |