linux-kernel - Re: [PATCH 14/21] cpuidle: coupled: don't compare cpu masks unnecessarily

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: <CAKohpokmdk2xJ6OFz68Oen4YRqC5MptSn7tGmH+nZrn9hoLmbA@mail.gmail.com>
Date:	Thu, 26 Sep 2013 12:06:04 +0530
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	Colin Cross <ccross@...gle.com>
Cc:	"Rafael J. Wysocki" <rjw@...k.pl>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	"linaro-kernel@...ts.linaro.org" <linaro-kernel@...ts.linaro.org>,
	"patches@...aro.org" <patches@...aro.org>,
	Linux PM list <linux-pm@...r.kernel.org>,
	lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 14/21] cpuidle: coupled: don't compare cpu masks unnecessarily

On 26 September 2013 05:55, Colin Cross <ccross@...gle.com> wrote:
> I don't agree with this.  This patch is a tiny optimization in code
> that is rarely called, and it moves a final sanity check somewhere
> that it might get missed if the code were later refactored.

This is what we are doing for the first cpu of coupled-cpus:
if (WARN_ON(!cpumask_equal(&dev->coupled_cpus, &dev->coupled_cpus)))
    coupled->prevent++;

i.e. comparing a variable to itself :)

And I believe my patch puts the sanity check at the right place (where
we are using coupled from existing CPUs.. And that is where it should
have been since the beginning)..

If people miss this during code re-factoring, then it would be even more
stupid on the part of Author and Reviewer.. And if it still gets missed
then this is not the only place where we need to worry about such stuff..

This is present everywhere in our code.. You can't really some part of
code to some place and leave the other as-is.. The change is supposed
to be more logical and so funny mistakes must be caught during reviews.
--
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/