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

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

[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().

Here is the dmesg when crash kernel booting:

...
Ignoring memory below 128MB
Ignoring memory above 720MB
    // aligns the address in IA64_GRANULE_SIZE(16M)
...
Initial ramdisk at: 0xe00000002c3f0000 (20176579 bytes)
    // initrd uses 707M-726M memory
...
Kernel command line:  root=/dev/disk/by-id/ata-STEC_MACH16_M16ISD2-100UCT_STM000142A2D-part3
console=ttyS0,115200n8 console=tty0 initcall_debug elevator=deadline sysrq=1 reset_devices
irqpoll maxcpus=1 initcall_debug linuxrc=trace elfcorehdr=745216K max_addr=728M min_addr=128M
    // show crash kernel parameters
...
Unpacking initramfs...
    // called by populate_rootfs()
Freeing initrd memory: 19648kB freed
    // called by free_initrd()->free_initrd_mem()
BUG: Bad page state in process swapper  pfn:02d00
    // it is a mistake to free over 720M memory to OS (ia64's page size is 64KB)
page:e0000000102dd800 flags:(null) count:0 mapcount:1 mapping:(null) index:0

Call Trace:
 [<a000000100018dc0>] show_stack+0x80/0xa0
                                sp=e000000021e8fbd0 bsp=e000000021e81360
 [<a00000010090fcc0>] dump_stack+0x30/0x50
                                sp=e000000021e8fda0 bsp=e000000021e81348
 [<a0000001001a3180>] bad_page+0x280/0x380
                                sp=e000000021e8fda0 bsp=e000000021e81308
 [<a0000001001a8740>] free_hot_cold_page+0x3a0/0x5c0
                                sp=e000000021e8fda0 bsp=e000000021e812a0
 [<a0000001001a8a50>] free_hot_page+0x30/0x60
                                sp=e000000021e8fda0 bsp=e000000021e81280
 [<a0000001001a8b30>] __free_pages+0xb0/0xe0
                                sp=e000000021e8fda0 bsp=e000000021e81258
 [<a0000001001a8c00>] free_pages+0xa0/0xc0
                                sp=e000000021e8fda0 bsp=e000000021e81230
 [<a000000100bb40c0>] free_initrd_mem+0x230/0x290
                                sp=e000000021e8fda0 bsp=e000000021e811d8
 [<a000000100ba6620>] populate_rootfs+0x1c0/0x280
                                sp=e000000021e8fdb0 bsp=e000000021e811a0
 [<a00000010000ac30>] do_one_initcall+0x3b0/0x3e0
                                sp=e000000021e8fdb0 bsp=e000000021e81158
 [<a000000100ba0a90>] kernel_init+0x3f0/0x4b0
                                sp=e000000021e8fdb0 bsp=e000000021e81108
 [<a000000100016890>] kernel_thread_helper+0xd0/0x100
                                sp=e000000021e8fe30 bsp=e000000021e810e0
 [<a00000010000a4c0>] start_kernel_thread+0x20/0x40
                                sp=e000000021e8fe30 bsp=e000000021e810e0
Disabling lock debugging due to kernel taint
BUG: Bad page state in process swapper  pfn:02d01
page:e0000000102dd838 flags:(null) count:0 mapcount:1 mapping:(null) index:0

Call Trace:
 [<a000000100018dc0>] show_stack+0x80/0xa0
                                sp=e000000021e8fbd0 bsp=e000000021e81360
 [<a00000010090fcc0>] dump_stack+0x30/0x50
                                sp=e000000021e8fda0 bsp=e000000021e81348
 [<a0000001001a3180>] bad_page+0x280/0x380
                                sp=e000000021e8fda0 bsp=e000000021e81308
 [<a0000001001a8740>] free_hot_cold_page+0x3a0/0x5c0
                                sp=e000000021e8fda0 bsp=e000000021e812a0
 [<a0000001001a8a50>] free_hot_page+0x30/0x60
                                sp=e000000021e8fda0 bsp=e000000021e81280
 [<a0000001001a8b30>] __free_pages+0xb0/0xe0
                                sp=e000000021e8fda0 bsp=e000000021e81258
 [<a0000001001a8c00>] free_pages+0xa0/0xc0
                                sp=e000000021e8fda0 bsp=e000000021e81230
 [<a000000100bb40c0>] free_initrd_mem+0x230/0x290
                                sp=e000000021e8fda0 bsp=e000000021e811d8
 [<a000000100ba6620>] populate_rootfs+0x1c0/0x280
                                sp=e000000021e8fdb0 bsp=e000000021e811a0
 [<a00000010000ac30>] do_one_initcall+0x3b0/0x3e0
                                sp=e000000021e8fdb0 bsp=e000000021e81158
 [<a000000100ba0a90>] kernel_init+0x3f0/0x4b0
                                sp=e000000021e8fdb0 bsp=e000000021e81108
 [<a000000100016890>] kernel_thread_helper+0xd0/0x100
                                sp=e000000021e8fe30 bsp=e000000021e810e0
 [<a00000010000a4c0>] start_kernel_thread+0x20/0x40
                                sp=e000000021e8fe30 bsp=e000000021e810e0
...

Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
---
 arch/ia64/include/asm/meminit.h |    2 ++
 arch/ia64/kernel/efi.c          |    2 +-
 arch/ia64/mm/init.c             |   11 +++++++++++
 3 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/arch/ia64/include/asm/meminit.h b/arch/ia64/include/asm/meminit.h
index 61c7b17..925ecb5 100644
--- a/arch/ia64/include/asm/meminit.h
+++ b/arch/ia64/include/asm/meminit.h
@@ -49,6 +49,8 @@ extern int reserve_elfcorehdr(u64 *start, u64 *end);
 #define GRANULEROUNDDOWN(n)	((n) & ~(IA64_GRANULE_SIZE-1))
 #define GRANULEROUNDUP(n)	(((n)+IA64_GRANULE_SIZE-1) & ~(IA64_GRANULE_SIZE-1))

+extern u64 max_addr;
+
 #ifdef CONFIG_NUMA
   extern void call_pernode_memory (unsigned long start, unsigned long len, void *func);
 #else
diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index f034563..f6522cb 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -49,7 +49,7 @@ extern efi_status_t efi_call_phys (void *, ...);
 struct efi efi;
 EXPORT_SYMBOL(efi);
 static efi_runtime_services_t *runtime;
-static u64 mem_limit = ~0UL, max_addr = ~0UL, min_addr = 0UL;
+u64 mem_limit = ~0UL, max_addr = ~0UL, min_addr = 0UL;

 #define efi_call_virt(f, args...)	(*(f))(args)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index b755ea9..cfdb1eb 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -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
+	 * 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);

-- 
1.7.1

Re: [PATCH] ia64/mm: fix a bad_page bug when crash kernel booting

[Matt Fleming]

On Tue, 2013-01-29 at 11:52 +0800, Xishi Qiu wrote:
> 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().

[...]

> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index b755ea9..cfdb1eb 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -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
> +	 * 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?

---

diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index f034563..8d579f1 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -482,7 +482,7 @@ efi_init (void)
 		if (memcmp(cp, "mem=", 4) == 0) {
 			mem_limit = memparse(cp + 4, &cp);
 		} else if (memcmp(cp, "max_addr=", 9) == 0) {
-			max_addr = GRANULEROUNDDOWN(memparse(cp + 9, &cp));
+			max_addr = GRANULEROUNDUP(memparse(cp + 9, &cp));
 		} else if (memcmp(cp, "min_addr=", 9) == 0) {
 			min_addr = GRANULEROUNDDOWN(memparse(cp + 9, &cp));
 		} else {

Re: [PATCH] ia64/mm: fix a bad_page bug when crash kernel booting

[Xishi Qiu]

On 2013/2/5 0:32, Matt Fleming wrote:

> On Tue, 2013-01-29 at 11:52 +0800, Xishi Qiu wrote:
>> 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().
> 
> [...]
> 
>> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
>> index b755ea9..cfdb1eb 100644
>> --- a/arch/ia64/mm/init.c
>> +++ b/arch/ia64/mm/init.c
>> @@ -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
>> +	 * 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?
> 
> ---
> 
> diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
> index f034563..8d579f1 100644
> --- a/arch/ia64/kernel/efi.c
> +++ b/arch/ia64/kernel/efi.c
> @@ -482,7 +482,7 @@ efi_init (void)
>  		if (memcmp(cp, "mem=", 4) == 0) {
>  			mem_limit = memparse(cp + 4, &cp);
>  		} else if (memcmp(cp, "max_addr=", 9) == 0) {
> -			max_addr = GRANULEROUNDDOWN(memparse(cp + 9, &cp));
> +			max_addr = GRANULEROUNDUP(memparse(cp + 9, &cp));
>  		} else if (memcmp(cp, "min_addr=", 9) == 0) {
>  			min_addr = GRANULEROUNDDOWN(memparse(cp + 9, &cp));
>  		} else {
> 
> 

Hi Matt,

Thanks for your awareness.

I have a question. If we set "crashkernel=1024M-:600M", the log shows 128M-728M memory
is reserved for crash kernel. So if "max_addr = GRANULEROUNDUP()", when crash kernel boot,
it uses 128M-736M memory, overflow! (suppose IA64_GRANULE_SIZE=16M).

Thanks,
Xishi Qiu

> 
> .
> 

[PATCH V2] ia64/mm: fix a bad_page bug when crash kernel booting

[Xishi Qiu]

On 2013/2/5 0:32, Matt Fleming wrote:

> On Tue, 2013-01-29 at 11:52 +0800, Xishi Qiu wrote:
>> 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().
> 
> [...]
> 
>> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
>> index b755ea9..cfdb1eb 100644
>> --- a/arch/ia64/mm/init.c
>> +++ b/arch/ia64/mm/init.c
>> @@ -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
>> +	 * 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?
> 
> ---
> 
> diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
> index f034563..8d579f1 100644
> --- a/arch/ia64/kernel/efi.c
> +++ b/arch/ia64/kernel/efi.c
> @@ -482,7 +482,7 @@ efi_init (void)
>  		if (memcmp(cp, "mem=", 4) == 0) {
>  			mem_limit = memparse(cp + 4, &cp);
>  		} else if (memcmp(cp, "max_addr=", 9) == 0) {
> -			max_addr = GRANULEROUNDDOWN(memparse(cp + 9, &cp));
> +			max_addr = GRANULEROUNDUP(memparse(cp + 9, &cp));
>  		} else if (memcmp(cp, "min_addr=", 9) == 0) {
>  			min_addr = GRANULEROUNDDOWN(memparse(cp + 9, &cp));
>  		} else {
> 
> 

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 <qiuxishi@huawei.com>
---
 arch/ia64/mm/init.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 082e383..23f2ee3 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -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)
+			continue;
 		page = virt_to_page(start);
 		ClearPageReserved(page);
 		init_page_count(page);
-- 
1.7.6.1

[PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting

[Xishi Qiu]

> 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 <qiuxishi@huawei.com>
> ---
>  arch/ia64/mm/init.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 082e383..23f2ee3 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -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 <qiuxishi@huawei.com>
---
 arch/ia64/mm/init.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 082e383..8a269f8 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -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
@@ -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);
-- 
1.7.6.1

Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting

[Tony Luck]

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 <qiuxishi@huawei.com>
Date:   Thu Feb 7 12:25:59 2013 +1100

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

-Tony

Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting

[Andrew Morton]

On Tue, 12 Feb 2013 16:11:33 -0800
Tony Luck <tony.luck@gmail.com> wrote:

> 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 <qiuxishi@huawei.com>
> Date:   Thu Feb 7 12:25:59 2013 +1100
> 
>     ia64/mm: fix a bad_page bug when crash kernel booting
> 

Presumably this:

--- a/arch/ia64/mm/init.c~ia64-mm-fix-a-bad_page-bug-when-crash-kernel-booting-fix
+++ a/arch/ia64/mm/init.c
@@ -27,6 +27,7 @@
 #include <asm/machvec.h>
 #include <asm/numa.h>
 #include <asm/patch.h>
+#include <asm/meminit.h>
 #include <asm/pgalloc.h>
 #include <asm/sal.h>
 #include <asm/sections.h>
_


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

Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting

[Andrew Morton]

On Tue, 12 Feb 2013 16:11:33 -0800
Tony Luck <tony.luck@gmail.com> wrote:

> 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'
> 

`max_addr' is an awesomely bad name for a global identifier, btw.

And this:

-static u64 mem_limit = ~0UL, max_addr = ~0UL, min_addr = 0UL;
+u64 mem_limit = ~0UL, max_addr = ~0UL, min_addr = 0UL;

needlessly exported `mem_limit' and `min_addr' as well.  These too are
very poor global identifiers.

Also, Matt suggested an alternative implementation which afaik hasn't
been responded to.


I think I'll just drop the patch.

Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting

[Tony Luck]

On Tue, Feb 12, 2013 at 4:19 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> But, umm, why am I sitting here trying to maintain an ia64 bugfix and
> handling bug reports from the ia64 maintainer?  Wanna swap?

That sounds like a plan.  I'll look out for a new version with the
missing #include
and less silly global variable names and try to take it before you
pull it into -mm

-Tony

Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting

[Matt Fleming]

On Thu, 2013-02-07 at 14:09 +0800, Xishi Qiu wrote:
> > 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 <qiuxishi@huawei.com>
> > ---
> >  arch/ia64/mm/init.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> > index 082e383..23f2ee3 100644
> > --- a/arch/ia64/mm/init.c
> > +++ b/arch/ia64/mm/init.c
> > @@ -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 <qiuxishi@huawei.com>
> ---
>  arch/ia64/mm/init.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 082e383..8a269f8 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -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
> @@ -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?

Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting

[Xishi Qiu]

On 2013/2/13 18:07, Matt Fleming wrote:

>> 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 <qiuxishi@huawei.com>
>> ---
>>  arch/ia64/mm/init.c |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
>> index 082e383..8a269f8 100644
>> --- a/arch/ia64/mm/init.c
>> +++ b/arch/ia64/mm/init.c
>> @@ -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
>> @@ -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?


Hi, I had the Spring Festival a few days ago. Please ignore the earlier
patch and consider the V3 one above.

Thanks,
Xishi Qiu

RE: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting

[Luck, Tony]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 388 bytes --]

> 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
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting

[Tony Luck]

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

> 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

Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting

[Xishi Qiu]

On 2013/2/20 5:56, Tony Luck wrote:

> Foolishly sent an earlier reply from Outlook which appears
> to have mangled/lost it. Trying again ...
> 
>> 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

Hi Tony, I think this is the real problem begins and it only appears when use Sparse-Memory.

> provide crashkernel
> parameters rounded to GRANULE boundaries?
> 

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

Thanks,
Xishi Qiu

> -Tony
>  

Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting

[Tony Luck]

On Tue, Feb 19, 2013 at 5:38 PM, Xishi Qiu <qiuxishi@huawei.com> wrote:
> Seems like a good idea, should we modify "\linux\Documentation\kernel-parameters.txt"?

Perhaps in Documentation/kdump/kdump.txt (which the crashkernel entry
in kernel-parameters.txt
points at).  The ia64 section of kdump.txt notes that the start
address will be rounded up to
a GRANULE boundary, but doesn't talk about restrictions on the size.

I wonder if any other architectures have alignment restrictions on the
addresses in
"crashkernel" parameters? Does x86 like them to be 2MB aligned?

Second question is whether we should check and warn in parse_crashkernel_mem()?
I think the answer is "yes" (since the consequences of getting this
wrong don't show
up till much later, and the errors aren't all that obviously connected
back to the original
mistake).  Perhaps each architecture that cares could provide defines:

#define ARCH_CRASH_KERNEL_START_ALIGN (... arch value here ...)
#define ARCH_CRASH_KERNEL_SIZE_ALIGN (... arch value here ...)

[Suggestion provided mostly to provoke somebody to provide a more
elegant solution]

-Tony