[PATCH 1/2] linux: sparsemem: Initialize all memmap entries within sections

[PATCH 1/2] linux: sparsemem: Initialize all memmap entries within sections

[Akira Takeuchi]

This commit fixes the problem for the kernel
with CONFIG_SPARSEMEM=y and CONFIG_HAVE_ARCH_PFN_VALID=y.

VM subsystem insists that memmap entries within the align to MAX_ORDER_NR_PAGES
must exist and be initialized.

However, in the kernel with CONFIG_SPARSEMEM=y and CONFIG_HAVE_ARCH_PFN_VALID=y,
the kernel only initializes the entries corresponding to the memory regions
specified by "mem=" options. This causes "kernel BUG at mm/page_alloc.c:777!"
This BUG message comes from the following BUG_ON() line in move_freepages().

    BUG_ON(page_zone(start_page) != page_zone(end_page));

Signed-off-by: Akira Takeuchi <takeuchi.akr@jp.panasonic.com>
Signed-off-by: Kiyoshi Owada <owada.kiyoshi@jp.panasonic.com>
---
 include/linux/mmzone.h |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index dff7115..1b7538c 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1088,13 +1088,15 @@ static inline struct mem_section *__pfn_to_section(unsigned long pfn)
 	return __nr_to_section(pfn_to_section_nr(pfn));
 }
 
-#ifndef CONFIG_HAVE_ARCH_PFN_VALID
-static inline int pfn_valid(unsigned long pfn)
+static inline int sparsemem_pfn_valid(unsigned long pfn)
 {
 	if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
 		return 0;
 	return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
 }
+
+#ifndef CONFIG_HAVE_ARCH_PFN_VALID
+#define pfn_valid(pfn) sparsemem_pfn_valid(pfn)
 #endif
 
 static inline int pfn_present(unsigned long pfn)
@@ -1119,7 +1121,7 @@ static inline int pfn_present(unsigned long pfn)
 #define pfn_to_nid(pfn)		(0)
 #endif
 
-#define early_pfn_valid(pfn)	pfn_valid(pfn)
+#define early_pfn_valid(pfn)	sparsemem_pfn_valid(pfn)
 void sparse_init(void);
 #else
 #define sparse_init()	do {} while (0)
-- 
1.7.4.1

[PATCH 2/2] linux: ARM: memmap: Revise freeing unused memmap entries for SPARSEMEM

[Akira Takeuchi]

The SPARSEMEM code allocates memmap entries for each present section.
Though free_unused_memmap() try to free unused memmap entries,
there remains some unused memmap entries which can be freed potentially,
since free_unused_memmap() does not take it into account that
there might exist unused memory regions on the top of each section.

Let's consider the following memory configuration:

    |<------section0------->|<---------hole-------->|<------section1------>|
    +---+-----+---+-----+---+-----------------------+---+--------------+---+
    |   |bank0|   |bank1|   |                       |   |     bank2    |   |
    +---+-----+---+-----+---+-----------------------+---+--------------+---+
    |<->|     |<->|     |<->|                       |<->|              |<->|
     F0        F1        F2                          F3                 F4

F0-F4 is the unused memory regions and their corresponding memmap entries
can be freed. However, free_unused_memmap() only frees the entries for
F1/F2/F4, and does not free the entries for F0/F3.

This patch revises free_unused_memmap() so that all unused memmap entries
are freed. I divide free_unused_memmap() into FLATMEM version and
SPARSEMEM version, and introduce a for loop by section for SPARSEMEM version.

Also, this patch fixes "start_pg = pfn_to_page(start_pfn - 1) + 1" line
in free_memmap(). This line was changed by the past commit
"3257f43d9296ed7adcc84e48f6ddf5313cf29266".
But this change should be reverted now, because I think it was a workaround
for the present free_unused_memmap() code.
Let's consider again the memory configuration above. Without reverted the line,
pfn_to_page() returns a wrong value when freeing F0/F3.

Signed-off-by: Akira Takeuchi <takeuchi.akr@jp.panasonic.com>
Signed-off-by: Kiyoshi Owada <owada.kiyoshi@jp.panasonic.com>
---
 arch/arm/mm/init.c |   81 +++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 245a55a..634486b 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -444,10 +444,15 @@ free_memmap(unsigned long start_pfn, unsigned long end_pfn)
 	struct page *start_pg, *end_pg;
 	unsigned long pg, pgend;
 
+	printk(KERN_INFO
+		"Freeing memmap entries: %4ld KB (PFN 0x%05lx-0x%05lx)\n",
+		((end_pfn - start_pfn) * sizeof(struct page) / 1024),
+		start_pfn, end_pfn);
+
 	/*
 	 * Convert start_pfn/end_pfn to a struct page pointer.
 	 */
-	start_pg = pfn_to_page(start_pfn - 1) + 1;
+	start_pg = pfn_to_page(start_pfn);
 	end_pg = pfn_to_page(end_pfn - 1) + 1;
 
 	/*
@@ -468,6 +473,63 @@ free_memmap(unsigned long start_pfn, unsigned long end_pfn)
 /*
  * The mem_map array can get very big.  Free the unused area of the memory map.
  */
+#ifdef CONFIG_SPARSEMEM
+static void __init free_unused_memmap(struct meminfo *mi)
+{
+	unsigned long pnum;
+
+	for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
+		unsigned long sec_start = section_nr_to_pfn(pnum);
+		unsigned long sec_end = sec_start + (1UL << PFN_SECTION_SHIFT);
+		unsigned long free_start, free_end;
+		unsigned int i;
+
+		if (!valid_section_nr(pnum))
+			continue;
+
+		free_start = sec_start;
+
+		/*
+		 * This relies on each bank being in address order.
+		 * The banks are sorted previously in bootmem_init().
+		 */
+		for_each_bank(i, mi) {
+			struct membank *bank = &mi->bank[i];
+			unsigned long bank_start = bank_pfn_start(bank);
+			unsigned long bank_end = bank_pfn_end(bank);
+
+			/*
+			 * If this bank is out of range for this section,
+			 * skip it.
+			 */
+			if (sec_end <= bank_start || bank_end <= sec_start)
+				continue;
+
+			/*
+			 * Align down here since the VM subsystem insists that
+			 * the memmap entries are valid from the bank start
+			 * aligned to MAX_ORDER_NR_PAGES.
+			 */
+			free_end = round_down(bank_start, MAX_ORDER_NR_PAGES);
+
+			if (free_start < free_end)
+				free_memmap(free_start, free_end);
+
+			/*
+			 * Align up here since the VM subsystem insists that
+			 * the memmap entries are valid from the bank end
+			 * aligned to MAX_ORDER_NR_PAGES.
+			 */
+			free_start = ALIGN(bank_end, MAX_ORDER_NR_PAGES);
+		}
+
+		free_end = sec_end;
+
+		if (free_start < free_end)
+			free_memmap(free_start, free_end);
+	}
+}
+#else /* CONFIG_SPARSEMEM */
 static void __init free_unused_memmap(struct meminfo *mi)
 {
 	unsigned long bank_start, prev_bank_end = 0;
@@ -482,21 +544,13 @@ static void __init free_unused_memmap(struct meminfo *mi)
 
 		bank_start = bank_pfn_start(bank);
 
-#ifdef CONFIG_SPARSEMEM
-		/*
-		 * Take care not to free memmap entries that don't exist
-		 * due to SPARSEMEM sections which aren't present.
-		 */
-		bank_start = min(bank_start,
-				 ALIGN(prev_bank_end, PAGES_PER_SECTION));
-#else
 		/*
 		 * Align down here since the VM subsystem insists that the
 		 * memmap entries are valid from the bank start aligned to
 		 * MAX_ORDER_NR_PAGES.
 		 */
 		bank_start = round_down(bank_start, MAX_ORDER_NR_PAGES);
-#endif
+
 		/*
 		 * If we had a previous bank, and there is a space
 		 * between the current bank and the previous, free it.
@@ -511,13 +565,8 @@ static void __init free_unused_memmap(struct meminfo *mi)
 		 */
 		prev_bank_end = ALIGN(bank_pfn_end(bank), MAX_ORDER_NR_PAGES);
 	}
-
-#ifdef CONFIG_SPARSEMEM
-	if (!IS_ALIGNED(prev_bank_end, PAGES_PER_SECTION))
-		free_memmap(prev_bank_end,
-			    ALIGN(prev_bank_end, PAGES_PER_SECTION));
-#endif
 }
+#endif /* CONFIG_SPARSEMEM */
 
 static void __init free_highpages(void)
 {
-- 
1.7.4.1

Re: [PATCH 2/2] linux: ARM: memmap: Revise freeing unused memmap entries for SPARSEMEM

[Russell King - ARM Linux]

On Thu, Mar 29, 2012 at 07:02:52PM +0900, Akira Takeuchi wrote:
> F0-F4 is the unused memory regions and their corresponding memmap entries
> can be freed. However, free_unused_memmap() only frees the entries for
> F1/F2/F4, and does not free the entries for F0/F3.

No we're not going to support having free areas at the start of an
otherwise populated bank of memory.  This is just insane.  Fix your
hardware instead or your sparsemem setup.

Re: [PATCH 2/2] linux: ARM: memmap: Revise freeing unused memmap entries for SPARSEMEM

[Akira Takeuchi]

On Thu, 29 Mar 2012 13:13:14 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Thu, Mar 29, 2012 at 07:02:52PM +0900, Akira Takeuchi wrote:
> > F0-F4 is the unused memory regions and their corresponding memmap entries
> > can be freed. However, free_unused_memmap() only frees the entries for
> > F1/F2/F4, and does not free the entries for F0/F3.
> 
> No we're not going to support having free areas at the start of an
> otherwise populated bank of memory.  This is just insane.  Fix your
> hardware instead or your sparsemem setup.

As you say, it is able to reduce the number of unused entries remained
by setting SECTION_SIZE_BITS smaller.
On the other hand, I think it's not always true to assign whole memory
to Linux. In some cases, there are free areas at the start of populated
bank of memory and these free areas are assigned to sub CPUs or DSPs
for example. Recent CE SoCs are apt to include multiple CPUs and DSPs
sharing memory among them. This patch is beneficial for such systems,
I think.


Regards,
Akira Takeuchi