* [PATCH] ia64/mm: fix a bad_page bug when crash kernel booting
@ 2013-01-29 3:52 Xishi Qiu
2013-02-04 16:32 ` Matt Fleming
0 siblings, 1 reply; 15+ messages in thread
From: Xishi Qiu @ 2013-01-29 3:52 UTC (permalink / raw)
To: Luck, Tony, fenghua.yu, matt.fleming, Xishi Qiu, Liujiang, Andrew Morton
Cc: linux-ia64, linux-kernel, linux-efi, linux-mm
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] ia64/mm: fix a bad_page bug when crash kernel booting
2013-01-29 3:52 [PATCH] ia64/mm: fix a bad_page bug when crash kernel booting Xishi Qiu
@ 2013-02-04 16:32 ` Matt Fleming
2013-02-05 3:48 ` Xishi Qiu
2013-02-07 2:32 ` [PATCH V2] " Xishi Qiu
0 siblings, 2 replies; 15+ messages in thread
From: Matt Fleming @ 2013-02-04 16:32 UTC (permalink / raw)
To: Xishi Qiu
Cc: Luck, Tony, fenghua.yu, Liujiang, Andrew Morton, linux-ia64,
linux-kernel, linux-efi, linux-mm
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 {
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-04 16:32 ` Matt Fleming
@ 2013-02-05 3:48 ` Xishi Qiu
2013-02-07 2:32 ` [PATCH V2] " Xishi Qiu
1 sibling, 0 replies; 15+ messages in thread
From: Xishi Qiu @ 2013-02-05 3:48 UTC (permalink / raw)
To: Matt Fleming
Cc: Luck, Tony, fenghua.yu, Liujiang, Andrew Morton, linux-ia64,
linux-kernel, linux-efi, linux-mm
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
>
> .
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH V2] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-04 16:32 ` Matt Fleming
2013-02-05 3:48 ` Xishi Qiu
@ 2013-02-07 2:32 ` Xishi Qiu
2013-02-07 6:09 ` [PATCH V3] " Xishi Qiu
1 sibling, 1 reply; 15+ messages in thread
From: Xishi Qiu @ 2013-02-07 2:32 UTC (permalink / raw)
To: Matt Fleming
Cc: Luck, Tony, fenghua.yu, Liujiang, Andrew Morton, linux-ia64,
linux-kernel, linux-efi, linux-mm, Hanjun Guo, WuJianguo,
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-07 2:32 ` [PATCH V2] " Xishi Qiu
@ 2013-02-07 6:09 ` Xishi Qiu
2013-02-13 0:11 ` Tony Luck
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Xishi Qiu @ 2013-02-07 6:09 UTC (permalink / raw)
To: Xishi Qiu
Cc: Matt Fleming, Luck, Tony, fenghua.yu, Liujiang, Andrew Morton,
linux-ia64, linux-kernel, linux-efi, linux-mm, Hanjun Guo,
WuJianguo
> 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-07 6:09 ` [PATCH V3] " Xishi Qiu
@ 2013-02-13 0:11 ` Tony Luck
2013-02-13 0:19 ` Andrew Morton
2013-02-13 0:22 ` Andrew Morton
2013-02-13 10:07 ` Matt Fleming
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Tony Luck @ 2013-02-13 0:11 UTC (permalink / raw)
To: Xishi Qiu
Cc: Matt Fleming, fenghua.yu, Liujiang, Andrew Morton, linux-ia64,
linux-kernel, linux-efi, linux-mm, Hanjun Guo, WuJianguo
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-13 0:11 ` Tony Luck
@ 2013-02-13 0:19 ` Andrew Morton
2013-02-13 0:32 ` Tony Luck
2013-02-13 0:22 ` Andrew Morton
1 sibling, 1 reply; 15+ messages in thread
From: Andrew Morton @ 2013-02-13 0:19 UTC (permalink / raw)
To: Tony Luck
Cc: Xishi Qiu, Matt Fleming, fenghua.yu, Liujiang, linux-ia64,
linux-kernel, linux-efi, linux-mm, Hanjun Guo, WuJianguo
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?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-13 0:11 ` Tony Luck
2013-02-13 0:19 ` Andrew Morton
@ 2013-02-13 0:22 ` Andrew Morton
1 sibling, 0 replies; 15+ messages in thread
From: Andrew Morton @ 2013-02-13 0:22 UTC (permalink / raw)
To: Tony Luck
Cc: Xishi Qiu, Matt Fleming, fenghua.yu, Liujiang, linux-ia64,
linux-kernel, linux-efi, linux-mm, Hanjun Guo, WuJianguo
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-13 0:19 ` Andrew Morton
@ 2013-02-13 0:32 ` Tony Luck
0 siblings, 0 replies; 15+ messages in thread
From: Tony Luck @ 2013-02-13 0:32 UTC (permalink / raw)
To: Andrew Morton
Cc: Xishi Qiu, Matt Fleming, fenghua.yu, Liujiang, linux-ia64,
linux-kernel, linux-efi, linux-mm, Hanjun Guo, WuJianguo
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-07 6:09 ` [PATCH V3] " Xishi Qiu
2013-02-13 0:11 ` Tony Luck
@ 2013-02-13 10:07 ` Matt Fleming
2013-02-16 1:55 ` Xishi Qiu
2013-02-19 21:38 ` Luck, Tony
2013-02-19 21:56 ` Tony Luck
3 siblings, 1 reply; 15+ messages in thread
From: Matt Fleming @ 2013-02-13 10:07 UTC (permalink / raw)
To: Xishi Qiu
Cc: Luck, Tony, fenghua.yu, Liujiang, Andrew Morton, linux-ia64,
linux-kernel, linux-efi, linux-mm, Hanjun Guo, WuJianguo
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?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-13 10:07 ` Matt Fleming
@ 2013-02-16 1:55 ` Xishi Qiu
0 siblings, 0 replies; 15+ messages in thread
From: Xishi Qiu @ 2013-02-16 1:55 UTC (permalink / raw)
To: Matt Fleming, Luck, Tony, fenghua.yu, Liujiang, Andrew Morton
Cc: linux-ia64, linux-kernel, linux-efi, linux-mm, Hanjun Guo, WuJianguo
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-07 6:09 ` [PATCH V3] " Xishi Qiu
2013-02-13 0:11 ` Tony Luck
2013-02-13 10:07 ` Matt Fleming
@ 2013-02-19 21:38 ` Luck, Tony
2013-02-19 21:56 ` Tony Luck
3 siblings, 0 replies; 15+ messages in thread
From: Luck, Tony @ 2013-02-19 21:38 UTC (permalink / raw)
To: Xishi Qiu
Cc: Fleming, Matt, Yu, Fenghua, Liujiang, Andrew Morton, linux-ia64,
linux-kernel, linux-efi, linux-mm, Hanjun Guo, WuJianguo
[-- 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¥
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-07 6:09 ` [PATCH V3] " Xishi Qiu
` (2 preceding siblings ...)
2013-02-19 21:38 ` Luck, Tony
@ 2013-02-19 21:56 ` Tony Luck
2013-02-20 1:38 ` Xishi Qiu
3 siblings, 1 reply; 15+ messages in thread
From: Tony Luck @ 2013-02-19 21:56 UTC (permalink / raw)
To: Xishi Qiu
Cc: Matt Fleming, fenghua.yu, Liujiang, Andrew Morton, linux-ia64,
linux-kernel, linux-efi, linux-mm, Hanjun Guo, WuJianguo
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-19 21:56 ` Tony Luck
@ 2013-02-20 1:38 ` Xishi Qiu
2013-02-21 18:21 ` Tony Luck
0 siblings, 1 reply; 15+ messages in thread
From: Xishi Qiu @ 2013-02-20 1:38 UTC (permalink / raw)
To: Tony Luck
Cc: Matt Fleming, fenghua.yu, Liujiang, Andrew Morton, linux-ia64,
linux-kernel, linux-efi, linux-mm, Hanjun Guo, WuJianguo
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V3] ia64/mm: fix a bad_page bug when crash kernel booting
2013-02-20 1:38 ` Xishi Qiu
@ 2013-02-21 18:21 ` Tony Luck
0 siblings, 0 replies; 15+ messages in thread
From: Tony Luck @ 2013-02-21 18:21 UTC (permalink / raw)
To: Xishi Qiu
Cc: Matt Fleming, fenghua.yu, Liujiang, Andrew Morton, linux-ia64,
linux-kernel, linux-efi, linux-mm, Hanjun Guo, WuJianguo,
linux-arch
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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-02-21 18:22 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-29 3:52 [PATCH] ia64/mm: fix a bad_page bug when crash kernel booting Xishi Qiu
2013-02-04 16:32 ` Matt Fleming
2013-02-05 3:48 ` Xishi Qiu
2013-02-07 2:32 ` [PATCH V2] " Xishi Qiu
2013-02-07 6:09 ` [PATCH V3] " Xishi Qiu
2013-02-13 0:11 ` Tony Luck
2013-02-13 0:19 ` Andrew Morton
2013-02-13 0:32 ` Tony Luck
2013-02-13 0:22 ` Andrew Morton
2013-02-13 10:07 ` Matt Fleming
2013-02-16 1:55 ` Xishi Qiu
2013-02-19 21:38 ` Luck, Tony
2013-02-19 21:56 ` Tony Luck
2013-02-20 1:38 ` Xishi Qiu
2013-02-21 18:21 ` Tony Luck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).