lkml.org 
[lkml]   [2010]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] sched_rt: Removes extra checking for nr_cpus_allowed when calling find_lowest_rq
From
Date
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@twins>, Peter
> Zijlstra <peterz@infradead.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




\
 
 \ /
  Last update: 2010-10-19 15:03    [from the cache]
©2003-2011 Jasper Spaans