ia64/mm: fix a bad_page bug when crash kernel booting

On ia64 platform, I set "crashkernel=1024M-:600M", and dmesg shows 128M-728M memory is reserved for crash kernel. Then "echo c > /proc/sysrq-trigger" to test kdump.

When crash kernel booting, efi_init() will aligns the memory address in IA64_GRANULE_SIZE(16M), so 720M-728M memory will be dropped, It means crash kernel only manage 128M-720M memory.

But initrd start and end are fixed in boot loader, it is before efi_init(), so initrd size maybe overflow when free_initrd_mem().

arch/ia64/mm/init.c +3 -0

index b755ea9..cfdb1eb 100644
@@ -207,6 +207,17 @@ free_initrd_mem (unsigned long start, unsigned long end)
 	start = PAGE_ALIGN(start);
 	end = end & PAGE_MASK;
 
+	/*
+	 * Initrd size is fixed in boot loader, but kernel parameter max_addr
+	 * which aligns in granules is fixed after boot loader, so initrd size

Quoted 10 lines · Xishi Qiu

On ia64 platform, I set "crashkernel=1024M-:600M", and dmesg shows 128M-728M memory is reserved for crash kernel. Then "echo c > /proc/sysrq-trigger" to test kdump.

When crash kernel booting, efi_init() will aligns the memory address in IA64_GRANULE_SIZE(16M), so 720M-728M memory will be dropped, It means crash kernel only manage 128M-720M memory.

But initrd start and end are fixed in boot loader, it is before efi_init(), so initrd size maybe overflow when free_initrd_mem().

[...]

Quoted 11 lines

arch/ia64/mm/init.c +3 -0

index b755ea9..cfdb1eb 100644
@@ -207,6 +207,17 @@ free_initrd_mem (unsigned long start, unsigned long end)
 	start = PAGE_ALIGN(start);
 	end = end & PAGE_MASK;
 
+	/*
+	 * Initrd size is fixed in boot loader, but kernel parameter max_addr
+	 * which aligns in granules is fixed after boot loader, so initrd size

arch/ia64/mm/init.c free_initrd_mem (unsigned long start, unsigned long end) · line 222

Quoted 10 lines

arch/ia64/mm/init.c +8 -0

+	 * maybe overflow.
+	 */
+	if (max_addr != ~0UL) {
+		end = GRANULEROUNDDOWN(end);
+		if (start > end)
+			start = end;
+	}
+
 	if (start < end)
 		printk(KERN_INFO "Freeing initrd memory: %ldkB freed\n", (end - start) >> 10);

I don't think this is the correct fix.

Now, my ia64-fu is weak, but could it be that there's actually a bug in efi_init() and that the following patch would be the best way to fix this?

---

arch/ia64/kernel/efi.c +0 -0

index f034563..8d579f1 100644

.

Quoted 10 lines · Xishi Qiu

On ia64 platform, I set "crashkernel=1024M-:600M", and dmesg shows 128M-728M memory is reserved for crash kernel. Then "echo c > /proc/sysrq-trigger" to test kdump.

When crash kernel booting, efi_init() will aligns the memory address in IA64_GRANULE_SIZE(16M), so 720M-728M memory will be dropped, It means crash kernel only manage 128M-720M memory.

But initrd start and end are fixed in boot loader, it is before efi_init(), so initrd size maybe overflow when free_initrd_mem().

[...]

Quoted 11 lines

arch/ia64/mm/init.c +3 -0

index b755ea9..cfdb1eb 100644
@@ -207,6 +207,17 @@ free_initrd_mem (unsigned long start, unsigned long end)
 	start = PAGE_ALIGN(start);
 	end = end & PAGE_MASK;
 
+	/*
+	 * Initrd size is fixed in boot loader, but kernel parameter max_addr
+	 * which aligns in granules is fixed after boot loader, so initrd size

arch/ia64/mm/init.c free_initrd_mem (unsigned long start, unsigned long end) · line 222

Quoted 10 lines

arch/ia64/mm/init.c +8 -0

+	 * maybe overflow.
+	 */
+	if (max_addr != ~0UL) {
+		end = GRANULEROUNDDOWN(end);
+		if (start > end)
+			start = end;
+	}
+
 	if (start < end)
 		printk(KERN_INFO "Freeing initrd memory: %ldkB freed\n", (end - start) >> 10);

I don't think this is the correct fix.

Now, my ia64-fu is weak, but could it be that there's actually a bug in efi_init() and that the following patch would be the best way to fix this?

---

arch/ia64/kernel/efi.c +0 -0

index f034563..8d579f1 100644

Sorry, this bug will be happen when use Sparse-Memory(section is valid, but last

several pages are invalid). If use Flat-Memory, crash kernel will boot successfully. I think the following patch would be better.

Hi Andrew, will you just ignore the earlier patch and consider the following one? :>

Signed-off-by Xishi Qiu <[email protected]>

---
 arch/ia64/mm/init.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

arch/ia64/mm/init.c +0 -0

index 082e383..23f2ee3 100644

Building linux-next today (tag next-20130212) I get the following errors when building arch/ia64/configs/{tiger_defconfig, zx1_defconfig, bigsur_defconfig, sim_defconfig}

arch/ia64/mm/init.c: In function 'free_initrd_mem':
arch/ia64/mm/init.c:215: error: 'max_addr' undeclared (first use in this function)
arch/ia64/mm/init.c:215: error: (Each undeclared identifier is reported only once
arch/ia64/mm/init.c:215: error: for each function it appears in.) arch/ia64/mm/init.c:216: error: implicit declaration of function 'GRANULEROUNDDOWN'

with "git blame" saying that these lines in init.c were added/changed by

commit 5a54b4fb8f554b15c6113e30ca8412b7fe11c62e

Author: Xishi Qiu <[email protected]>
Date:   Thu Feb 7 12:25:59 2013 +1100

ia64/mm: fix a bad_page bug when crash kernel booting

But, umm, why am I sitting here trying to maintain an ia64 bugfix and handling bug reports from the ia64 maintainer? Wanna swap?

Building linux-next today (tag next-20130212) I get the following errors when building arch/ia64/configs/{tiger_defconfig, zx1_defconfig, bigsur_defconfig, sim_defconfig}

arch/ia64/mm/init.c: In function 'free_initrd_mem':
arch/ia64/mm/init.c:215: error: 'max_addr' undeclared (first use in this function)
arch/ia64/mm/init.c:215: error: (Each undeclared identifier is reported only once
arch/ia64/mm/init.c:215: error: for each function it appears in.) arch/ia64/mm/init.c:216: error: implicit declaration of function 'GRANULEROUNDDOWN'

Quoted 17 lines

Sorry, this bug will be happen when use Sparse-Memory(section is valid, but last

several pages are invalid). If use Flat-Memory, crash kernel will boot successfully. I think the following patch would be better.

Hi Andrew, will you just ignore the earlier patch and consider the following one? :>

Signed-off-by Xishi Qiu <[email protected]>

---
 arch/ia64/mm/init.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

arch/ia64/mm/init.c +0 -0

index 082e383..23f2ee3 100644

arch/ia64/mm/init.c free_initrd_mem (unsigned long start, unsigned long end) · line 216

Quoted 4 lines

arch/ia64/mm/init.c +1 -0

@@ -213,6 +213,8 @@ free_initrd_mem (unsigned long start, unsigned long end)
 	for (; start < end; start += PAGE_SIZE) {
 		if (!virt_addr_valid(start))
 			continue;
+		if ((start >> PAGE_SHIFT) >= max_low_pfn)

I confused the vaddr and paddr, really sorry for it.

In efi_init() memory aligns in IA64_GRANULE_SIZE(16M). If set "crashkernel=1024M-:600M" and use sparse memory model, when crash kernel booting it changes [128M-728M] to [128M-720M]. But initrd memory is in [709M-727M], and virt_addr_valid() *can not* check the invalid pages when freeing initrd memory. There are some pages missed at the end of the seciton.

ChangeLog V3:
fixed vaddr mistake
ChangeLog V2:
add invalid pages check when freeing initrd memory

Signed-off-by Xishi Qiu <[email protected]>

---
 arch/ia64/mm/init.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

arch/ia64/mm/init.c +1 -0

index 082e383..8a269f8 100644
@@ -173,6 +173,7 @@ void __init
 free_initrd_mem (unsigned long start, unsigned long end)
 {
 	struct page *page;
+	unsigned long pfn;
 	/*
 	 * EFI uses 4KB pages while the kernel can use 4KB or bigger.
 	 * Thus EFI and the kernel may have different page sizes. It is

Quoted 28 lines

In efi_init() memory aligns in IA64_GRANULE_SIZE(16M). If set "crashkernel=1024M-:600M" and use sparse memory model, when crash kernel booting it changes [128M-728M] to [128M-720M]. But initrd memory is in [709M-727M], and virt_addr_valid() *can not* check the invalid pages when freeing initrd memory. There are some pages missed at the end of the seciton.

ChangeLog V3:
fixed vaddr mistake
ChangeLog V2:
add invalid pages check when freeing initrd memory

Signed-off-by Xishi Qiu <[email protected]>

---
 arch/ia64/mm/init.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

arch/ia64/mm/init.c +1 -0

index 082e383..8a269f8 100644
@@ -173,6 +173,7 @@ void __init
 free_initrd_mem (unsigned long start, unsigned long end)
 {
 	struct page *page;
+	unsigned long pfn;
 	/*
 	 * EFI uses 4KB pages while the kernel can use 4KB or bigger.
 	 * Thus EFI and the kernel may have different page sizes. It is

arch/ia64/mm/init.c free_initrd_mem (unsigned long start, unsigned long end) · line 222

Quoted 9 lines

arch/ia64/mm/init.c +3 -0

@@ -213,6 +214,9 @@ free_initrd_mem (unsigned long start, unsigned long end)
 	for (; start < end; start += PAGE_SIZE) {
 		if (!virt_addr_valid(start))
 			continue;
+		pfn = __pa(start) >> PAGE_SHIFT;
+		if (pfn >= max_low_pfn)
+			continue;
 		page = virt_to_page(start);
 		ClearPageReserved(page);
 		init_page_count(page);

I would have presumed that fixing this bug would involve modifying the ia64-specific kexec code?

Tony, Fenghua? Any thoughts?

In efi_init() memory aligns in IA64_GRANULE_SIZE(16M). If set "crashkernel=1024M-:600M"

Foolishly sent an earlier reply from Outlook which appears to have mangled/lost it. Trying again ...

Quoted 1 line

In efi_init() memory aligns in IA64_GRANULE_SIZE(16M). If set "crashkernel=1024M-:600M"

Is this where the real problem begins? Should we insist that users

provide crashkernel
parameters rounded to GRANULE boundaries?

-Tony

Seems like a good idea, should we modify "\linux\Documentation\kernel-parameters.txt"?