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]

Date:	Tue, 19 Oct 2010 20:40:00 +0600
From:	Rakib Mullick <rakib.mullick@...il.com>
To:	Gregory Haskins <ghaskins@...ell.com>
Cc:	Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...e.hu>,
	Steven Rostedt <rostedt@...dmis.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/10, Gregory Haskins <ghaskins@...ell.com> 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:
>
> 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.
>
I also think that checking nr_cpus_allowed before calling
find_lowest_rq is cheaper. We can take even cheaper way by introducing
the same check in find_lock_lowest_rq(), which also calls
find_lowest_rq. Scenario in find_lock_lowest_rq is even worse, a check
for nr_cpus_allowed (maybe even before for loop) will reduce more
overhead and removing the  nr_cpus_allowed check completely from
find_lowest_rq. And it will keep find_lowest_rq simple. We can also
made a comment whenever someone is calling find_lowest_rq, should
check nr_cpus_allowed first. (I didn't propose it in patch, cause it's
duplicate check. And it looks clean).

> 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.
>
If we made explicit check before calling find_lowest_rq, then I don't
think we need the change that Steve's suggesting. I think explicitly
checking is much more easier and removes extra overhead of function
calling.

Thanks,
Rakib
> Kind Regards,
> -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/