linux-kernel - Re: [PATCH] sched_rt: Removes extra checking for nr_cpus_allowed when calling find_lowest

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: <4CBD5FC90200005A00074945@soto.provo.novell.com>
Date:	Tue, 19 Oct 2010 07:07:21 -0600
From:	"Gregory Haskins" <ghaskins@...ell.com>
To:	"Steven Rostedt" <rostedt@...dmis.org>
Cc:	"Ingo Molnar" <mingo@...e.hu>,
	"Rakib Mullick" <rakib.mullick@...il.com>,
	"Peter Zijlstra" <peterz@...radead.org>,
	"LKML" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] sched_rt: Removes extra checking for
 nr_cpus_allowed when calling find_lowest_rq

>>> On 10/19/2010 at 09:01 AM, in message
<1287493260.16971.364.camel@...dalf.stny.rr.com>, Steven Rostedt
<rostedt@...dmis.org> wrote: 
> On Tue, 2010-10-19 at 06:57 -0600, Gregory Haskins wrote:
>> >>> On 10/19/2010 at 07:02 AM, in message <1287486167.1994.1.camel@...ns>, Peter
>> Zijlstra <peterz@...radead.org> wrote: 
>> > On Tue, 2010-10-19 at 16:57 +0600, Rakib Mullick wrote:
>> 
>> [snip]
>> 
>> >> --- linus-rc8/kernel/sched_rt.c	2010-10-15 05:26:43.000000000 +0600
>> >> +++ rakib-rc8/kernel/sched_rt.c	2010-10-19 16:22:30.000000000 +0600
>> >> @@ -971,8 +971,7 @@ select_task_rq_rt(struct rq *rq, struct
>> >>  	 * that is just being woken and probably will have
>> >>  	 * cold cache anyway.
>> >>  	 */
>> >> -	if (unlikely(rt_task(rq->curr)) &&
>> >> -	    (p->rt.nr_cpus_allowed > 1)) {
>> 
>> I think the motivation here was that checking nr_cpus_allowed is far
>> cheaper than taking the hit on a function call in this particularly
>> hot path.  As Steven points out in a follow-up reply, the function
>> call has additional overhead before the equivalent check is made
>> again.  We could possibly optimize this with some of the suggestions
>> he made, but I am not sure if it is worth it (alone) as the call
>> overhead would still be present.  OTOH, the cases where
>> nr_cpus_allowed <= 1 are probably rare in the grand scheme of things.
>> 
>> My opinion is the check should probably remain (if but perhaps with a
>> comment to explain its existence) unless someone (Rakib, hint hint) is
>> willing to do some benchmarking to demonstrate that it doesn't
>> actually have any positive impact.  It probably also makes sense to
>> take Steve's suggested changes to improve the places that use the
>> function without external optimization.
> 
> Yeah, it probably is not worth removing the check here, as a function
> call will add overhead.
> 
> And do not think that it is a unlikely case to have an RT task pinned to
> a CPU. In true RT systems, that should be the norm. Any benchmark should
> test the impact on tasks that are pinned to a CPU, not a general
> scenario.
> 

Yes, agreed and excellent point, Steve.

-Greg


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