| Message ID | 152265062142.23251.2863873644423472277.stgit@jupiter.in.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/fadump: Improvements and fixes for firmware-assisted dump. | expand |
On Monday 02 April 2018 12:00 PM, Mahesh J Salgaonkar wrote: > From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com> > > The second kernel, during early boot after the crash, reserves rest of > the memory above boot memory size to make sure it does not touch any of the > dump memory area. It uses memblock_reserve() that reserves the specified > memory region irrespective of memory holes present within that region. > There are chances where previous kernel would have hot removed some of > its memory leaving memory holes behind. In such cases fadump kernel reports > incorrect number of reserved pages through arch_reserved_kernel_pages() > hook causing kernel to hang or panic. > > Fix this by excluding memory holes while reserving rest of the memory > above boot memory size during second kernel boot after crash. > > Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com> > --- > arch/powerpc/kernel/fadump.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c > index 011f1aa7abab..a497e9fb93fe 100644 > --- a/arch/powerpc/kernel/fadump.c > +++ b/arch/powerpc/kernel/fadump.c > @@ -433,6 +433,21 @@ static inline unsigned long get_fadump_metadata_base( > return be64_to_cpu(fdm->metadata_region.source_address); > } > > +static void fadump_memblock_reserve(unsigned long base, unsigned long size) > +{ > + struct memblock_region *reg; > + unsigned long start, end; > + > + for_each_memblock(memory, reg) { > + start = max(base, (unsigned long)reg->base); > + end = reg->base + reg->size; > + end = min(base + size, end); > + > + if (start < end) > + memblock_reserve(start, end - start); > + } > +} > + > int __init fadump_reserve_mem(void) > { > unsigned long base, size, memory_boundary; > @@ -487,7 +502,7 @@ int __init fadump_reserve_mem(void) > */ > base = fw_dump.boot_memory_size; > size = memory_boundary - base; > - memblock_reserve(base, size); > + fadump_memblock_reserve(base, size); > printk(KERN_INFO "Reserved %ldMB of memory at %ldMB " Mahesh, you may want to change this print as well as it would be misleading in case of holes in the memory. Thanks Hari > "for saving crash dump\n", > (unsigned long)(size >> 20), >
On 04/03/2018 03:21 PM, Hari Bathini wrote: > > > On Monday 02 April 2018 12:00 PM, Mahesh J Salgaonkar wrote: >> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com> >> >> The second kernel, during early boot after the crash, reserves rest of >> the memory above boot memory size to make sure it does not touch any >> of the >> dump memory area. It uses memblock_reserve() that reserves the specified >> memory region irrespective of memory holes present within that region. >> There are chances where previous kernel would have hot removed some of >> its memory leaving memory holes behind. In such cases fadump kernel >> reports >> incorrect number of reserved pages through arch_reserved_kernel_pages() >> hook causing kernel to hang or panic. >> >> Fix this by excluding memory holes while reserving rest of the memory >> above boot memory size during second kernel boot after crash. >> >> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com> >> --- >> arch/powerpc/kernel/fadump.c | 17 ++++++++++++++++- >> 1 file changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c >> index 011f1aa7abab..a497e9fb93fe 100644 >> --- a/arch/powerpc/kernel/fadump.c >> +++ b/arch/powerpc/kernel/fadump.c >> @@ -433,6 +433,21 @@ static inline unsigned long >> get_fadump_metadata_base( >> return be64_to_cpu(fdm->metadata_region.source_address); >> } >> >> +static void fadump_memblock_reserve(unsigned long base, unsigned long >> size) >> +{ >> + struct memblock_region *reg; >> + unsigned long start, end; >> + >> + for_each_memblock(memory, reg) { >> + start = max(base, (unsigned long)reg->base); >> + end = reg->base + reg->size; >> + end = min(base + size, end); >> + >> + if (start < end) >> + memblock_reserve(start, end - start); >> + } >> +} >> + >> int __init fadump_reserve_mem(void) >> { >> unsigned long base, size, memory_boundary; >> @@ -487,7 +502,7 @@ int __init fadump_reserve_mem(void) >> */ >> base = fw_dump.boot_memory_size; >> size = memory_boundary - base; >> - memblock_reserve(base, size); >> + fadump_memblock_reserve(base, size); >> printk(KERN_INFO "Reserved %ldMB of memory at %ldMB " > > Mahesh, you may want to change this print as well as it would be > misleading in case of > holes in the memory. Yeah, may be we can just move that printk also inside fadump_memblock_reserve(). Thanks, -Mahesh. > > Thanks > Hari > >> "for saving crash dump\n", >> (unsigned long)(size >> 20), >> >
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index 011f1aa7abab..a497e9fb93fe 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -433,6 +433,21 @@ static inline unsigned long get_fadump_metadata_base( return be64_to_cpu(fdm->metadata_region.source_address); } +static void fadump_memblock_reserve(unsigned long base, unsigned long size) +{ + struct memblock_region *reg; + unsigned long start, end; + + for_each_memblock(memory, reg) { + start = max(base, (unsigned long)reg->base); + end = reg->base + reg->size; + end = min(base + size, end); + + if (start < end) + memblock_reserve(start, end - start); + } +} + int __init fadump_reserve_mem(void) { unsigned long base, size, memory_boundary; @@ -487,7 +502,7 @@ int __init fadump_reserve_mem(void) */ base = fw_dump.boot_memory_size; size = memory_boundary - base; - memblock_reserve(base, size); + fadump_memblock_reserve(base, size); printk(KERN_INFO "Reserved %ldMB of memory at %ldMB " "for saving crash dump\n", (unsigned long)(size >> 20),