initrd support broken in mips kernel 4.2

DISCUSSION Tony Wu 2 participants 3 messages Aug 31, 2015 – Sep 2, 2015 MIPS

Share & tools

Tony Wu

author

2015-08-31 04:51 UTC #0 lore raw

Hello,

Commit a6335fa11 (MIPS: bootmem: Don't use memory holes for page bitmap) crashes kernel with a initramfs unpacking error when initrd is enabled.

---- error message ----
Unpacking initramfs...
Initramfs unpacking failed: junk in compressed archive

BUG: Bad page state in process swapper  pfn:00261
page:81004c20 count:0 mapcount:-127 mapping:  (null) index:0x2
flags: 0x0()
page dumped because: nonzero mapcount
CPU: 0 PID: 1 Comm: swapper Not tainted 4.2.0+ #1782
-----------------------

The modified logic in bootmem_init does not guarantee mapstart to be placed after initrd_end. mapstart is set to the maximum of reserved_end and start. In case initrd_end is greater than reserved_end, mapstart is placed before initrd_end, and causes initramfs unpacking error.

----- bootmem_init ---
if (end <= reserved_end)
continue;
+#ifdef CONFIG_BLK_DEV_INITRD
+ /* mapstart should be after initrd_end */
+ if (initrd_end && end <= (unsigned long)PFN_UP(__pa(initrd_end)))
+ continue;
+#endif

                if (start >= mapstart)
                        continue;
                mapstart = max(reserved_end, start);

-----------------------

Thanks,
Tony

Alexander Sverdlin

2015-09-02 21:25 UTC #1 lore raw

Hello Tony,

Quoted 13 lines · ext Tony Wu

Commit a6335fa11 (MIPS: bootmem: Don't use memory holes for page bitmap) crashes kernel with a initramfs unpacking error when initrd is enabled.
---- error message ----
Unpacking initramfs...
Initramfs unpacking failed: junk in compressed archive
BUG: Bad page state in process swapper  pfn:00261
page:81004c20 count:0 mapcount:-127 mapping:  (null) index:0x2
flags: 0x0()
page dumped because: nonzero mapcount
CPU: 0 PID: 1 Comm: swapper Not tainted 4.2.0+ #1782
-----------------------

Quoted 4 lines

The modified logic in bootmem_init does not guarantee mapstart to be placed after initrd_end. mapstart is set to the maximum of reserved_end and start. In case initrd_end is greater than reserved_end, mapstart is placed before initrd_end, and causes initramfs unpacking error.

Indeed, seems that there are two problems with the patch. First, "<=" is wrong in the condition. This will fail if initrd and next zone are separated (in boot_mem_map), but have not gap in between. Second, without gap, initrd and PFN area could be combined together by add_memory_region(), so seems that we need to restore max() that was there before a6335fa11.

Review

Quoted 12 lines

----- bootmem_init ---
if (end <= reserved_end)
continue;

(quoted diff) +5 -1

+#ifdef CONFIG_BLK_DEV_INITRD
+               /* mapstart should be after initrd_end */
+               if (initrd_end && end <= (unsigned long)PFN_UP(__pa(initrd_end)))
+                       continue;
+#endif
                if (start >= mapstart)
                        continue;
                mapstart = max(reserved_end, start);
-----------------------

Could you please test the following patch? It fixes the case with gap between initrd and first usable zone and restores the effect of f9a7febd for the case when initrd and the next zone are combined together.

arch/mips/kernel/setup.c +9 -1

@@ -339,7 +339,7 @@ static void __init bootmem_init(void)
 			continue;
 #ifdef CONFIG_BLK_DEV_INITRD
 		/* mapstart should be after initrd_end */
-		if (initrd_end && end <= (unsigned long)PFN_UP(__pa(initrd_end)))
+		if (initrd_end && end < (unsigned long)PFN_UP(__pa(initrd_end)))
 			continue;
 #endif
 		if (start >= mapstart)

@@ -371,6 +371,14 @@ static void __init bootmem_init(void)
 		max_low_pfn = PFN_DOWN(HIGHMEM_START);
 	}
 
+#ifdef CONFIG_BLK_DEV_INITRD
+	/*
+	 * mapstart should be after initrd_end
+	 */
+	if (initrd_end)
+		mapstart = max(mapstart, (unsigned long)PFN_UP(__pa(initrd_end)));
+#endif
+
 	/*
 	 * Initialize the boot-time allocator with low memory only.
 	 */

Alexander Sverdlin

2015-09-02 21:32 UTC #2 lore raw

Hello Tony,

Quoted 13 lines · Alexander Sverdlin

Quoted 13 lines
Commit a6335fa11 (MIPS: bootmem: Don't use memory holes for page bitmap)
Quoted 12 lines
crashes kernel with a initramfs unpacking error when initrd is enabled.
---- error message ----
Unpacking initramfs...
Initramfs unpacking failed: junk in compressed archive
BUG: Bad page state in process swapper  pfn:00261
page:81004c20 count:0 mapcount:-127 mapping:  (null) index:0x2
flags: 0x0()
page dumped because: nonzero mapcount
CPU: 0 PID: 1 Comm: swapper Not tainted 4.2.0+ #1782
-----------------------

Quoted 7 lines

Quoted 4 lines
Quoted 4 lines
The modified logic in bootmem_init does not guarantee mapstart to be placed after initrd_end. mapstart is set to the maximum of reserved_end and start. In case initrd_end is greater than reserved_end, mapstart is placed before initrd_end, and causes initramfs unpacking error.
Indeed, seems that there are two problems with the patch. First, "<=" is wrong in the condition. This will fail if initrd and next zone are separated (in boot_mem_map), but have not gap in between. Second, without gap, initrd and PFN

Oh, "<=" is indeed correct. Could you please test this patch instead of the previously proposed:

arch/mips/kernel/setup.c +9 -1

@@ -338,7 +338,7 @@ static void __init bootmem_init(void)
 		if (end <= reserved_end)
 			continue;
 #ifdef CONFIG_BLK_DEV_INITRD
-		/* mapstart should be after initrd_end */
+		/* Skip zones before initrd and initrd itself */
 		if (initrd_end && end <= (unsigned long)PFN_UP(__pa(initrd_end)))
 			continue;
 #endif

@@ -371,6 +371,14 @@ static void __init bootmem_init(void)
 		max_low_pfn = PFN_DOWN(HIGHMEM_START);
 	}
 
+#ifdef CONFIG_BLK_DEV_INITRD
+	/*
+	 * mapstart should be after initrd_end
+	 */
+	if (initrd_end)
+		mapstart = max(mapstart, (unsigned long)PFN_UP(__pa(initrd_end)));
+#endif
+
 	/*
 	 * Initialize the boot-time allocator with low memory only.
 	 */