On Wed, Oct 24, 2012 at 09:47:47AM -0600, Mike Yoknis wrote:

On Sat, 2012-10-20 at 09:29 +0100, Mel Gorman wrote:

quoted

On Fri, Oct 19, 2012 at 01:53:18PM -0600, Mike Yoknis wrote:

quoted

On Tue, 2012-10-09 at 08:56 -0600, Mike Yoknis wrote:

quoted

On Mon, 2012-10-08 at 16:16 +0100, Mel Gorman wrote:

quoted

On Wed, Oct 03, 2012 at 08:56:14AM -0600, Mike Yoknis wrote:

quoted

memmap_init_zone() loops through every Page Frame Number (pfn),
including pfn values that are within the gaps between existing
memory sections.  The unneeded looping will become a boot
performance issue when machines configure larger memory ranges
that will contain larger and more numerous gaps.

The code will skip across invalid sections to reduce the
number of loops executed.

Signed-off-by: Mike Yoknis <redacted>

I do not see the need for
the additional complexity unless you can show it makes a big difference
to boot times.

Mel,

Let me pass along the numbers I have.  We have what we call an
"architectural simulator".  It is a computer program that pretends that
it is a computer system.  We use it to test the firmware before real
hardware is available.  We have booted Linux on our simulator.  As you
would expect it takes longer to boot on the simulator than it does on
real hardware.

With my patch - boot time 41 minutes
Without patch - boot time 94 minutes

These numbers do not scale linearly to real hardware.  But indicate to
me a place where Linux can be improved.

Mike Yoknis

Mel,
I finally got access to prototype hardware.  
It is a relatively small machine with only 64GB of RAM.
 
I put in a time measurement by reading the TSC register.
I booted both with and without my patch -
 
Without patch -
[    0.000000]   Normal zone: 13400064 pages, LIFO batch:31
[    0.000000] memmap_init_zone() enter 1404184834218
[    0.000000] memmap_init_zone() exit  1411174884438  diff = 6990050220
 
With patch -
[    0.000000]   Normal zone: 13400064 pages, LIFO batch:31
[    0.000000] memmap_init_zone() enter 1555530050778
[    0.000000] memmap_init_zone() exit  1559379204643  diff = 3849153865
 
This shows that without the patch the routine spends 45% 
of its time spinning unnecessarily.
 

I'm travelling at the moment so apologies that I have not followed up on
this. My problem is still the same with the patch - it changes more
headers than is necessary and it is sparsemem specific. At minimum, try
the suggestion of 

if (!early_pfn_valid(pfn)) {
      pfn = ALIGN(pfn + MAX_ORDER_NR_PAGES, MAX_ORDER_NR_PAGES) - 1;
      continue;
}

and see how much it gains you as it should work on all memory models. If
it turns out that you really need to skip whole sections then the strice
could MAX_ORDER_NR_PAGES on all memory models except sparsemem where the
stride would be PAGES_PER_SECTION

Mel,
I tried your suggestion.  I re-ran all 3 methods on our latest firmware.

The following are TSC difference numbers (*10^6) to execute
memmap_init_zone() -

No patch   - 7010
Mel's patch- 3918
My patch   - 3847

The incremental improvement of my method is not significant vs. yours.

If you believe your suggested change is worthwhile I will create a v2
patch.

I think it is a reasonable change and I prefer my suggestion because it
should work for all memory models. Please do a V2 of the patch. I'm still
travelling at the moment (writing this from an airport) but I'll be back
online next Tuesday and will review it when I can.

-- 
Mel Gorman
SUSE Labs