Messages in this thread | | | Subject | Re: [PATCH] sched_rt: Removes extra checking for nr_cpus_allowed when calling find_lowest_rq | From | Steven Rostedt <> | Date | Tue, 19 Oct 2010 09:01:00 -0400 |
| |
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
| |