Re: [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel

[Hari Bathini]

On Wednesday 03 May 2017 12:43 PM, Hari Bathini wrote:
> On Tuesday 02 May 2017 09:26 PM, Michal Suchanek wrote:
> > With fadump (dump capture) kernel booting like a regular kernel, it 
> > almost
> > needs the same amount of memory to boot as the production kernel, 
> > which is
> > unwarranted for a dump capture kernel. But with no option to disable 
> > some
> > of the unnecessary subsystems in fadump kernel, that much memory is 
> > wasted
> > on fadump, depriving the production kernel of that memory.
> >
> > Introduce kernel parameter 'fadump_append=' that would take regular 
> > kernel
> > parameters to be appended when fadump is active.
> > This 'fadump_append=' parameter can be leveraged to pass parameters like
> > nr_cpus=1, cgroup_disable=memory and numa=off, to disable unwarranted
> > resources/subsystems.
> >
> > Also, ensure the log "Firmware-assisted dump is active" is printed early
> > in the boot process to put the subsequent fadump messages in context.
> >
> > Suggested-by: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
> > Signed-off-by: Hari Bathini <hbathini@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Michal Suchanek <msuchanek@xxxxxxx>
> > ---
> > v4:
> >   - use space separated arguments instead of comma separated
> >   - do not append parameters when fadummp is disabled
> > ---
> >   arch/powerpc/kernel/fadump.c | 27 ++++++++++++++++++++++++---
> >   1 file changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> > index ebf2e9c..e0c728a 100644
> > --- a/arch/powerpc/kernel/fadump.c
> > +++ b/arch/powerpc/kernel/fadump.c
> > @@ -79,8 +79,10 @@ int __init early_init_dt_scan_fw_dump(unsigned 
> > long node,
> >        * dump data waiting for us.
> >        */
> >       fdm_active = of_get_flat_dt_prop(node, "ibm,kernel-dump", NULL);
> > -    if (fdm_active)
> > +    if (fdm_active) {
> > +        pr_info("Firmware-assisted dump is active.\n");
> >           fw_dump.dump_active = 1;
> > +    }
> >
> >       /* Get the sizes required to store dump data for the firmware 
> > provided
> >        * dump sections.
> > @@ -263,8 +265,12 @@ int __init fadump_reserve_mem(void)
> >   {
> >       unsigned long base, size, memory_boundary;
> >
> > -    if (!fw_dump.fadump_enabled)
> > +    if (!fw_dump.fadump_enabled) {
> > +        if (fw_dump.dump_active)
> > +            pr_warn("Firmware-assisted dump was active but kernel"
> > +                " booted with fadump disabled!\n");
> >           return 0;
> > +    }
> >
> >       if (!fw_dump.fadump_supported) {
> >           printk(KERN_INFO "Firmware-assisted dump is not supported on"
> > @@ -304,7 +310,6 @@ int __init fadump_reserve_mem(void)
> >           memory_boundary = memblock_end_of_DRAM();
> >
> >       if (fw_dump.dump_active) {
> > -        printk(KERN_INFO "Firmware-assisted dump is active.\n");
> >           /*
> >            * If last boot has crashed then reserve all the memory
> >            * above boot_memory_size so that we don't touch it until
> > @@ -363,6 +368,22 @@ unsigned long __init 
> > arch_reserved_kernel_pages(void)
> >       return memblock_reserved_size() / PAGE_SIZE;
> >   }
> >
> > +/* Look for fadump_append= cmdline option. */
> > +static int __init early_fadump_append_param(char *p)
> > +{
> > +    if (!p)
> > +        return 1;
> > +
> > +    if (fw_dump.fadump_enabled && fw_dump.dump_active) {
> > +    pr_info("enforcing additional parameters (%s) passed through "
> > +        "'fadump_append=' parameter\n", p);
> > +        parse_early_options(p);
>
> While testing this on a system with yaboot bootloader, it seems that 
> parsing
> a parameter with syntax like fadump_append="nr_cpus numa=off" is not

fadump_append="nr_cpus numa=off" should have read 
fadump_append="nr_cpus=1 numa=off".
Nonetheless, considering different environments this is supposed to 
work, I was trying to convey
comma-separated append list maybe better over space-separated one..

Thanks
Hari