Messages in this thread | | | Date | Wed, 27 Nov 2013 15:19:43 +0100 | From | Juri Lelli <> | Subject | Re: [PATCH 08/14] sched: add latency tracing for -deadline tasks. |
| |
On 11/27/2013 03:16 PM, Steven Rostedt wrote: > On Wed, 27 Nov 2013 14:43:45 +0100 > Juri Lelli <juri.lelli@gmail.com> wrote: > diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c >> index f76f8d6..ad94604 100644 >> --- a/kernel/trace/trace_selftest.c >> +++ b/kernel/trace/trace_selftest.c >> @@ -1023,16 +1023,16 @@ trace_selftest_startup_nop(struct tracer *trace, struct trace_array *tr) >> static int trace_wakeup_test_thread(void *data) >> { >> /* Make this a -deadline thread */ >> - struct sched_param2 paramx = { >> + static const struct sched_param2 param = { >> .sched_priority = 0, >> + .sched_flags = 0, >> .sched_runtime = 100000ULL, >> .sched_deadline = 10000000ULL, >> .sched_period = 10000000ULL >> - .sched_flags = 0 > > Assigning structures like this, you don't need to set the zero fields. > all fields not explicitly stated, are set to zero. >
Right.
>> }; >> struct completion *x = data; >> >> >> -------------------------------- >> kernel/trace/trace_sched_wakeup.c | 28 ++++++++++++++++++++++++---- >> 1 file changed, 24 insertions(+), 4 deletions(-) >> >> diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c >> index 1457fb1..090c4d9 100644 >> --- a/kernel/trace/trace_sched_wakeup.c >> +++ b/kernel/trace/trace_sched_wakeup.c >> @@ -28,6 +28,7 @@ static int wakeup_current_cpu; >> static unsigned wakeup_prio = -1; >> static int wakeup_rt; >> static int wakeup_dl; >> +static int tracing_dl = 0; > > Get rid of the ' = 0', its implicit to all static and global variables > that are not given any value. >
And right.
>> >> static arch_spinlock_t wakeup_lock = >> (arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED; >> @@ -438,6 +439,7 @@ static void __wakeup_reset(struct trace_array *tr) >> { >> wakeup_cpu = -1; >> wakeup_prio = -1; >> + tracing_dl = 0; >> >> if (wakeup_task) >> put_task_struct(wakeup_task); >> @@ -481,9 +483,9 @@ probe_wakeup(void *ignore, struct task_struct *p, int success) >> * sched_rt class; >> * - wakeup_dl handles tasks belonging to sched_dl class only. >> */ >> - if ((wakeup_dl && !dl_task(p)) || >> + if (tracing_dl || (wakeup_dl && !dl_task(p)) || >> (wakeup_rt && !dl_task(p) && !rt_task(p)) || >> - (p->prio >= wakeup_prio || p->prio >= current->prio)) >> + (!dl_task(p) && (p->prio >= wakeup_prio || p->prio >= current->prio))) >> return; >> >> pc = preempt_count(); >> @@ -495,7 +497,8 @@ probe_wakeup(void *ignore, struct task_struct *p, int success) >> arch_spin_lock(&wakeup_lock); >> >> /* check for races. */ >> - if (!tracer_enabled || (!dl_task(p) && p->prio >= wakeup_prio)) >> + if (!tracer_enabled || tracing_dl || >> + (!dl_task(p) && p->prio >= wakeup_prio)) >> goto out_locked; >> >> /* reset the trace */ >> @@ -505,6 +508,15 @@ probe_wakeup(void *ignore, struct task_struct *p, int success) >> wakeup_current_cpu = wakeup_cpu; >> wakeup_prio = p->prio; >> >> + /* >> + * Once you start tracing a -deadline task, don't bother tracing >> + * another task until the first one wakes up. >> + */ >> + if (dl_task(p)) >> + tracing_dl = 1; >> + else >> + tracing_dl = 0; > > Do we need the else statement? I would think the only way to get here > is if tracing_dl is already set to zero. >
No, indeed.
Thanks,
- Juri
>> + >> wakeup_task = p; >> get_task_struct(wakeup_task); >> >> @@ -700,10 +712,18 @@ static struct tracer wakeup_dl_tracer __read_mostly = >> .start = wakeup_tracer_start, >> .stop = wakeup_tracer_stop, >> .wait_pipe = poll_wait_pipe, >> - .print_max = 1, >> + .print_max = true, >> + .print_header = wakeup_print_header, >> + .print_line = wakeup_print_line, >> + .flags = &tracer_flags, >> + .set_flag = wakeup_set_flag, >> + .flag_changed = wakeup_flag_changed, >> #ifdef CONFIG_FTRACE_SELFTEST >> .selftest = trace_selftest_startup_wakeup, >> #endif >> + .open = wakeup_trace_open, >> + .close = wakeup_trace_close, >> + .use_max_tr = true, >> }; >> >> __init static int init_wakeup_tracer(void) >> ----------------------------------- >> >> Makes sense? :) >> >> Thanks, >> >> - Juri >
| |