Re: [Patch] numa:x86_64: Cacheline aliasing makes for_each_populated_zone extremely expensive -V2.

["H. Peter Anvin" <hpa@zytor.com>]

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