[ 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 <[email protected]>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
