Re: Early boot panic on machine with lots of memory

[Yinghai Lu <yinghai@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]

On Fri, Jun 22, 2012 at 12:29 PM, Tejun Heo <tj@kernel.org> wrote:
> I wish we had a single call - say, memblock_die(), or whatever - so
> that there's a clear indication that memblock usage is done, but yeah
> maybe another day.  Will review the patch itself.  BTW, can't you post
> patches inline anymore?  Attaching is better than corrupt but is still
> a bit annoying for review.

please check the three patches:

Thanks

Yinghai

[-- Attachment #2: fix_free_memblock_reserve_v4_5.patch --]
[-- Type: application/octet-stream, Size: 6770 bytes --]

Subject: [PATCH] memblock: free allocated memblock_reserved_regions later

In memblock_free_reserved_regions, will call memblock_free(),
but memblock_free() would double reserved.regions too, so we could free
old range for reserved.regions.

Also tj said there is another bug could be related to this too.

| I don't think we're saving any noticeable
| amount by doing this "free - give it to page allocator - reserve
| again" dancing.  We should just allocate regions aligned to page
| boundaries and free them later when memblock is no longer in use.

So try to allocate that in PAGE_SIZE alignment and free that later.

-v5: Use new_alloc_size, and old_alloc_size to simplify it according to tj.

Cc: Tejun Heo <tj@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 include/linux/memblock.h |    4 ---
 mm/memblock.c            |   51 +++++++++++++++++++++--------------------------
 mm/nobootmem.c           |   36 ++++++++++++++++++++-------------
 3 files changed, 46 insertions(+), 45 deletions(-)

Index: linux-2.6/include/linux/memblock.h
===================================================================
--- linux-2.6.orig/include/linux/memblock.h
+++ linux-2.6/include/linux/memblock.h
@@ -50,9 +50,7 @@ phys_addr_t memblock_find_in_range_node(
 				phys_addr_t size, phys_addr_t align, int nid);
 phys_addr_t memblock_find_in_range(phys_addr_t start, phys_addr_t end,
 				   phys_addr_t size, phys_addr_t align);
-int memblock_free_reserved_regions(void);
-int memblock_reserve_reserved_regions(void);
-
+phys_addr_t get_allocated_memblock_reserved_regions_info(phys_addr_t *addr);
 void memblock_allow_resize(void);
 int memblock_add_node(phys_addr_t base, phys_addr_t size, int nid);
 int memblock_add(phys_addr_t base, phys_addr_t size);
Index: linux-2.6/mm/memblock.c
===================================================================
--- linux-2.6.orig/mm/memblock.c
+++ linux-2.6/mm/memblock.c
@@ -143,30 +143,6 @@ phys_addr_t __init_memblock memblock_fin
 					   MAX_NUMNODES);
 }
 
-/*
- * Free memblock.reserved.regions
- */
-int __init_memblock memblock_free_reserved_regions(void)
-{
-	if (memblock.reserved.regions == memblock_reserved_init_regions)
-		return 0;
-
-	return memblock_free(__pa(memblock.reserved.regions),
-		 sizeof(struct memblock_region) * memblock.reserved.max);
-}
-
-/*
- * Reserve memblock.reserved.regions
- */
-int __init_memblock memblock_reserve_reserved_regions(void)
-{
-	if (memblock.reserved.regions == memblock_reserved_init_regions)
-		return 0;
-
-	return memblock_reserve(__pa(memblock.reserved.regions),
-		 sizeof(struct memblock_region) * memblock.reserved.max);
-}
-
 static void __init_memblock memblock_remove_region(struct memblock_type *type, unsigned long r)
 {
 	type->total_size -= type->regions[r].size;
@@ -184,6 +160,18 @@ static void __init_memblock memblock_rem
 	}
 }
 
+phys_addr_t __init_memblock get_allocated_memblock_reserved_regions_info(
+					phys_addr_t *addr)
+{
+	if (memblock.reserved.regions == memblock_reserved_init_regions)
+		return 0;
+
+	*addr = __pa(memblock.reserved.regions);
+
+	return PAGE_ALIGN(sizeof(struct memblock_region) *
+			  memblock.reserved.max);
+}
+
 /**
  * memblock_double_array - double the size of the memblock regions array
  * @type: memblock type of the regions array being doubled
@@ -204,6 +192,7 @@ static int __init_memblock memblock_doub
 						phys_addr_t new_area_size)
 {
 	struct memblock_region *new_array, *old_array;
+	phys_addr_t old_alloc_size, new_alloc_size;
 	phys_addr_t old_size, new_size, addr;
 	int use_slab = slab_is_available();
 	int *in_slab;
@@ -217,6 +206,12 @@ static int __init_memblock memblock_doub
 	/* Calculate new doubled size */
 	old_size = type->max * sizeof(struct memblock_region);
 	new_size = old_size << 1;
+	/*
+	 * We need to allocated new one align to PAGE_SIZE,
+	 *  so late could free them completely.
+	 */
+	old_alloc_size = PAGE_ALIGN(old_size);
+	new_alloc_size = PAGE_ALIGN(new_size);
 
 	/* Retrieve the slab flag */
 	if (type == &memblock.memory)
@@ -245,11 +240,11 @@ static int __init_memblock memblock_doub
 
 		addr = memblock_find_in_range(new_area_start + new_area_size,
 						memblock.current_limit,
-						new_size, sizeof(phys_addr_t));
+						new_alloc_size, PAGE_SIZE);
 		if (!addr && new_area_size)
 			addr = memblock_find_in_range(0,
 					min(new_area_start, memblock.current_limit),
-					new_size, sizeof(phys_addr_t));
+					new_alloc_size, PAGE_SIZE);
 
 		new_array = addr ? __va(addr) : 0;
 	}
@@ -279,13 +274,13 @@ static int __init_memblock memblock_doub
 		kfree(old_array);
 	else if (old_array != memblock_memory_init_regions &&
 		 old_array != memblock_reserved_init_regions)
-		memblock_free(__pa(old_array), old_size);
+		memblock_free(__pa(old_array), old_alloc_size);
 
 	/* Reserve the new array if that comes from the memblock.
 	 * Otherwise, we needn't do it
 	 */
 	if (!use_slab)
-		BUG_ON(memblock_reserve(addr, new_size));
+		BUG_ON(memblock_reserve(addr, new_alloc_size));
 
 	/* Update slab flag */
 	*in_slab = use_slab;
Index: linux-2.6/mm/nobootmem.c
===================================================================
--- linux-2.6.orig/mm/nobootmem.c
+++ linux-2.6/mm/nobootmem.c
@@ -105,27 +105,35 @@ static void __init __free_pages_memory(u
 		__free_pages_bootmem(pfn_to_page(i), 0);
 }
 
+static unsigned long __init __free_memory_core(phys_addr_t start,
+				 phys_addr_t end)
+{
+	unsigned long start_pfn = PFN_UP(start);
+	unsigned long end_pfn = min_t(unsigned long,
+				      PFN_DOWN(end), max_low_pfn);
+
+	if (start_pfn > end_pfn)
+		return 0;
+
+	__free_pages_memory(start_pfn, end_pfn);
+
+	return end_pfn - start_pfn;
+}
+
 unsigned long __init free_low_memory_core_early(int nodeid)
 {
 	unsigned long count = 0;
-	phys_addr_t start, end;
+	phys_addr_t start, end, size;
 	u64 i;
 
-	/* free reserved array temporarily so that it's treated as free area */
-	memblock_free_reserved_regions();
+	for_each_free_mem_range(i, MAX_NUMNODES, &start, &end, NULL)
+		count += __free_memory_core(start, end);
 
-	for_each_free_mem_range(i, MAX_NUMNODES, &start, &end, NULL) {
-		unsigned long start_pfn = PFN_UP(start);
-		unsigned long end_pfn = min_t(unsigned long,
-					      PFN_DOWN(end), max_low_pfn);
-		if (start_pfn < end_pfn) {
-			__free_pages_memory(start_pfn, end_pfn);
-			count += end_pfn - start_pfn;
-		}
-	}
+	/* free range that is used for reserved array if we allocate it */
+	size = get_allocated_memblock_reserved_regions_info(&start);
+	if (size)
+		count += __free_memory_core(start, start + size);
 
-	/* put region array back? */
-	memblock_reserve_reserved_regions();
 	return count;
 }
 

[-- Attachment #3: fix_free_memblock_memory.patch --]
[-- Type: application/octet-stream, Size: 3624 bytes --]

Subject: [PATCH] memblock: Free allocated memblock.memory.regions

We could allocate that too.

So free it like reserved.regions.

For x86_64, need to move absent_page calculating early.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/mm/init_64.c    |    4 +++-
 include/linux/memblock.h |    1 +
 mm/memblock.c            |   23 ++++++++++++++++++-----
 mm/nobootmem.c           |    5 +++++
 4 files changed, 27 insertions(+), 6 deletions(-)

Index: linux-2.6/arch/x86/mm/init_64.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/init_64.c
+++ linux-2.6/arch/x86/mm/init_64.c
@@ -690,6 +690,9 @@ void __init mem_init(void)
 
 	reservedpages = 0;
 
+	/* get that before num_free_all_bootmem, it will free memory.regions */
+	absent_pages = absent_pages_in_range(0, max_pfn);
+
 	/* this will put all low memory onto the freelists */
 #ifdef CONFIG_NUMA
 	totalram_pages = numa_free_all_bootmem();
@@ -697,7 +700,6 @@ void __init mem_init(void)
 	totalram_pages = free_all_bootmem();
 #endif
 
-	absent_pages = absent_pages_in_range(0, max_pfn);
 	reservedpages = max_pfn - totalram_pages - absent_pages;
 	after_bootmem = 1;
 
Index: linux-2.6/include/linux/memblock.h
===================================================================
--- linux-2.6.orig/include/linux/memblock.h
+++ linux-2.6/include/linux/memblock.h
@@ -51,6 +51,7 @@ phys_addr_t memblock_find_in_range_node(
 phys_addr_t memblock_find_in_range(phys_addr_t start, phys_addr_t end,
 				   phys_addr_t size, phys_addr_t align);
 phys_addr_t get_allocated_memblock_reserved_regions_info(phys_addr_t *addr);
+phys_addr_t get_allocated_memblock_memory_regions_info(phys_addr_t *addr);
 void memblock_allow_resize(void);
 int memblock_add_node(phys_addr_t base, phys_addr_t size, int nid);
 int memblock_add(phys_addr_t base, phys_addr_t size);
Index: linux-2.6/mm/memblock.c
===================================================================
--- linux-2.6.orig/mm/memblock.c
+++ linux-2.6/mm/memblock.c
@@ -160,16 +160,29 @@ static void __init_memblock memblock_rem
 	}
 }
 
-phys_addr_t __init_memblock get_allocated_memblock_reserved_regions_info(
+static phys_addr_t __init_memblock get_allocated_memblock_regions_info(
+					struct memblock_type *type,
 					phys_addr_t *addr)
 {
-	if (memblock.reserved.regions == memblock_reserved_init_regions)
+        if (type->regions == memblock_memory_init_regions ||
+            type->regions == memblock_reserved_init_regions)
 		return 0;
 
-	*addr = __pa(memblock.reserved.regions);
+	*addr = __pa(type->regions);
+
+	return PAGE_ALIGN(sizeof(struct memblock_region) * type->max);
+}
+
+phys_addr_t __init_memblock get_allocated_memblock_reserved_regions_info(
+					phys_addr_t *addr)
+{
+	return get_allocated_memblock_regions_info(&memblock.reserved, addr);
+}
 
-	return PAGE_ALIGN(sizeof(struct memblock_region) *
-			  memblock.reserved.max);
+phys_addr_t __init_memblock get_allocated_memblock_memory_regions_info(
+					phys_addr_t *addr)
+{
+	return get_allocated_memblock_regions_info(&memblock.memory, addr);
 }
 
 /**
Index: linux-2.6/mm/nobootmem.c
===================================================================
--- linux-2.6.orig/mm/nobootmem.c
+++ linux-2.6/mm/nobootmem.c
@@ -134,6 +134,11 @@ unsigned long __init free_low_memory_cor
 	if (size)
 		count += __free_memory_core(start, start + size);
 
+	/* free range that is used for memory array if we allocate it */
+	size = get_allocated_memblock_memory_regions_info(&start);
+	if (size)
+		count += __free_memory_core(start, start + size);
+
 	return count;
 }
 

[-- Attachment #4: memblock_clear_check.patch --]
[-- Type: application/octet-stream, Size: 5256 bytes --]

Subject: [PATCH] memblock: Add checking about illegal using memblock

After memblock is not used anymore, Clear the memblock so we will not
use it wrongly.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/mm/init_32.c    |    3 +++
 arch/x86/mm/init_64.c    |    2 ++
 include/linux/memblock.h |    1 +
 mm/memblock.c            |   28 ++++++++++++++++++++++++++++
 4 files changed, 34 insertions(+)

Index: linux-2.6/arch/x86/mm/init_32.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/init_32.c
+++ linux-2.6/arch/x86/mm/init_32.c
@@ -759,6 +759,9 @@ void __init mem_init(void)
 		if (page_is_ram(tmp) && PageReserved(pfn_to_page(tmp)))
 			reservedpages++;
 
+	/* clear to catch wrong usage */
+	memblock_clear();
+
 	codesize =  (unsigned long) &_etext - (unsigned long) &_text;
 	datasize =  (unsigned long) &_edata - (unsigned long) &_etext;
 	initsize =  (unsigned long) &__init_end - (unsigned long) &__init_begin;
Index: linux-2.6/arch/x86/mm/init_64.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/init_64.c
+++ linux-2.6/arch/x86/mm/init_64.c
@@ -701,6 +701,8 @@ void __init mem_init(void)
 #endif
 
 	reservedpages = max_pfn - totalram_pages - absent_pages;
+	/* clear to catch wrong usage */
+	memblock_clear();
 	after_bootmem = 1;
 
 	codesize =  (unsigned long) &_etext - (unsigned long) &_text;
Index: linux-2.6/include/linux/memblock.h
===================================================================
--- linux-2.6.orig/include/linux/memblock.h
+++ linux-2.6/include/linux/memblock.h
@@ -46,6 +46,7 @@ extern int memblock_debug;
 #define memblock_dbg(fmt, ...) \
 	if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
 
+void memblock_clear(void);
 phys_addr_t memblock_find_in_range_node(phys_addr_t start, phys_addr_t end,
 				phys_addr_t size, phys_addr_t align, int nid);
 phys_addr_t memblock_find_in_range(phys_addr_t start, phys_addr_t end,
Index: linux-2.6/mm/memblock.c
===================================================================
--- linux-2.6.orig/mm/memblock.c
+++ linux-2.6/mm/memblock.c
@@ -101,6 +101,8 @@ phys_addr_t __init_memblock memblock_fin
 	phys_addr_t this_start, this_end, cand;
 	u64 i;
 
+	WARN_ONCE(!memblock.reserved.max, "memblock.reserved was cleared already!");
+
 	/* pump up @end */
 	if (end == MEMBLOCK_ALLOC_ACCESSIBLE)
 		end = memblock.current_limit;
@@ -143,6 +145,15 @@ phys_addr_t __init_memblock memblock_fin
 					   MAX_NUMNODES);
 }
 
+/*
+ * Clear memblock
+ */
+void __init_memblock memblock_clear(void)
+{
+	memset(&memblock.reserved, 0, sizeof(memblock.reserved));
+	memset(&memblock.memory, 0, sizeof(memblock.memory));
+}
+
 static void __init_memblock memblock_remove_region(struct memblock_type *type, unsigned long r)
 {
 	type->total_size -= type->regions[r].size;
@@ -448,11 +459,15 @@ repeat:
 int __init_memblock memblock_add_node(phys_addr_t base, phys_addr_t size,
 				       int nid)
 {
+	WARN_ONCE(!memblock.memory.max, "memblock.memory was cleared already!");
+
 	return memblock_add_region(&memblock.memory, base, size, nid);
 }
 
 int __init_memblock memblock_add(phys_addr_t base, phys_addr_t size)
 {
+	WARN_ONCE(!memblock.memory.max, "memblock.memory was cleared already!");
+
 	return memblock_add_region(&memblock.memory, base, size, MAX_NUMNODES);
 }
 
@@ -547,11 +562,15 @@ static int __init_memblock __memblock_re
 
 int __init_memblock memblock_remove(phys_addr_t base, phys_addr_t size)
 {
+	WARN_ONCE(!memblock.memory.max, "memblock.memory was cleared already!");
+
 	return __memblock_remove(&memblock.memory, base, size);
 }
 
 int __init_memblock memblock_free(phys_addr_t base, phys_addr_t size)
 {
+	WARN_ONCE(!memblock.reserved.max, "memblock.reserved was cleared already!");
+
 	memblock_dbg("   memblock_free: [%#016llx-%#016llx] %pF\n",
 		     (unsigned long long)base,
 		     (unsigned long long)base + size,
@@ -564,6 +583,7 @@ int __init_memblock memblock_reserve(phy
 {
 	struct memblock_type *_rgn = &memblock.reserved;
 
+	WARN_ONCE(!memblock.reserved.max, "memblock.reserved was cleared already!");
 	memblock_dbg("memblock_reserve: [%#016llx-%#016llx] %pF\n",
 		     (unsigned long long)base,
 		     (unsigned long long)base + size,
@@ -604,6 +624,9 @@ void __init_memblock __next_free_mem_ran
 	int mi = *idx & 0xffffffff;
 	int ri = *idx >> 32;
 
+	WARN_ONCE(!mem->max, "memblock.memory was cleared already!");
+	WARN_ONCE(!rsv->max, "memblock.reserved was cleared already!");
+
 	for ( ; mi < mem->cnt; mi++) {
 		struct memblock_region *m = &mem->regions[mi];
 		phys_addr_t m_start = m->base;
@@ -667,6 +690,9 @@ void __init_memblock __next_free_mem_ran
 	int mi = *idx & 0xffffffff;
 	int ri = *idx >> 32;
 
+	WARN_ONCE(!mem->max, "memblock.memory was cleared already!");
+	WARN_ONCE(!rsv->max, "memblock.reserved was cleared already!");
+
 	if (*idx == (u64)ULLONG_MAX) {
 		mi = mem->cnt - 1;
 		ri = rsv->cnt;
@@ -763,6 +789,8 @@ int __init_memblock memblock_set_node(ph
 	int start_rgn, end_rgn;
 	int i, ret;
 
+	WARN_ONCE(!memblock.memory.max, "memblock.memory was cleared already!");
+
 	ret = memblock_isolate_range(type, base, size, &start_rgn, &end_rgn);
 	if (ret)
 		return ret;