[Skiboot] [PATCH 2/2] memory: Sort memory regions list

[Vasant Hegde]

local_alloc() function tries to find first allocatable local region and
tries to allocate memory. If that region doesn't have sufficient memory to
allocate then it will log below warning and tries to allocate memory from
next available region.

Warning:
--------
[  268.346728058,3] mem_alloc(0x800000, 0x800000, "hw/xive.c:1630", ibm,firmware-allocs-memory at 0) failed !
[  268.346732718,6] Memory regions:
[  268.346734353,6]   0x000030500000..000030ffffff : ibm,firmware-heap
[  268.346833468,5]       420 allocs of 0x00000058 bytes at core/device.c:41 (total 0x9060)
[  268.346978805,5]      2965 allocs of 0x00000038 bytes at core/device.c:424 (total 0x28898)
[  268.347035614,5]       434 allocs of 0x00000040 bytes at core/device.c:424 (total 0x6c80)
[  268.347093567,5]       365 allocs of 0x00000028 bytes at libc/string/strdup.c:23 (total 0x3908)
[  268.347136818,5]        84 allocs of 0x00000048 bytes at core/device.c:424 (total 0x17a0)
[  268.347179123,5]        21 allocs of 0x00000030 bytes at libc/string/strdup.c:23 (total 0x3f0)
....
....

Hostboot reserves memory for various nodes and passes this information via HDAT.
In some cases there will be small memory holes between two reservations
(ex: 16MB free space between two reservation).

add_region() function adds new region to the head of regions list.
mem_region_init() adds OPAL regions first and then hostboot regions. So
these smaller regions will be added to head of list. If we have smaller
free regions then we may hit above warning.

Hostboot uses top of the memory for various reservations. So if we sort
memory regions then allocator will use bigger region (region after OPAL
memory) for local allocation. And we will not hit above warning.

Memory region layout with this patch:
  0     -  756MB : OS reserved region (for loading kernel)
  756MB - ~856MB : OPAL memory (actual size depends on PIR)
  856MB - ~956MB : Memory for MPIPL (actual size depends on OPAL runtime size)
  956MB - ...    : We will have free memory after 956MB which we can use
                   for local_alloc(). Typically this will be multiple GBs.
		   So it works fine.
  ....  - top_mem: Hostboot reservations + small holes

Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
---
 core/mem_region.c                             | 20 ++++++++++++++++---
 core/test/run-mem_region_next.c               |  4 ++--
 .../run-mem_region_release_unused_noalloc.c   |  2 +-
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/core/mem_region.c b/core/mem_region.c
index eb48a1a11..003a91812 100644
--- a/core/mem_region.c
+++ b/core/mem_region.c
@@ -734,7 +734,7 @@ static bool maybe_split(struct mem_region *r, uint64_t split_at)
 		return false;
 
 	/* Tail add is important: we may need to split again! */
-	list_add_tail(&regions, &tail->list);
+	list_add_after(&regions, &tail->list, &r->list);
 	return true;
 }
 
@@ -763,6 +763,20 @@ static struct mem_region *get_overlap(const struct mem_region *region)
 	return NULL;
 }
 
+static void add_region_to_regions(struct mem_region *region)
+{
+	struct mem_region *r;
+
+	list_for_each(&regions, r, list) {
+		if (r->start < region->start)
+			continue;
+
+		list_add_before(&regions, &region->list, &r->list);
+		return;
+	}
+	list_add_tail(&regions, &region->list);
+}
+
 static bool add_region(struct mem_region *region)
 {
 	struct mem_region *r;
@@ -807,7 +821,7 @@ static bool add_region(struct mem_region *region)
 	}
 
 	/* Finally, add in our own region. */
-	list_add(&regions, &region->list);
+	add_region_to_regions(region);
 	return true;
 }
 
@@ -1091,7 +1105,7 @@ void mem_region_init(void)
 			prerror("MEM: Could not add mem region %s!\n", i->name);
 			abort();
 		}
-		list_add(&regions, &region->list);
+		add_region_to_regions(region);
 		if ((start + len) > top_of_ram)
 			top_of_ram = start + len;
 		unlock(&mem_region_lock);
diff --git a/core/test/run-mem_region_next.c b/core/test/run-mem_region_next.c
index b14e41d07..bab214f74 100644
--- a/core/test/run-mem_region_next.c
+++ b/core/test/run-mem_region_next.c
@@ -85,13 +85,13 @@ int main(void)
 
 	r = mem_region_next(NULL);
 	assert(r);
-	assert(r->start == 0x2000);
+	assert(r->start == 0x1000);
 	assert(r->len == 0x1000);
 	assert(r->type == REGION_RESERVED);
 
 	r = mem_region_next(r);
 	assert(r);
-	assert(r->start == 0x1000);
+	assert(r->start == 0x2000);
 	assert(r->len == 0x1000);
 	assert(r->type == REGION_RESERVED);
 
diff --git a/core/test/run-mem_region_release_unused_noalloc.c b/core/test/run-mem_region_release_unused_noalloc.c
index aa8c23a26..5c4b1ddce 100644
--- a/core/test/run-mem_region_release_unused_noalloc.c
+++ b/core/test/run-mem_region_release_unused_noalloc.c
@@ -147,8 +147,8 @@ int main(void)
 			 * only in test and only for valgrind
 			 */
 			free((void*)r->name);
+			last = r->name;
 		}
-		last = r->name;
 	}
 
 	dt_free(dt_root);
-- 
2.21.1