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: <4F868156.30708@gmail.com>
Date:	Thu, 12 Apr 2012 09:16:38 +0200
From:	Juri Lelli <juri.lelli@...il.com>
To:	Steven Rostedt <rostedt@...dmis.org>
CC:	peterz@...radead.org, tglx@...utronix.de, mingo@...hat.com,
	cfriesen@...tel.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@...csson.com, liming.wang@...driver.com
Subject: Re: [PATCH 11/16] sched: add latency tracing for -deadline tasks.

On 04/11/2012 11:03 PM, Steven Rostedt wrote:
> On Fri, 2012-04-06 at 09:14 +0200, Juri Lelli wrote:
>> From: Dario Faggioli<raistlin@...ux.it>
>>
>> It is very likely that systems that wants/needs to use the new
>> SCHED_DEADLINE policy also want to have the scheduling latency of
>> the -deadline tasks under control.
>>
>> For this reason a new version of the scheduling wakeup latency,
>> called "wakeup_dl", is introduced.
>>
>> As a consequence of applying this patch there will be three wakeup
>> latency tracer:
>>   * "wakeup", that deals with all tasks in the system;
>>   * "wakeup_rt", that deals with -rt and -deadline tasks only;
>>   * "wakeup_dl", that deals with -deadline tasks only.
>>
>> Signed-off-by: Dario Faggioli<raistlin@...ux.it>
>> Signed-off-by: Juri Lelli<juri.lelli@...il.com>
>> ---
>>   kernel/trace/trace_sched_wakeup.c |   41 ++++++++++++++++++++++++++++++++++++-
>>   kernel/trace/trace_selftest.c     |   30 ++++++++++++++++----------
>>   2 files changed, 58 insertions(+), 13 deletions(-)
>>
>> diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
>> index e4a70c0..9c9b1be 100644
>> --- a/kernel/trace/trace_sched_wakeup.c
>> +++ b/kernel/trace/trace_sched_wakeup.c
>> @@ -27,6 +27,7 @@ static int			wakeup_cpu;
>>   static int			wakeup_current_cpu;
>>   static unsigned			wakeup_prio = -1;
>>   static int			wakeup_rt;
>> +static int			wakeup_dl;
>>
>>   static arch_spinlock_t wakeup_lock =
>>   	(arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED;
>> @@ -420,6 +421,17 @@ probe_wakeup(void *ignore, struct task_struct *p, int success)
>>   	if ((wakeup_rt&&  !rt_task(p)) ||
>>   			p->prio>= wakeup_prio ||
>>   			p->prio>= current->prio)
>
> I don't think you meant to keep both if statements. Look above and
> below ;-)
>

Ouch! Forgot to cut something! :-(
  
>> +	/*
>> +	 * Semantic is like this:
>> +	 *  - wakeup tracer handles all tasks in the system, independently
>> +	 *    from their scheduling class;
>> +	 *  - wakeup_rt tracer handles tasks belonging to sched_dl and
>> +	 *    sched_rt class;
>> +	 *  - wakeup_dl handles tasks belonging to sched_dl class only.
>> +	 */
>> +	if ((wakeup_dl&&  !dl_task(p)) ||
>> +	    (wakeup_rt&&  !dl_task(p)&&  !rt_task(p)) ||
>> +	    (p->prio>= wakeup_prio || p->prio>= current->prio))
>>   		return;
>
> Anyway, perhaps this should be broken up, as we don't want the double
> test, that is, wakeup_rt and wakeup_dl are both checked. Perhaps do:
>
> 	if (wakeup_dl&&  !dl_task(p))
> 		return;
> 	else if (wakeup_rt&&  !dl_task(p)&&  !rt_task(p))
> 		return;
>
> 	if (p->prio>= wakeup_prio || p->prio>= current->prio)
> 		return;

Yes, way better.

Thanks!

- Juri

>>
>>   	pc = preempt_count();
>> @@ -431,7 +443,7 @@ probe_wakeup(void *ignore, struct task_struct *p, int success)
>>   	arch_spin_lock(&wakeup_lock);
>>
>>   	/* check for races. */
>> -	if (!tracer_enabled || p->prio>= wakeup_prio)
>> +	if (!tracer_enabled || (!dl_task(p)&&  p->prio>= wakeup_prio))
>>   		goto out_locked;
>>
>>   	/* reset the trace */
>> @@ -539,16 +551,25 @@ static int __wakeup_tracer_init(struct trace_array *tr)
>>
>
>
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ