linux-kernel - Re: [PATCH 08/14] sched: add latency tracing for -deadline tasks.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20131127091647.4e16ce53@gandalf.local.home>
Date:	Wed, 27 Nov 2013 09:16:47 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Juri Lelli <juri.lelli@...il.com>
Cc:	peterz@...radead.org, tglx@...utronix.de, mingo@...hat.com,
	oleg@...hat.com, fweisbec@...il.com, darren@...art.com,
	johan.eker@...csson.com, p.faure@...tech.ch,
	linux-kernel@...r.kernel.org, claudio@...dence.eu.com,
	michael@...rulasolutions.com, fchecconi@...il.com,
	tommaso.cucinotta@...up.it, nicola.manica@...i.unitn.it,
	luca.abeni@...tn.it, dhaval.giani@...il.com, hgu1972@...il.com,
	paulmck@...ux.vnet.ibm.com, raistlin@...ux.it,
	insop.song@...il.com, liming.wang@...driver.com, jkacur@...hat.com,
	harald.gustafsson@...csson.com, vincent.guittot@...aro.org,
	bruce.ashfield@...driver.com
Subject: Re: [PATCH 08/14] sched: add latency tracing for -deadline tasks.

On Wed, 27 Nov 2013 14:43:45 +0100
Juri Lelli <juri.lelli@...il.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.

>  	};
>  	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.

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

-- Steve

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/