linux-kernel - Re: [Patch] numa:x86_64: Cacheline aliasing makes for_each_populated

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: <4C72EB41.9000002@zytor.com>
Date:	Mon, 23 Aug 2010 14:42:25 -0700
From:	"H. Peter Anvin" <hpa@...or.com>
To:	Robin Holt <holt@....com>
CC:	Jack Steiner <steiner@....com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...e.hu>, x86@...nel.org,
	Yinghai Lu <yinghai@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Joerg Roedel <joerg.roedel@....com>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Stable Maintainers <stable@...nel.org>
Subject: Re: [Patch] numa:x86_64: Cacheline aliasing makes for_each_populated_zone
 extremely expensive -V2.

On 08/21/2010 06:07 AM, Robin Holt wrote:
> 
> It does make sense.
> 
> I am not sure how to proceed.  You are saying the e820 allocator is
> being replaced.  Yet, this is the allocator used for this section of code.
> I feel sort of foolish to tweak the e820 allocator to allow for handling
> color only to have it replaced in the near future.
> 
> Add to that this simple fix is enough to break up the most egregious
> problem which is the scanning of all zones in the system and checking
> that zone's pages_present.  If this is adequate, would you accept
> this simple patch for now and place expectations on adjusting the e820
> replacement allocator later to support color with a simple patch to fix
> up the node_data allocations later?
> 

No, this isn't really the right way to go about it.

The whole point is to avoid reliance on undocumented side effects of the
specific implementations of an interface.  That means creating a new
interface with the desired semantics, instead of creating a "local"
patch which happens to work for the current implementation -- and which
will break silently when the new implementation of the interface is
created, and which means the new implementation will be seen as causing
a performance regression.

So yes, this means adding an interface to the e820 allocator, even
though it's scheduled to be replaced.  Because the new implementation
will see the new interface and know they have to implement it, and the
interface will make it clear just what exactly is expected of the
implementation.

So therefore I will not accept your "simple" patch, exactly because it
really isn't simple at all.

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