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: <1287493260.16971.364.camel@gandalf.stny.rr.com>
Date:	Tue, 19 Oct 2010 09:01:00 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Gregory Haskins <ghaskins@...ell.com>
Cc:	Rakib Mullick <rakib.mullick@...il.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...e.hu>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] sched_rt: Removes extra checking for nr_cpus_allowed
 when calling find_lowest_rq

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.

-- Steve


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