[PATCH v3 1/2] powerpc/fadump: reduce memory consumption for capture kernel

[PATCH v3 1/2] powerpc/fadump: reduce memory consumption for capture kernel

[Hari Bathini]

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 as a comma separated list, to be enforced 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@ellerman.id.au>
Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---

Changes from v2:
* Introducing 'fadump_append=' parameter to pass additional kernel                          
  parameters instead of using a handover area.
        

 arch/powerpc/kernel/fadump.c |   41 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 8ff0dd4..87edc7b 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.
@@ -257,8 +259,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"
@@ -298,7 +304,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
@@ -338,6 +343,36 @@ unsigned long __init arch_reserved_kernel_pages(void)
 	return memblock_reserved_size() / PAGE_SIZE;
 }
 
+static void __init parse_fadump_append_params(const char *p)
+{
+	static char fadump_cmdline[COMMAND_LINE_SIZE] __initdata;
+	char *token;
+
+	strlcpy(fadump_cmdline, p, COMMAND_LINE_SIZE);
+	token = strchr(fadump_cmdline, ',');
+	while (token) {
+		*token = ' ';
+		token = strchr(token, ',');
+	}
+
+	pr_info("enforcing additional parameters (%s) passed through "
+		"'fadump_append=' parameter\n", fadump_cmdline);
+	parse_early_options(fadump_cmdline);
+}
+
+/* Look for fadump_append= cmdline option. */
+static int __init early_fadump_append_param(char *p)
+{
+	if (!p)
+		return 1;
+
+	if (fw_dump.dump_active)
+		parse_fadump_append_params(p);
+
+	return 0;
+}
+early_param("fadump_append", early_fadump_append_param);
+
 /* Look for fadump= cmdline option. */
 static int __init early_fadump_param(char *p)
 {

[PATCH v3 2/2] powerpc/fadump: update documentation about 'fadump_append=' parameter

[Hari Bathini]

With the introduction of 'fadump_append=' parameter to pass additional
parameters to fadump (capture) kernel, update documentation about it.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 Documentation/powerpc/firmware-assisted-dump.txt |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/powerpc/firmware-assisted-dump.txt b/Documentation/powerpc/firmware-assisted-dump.txt
index 3007bc9..8e12380 100644
--- a/Documentation/powerpc/firmware-assisted-dump.txt
+++ b/Documentation/powerpc/firmware-assisted-dump.txt
@@ -158,7 +158,11 @@ How to enable firmware-assisted dump (fadump):
 
 1. Set config option CONFIG_FA_DUMP=y and build kernel.
 2. Boot into linux kernel with 'fadump=on' kernel cmdline option.
-3. Optionally, user can also set 'fadump_reserve_mem=' kernel cmdline
+3. A user can pass additional kernel parameters as a comma separated list
+   through 'fadump_append=' parameter, to be enforced when fadump is active.
+   This can be used to pass parameters like nr_cpus=1, numa=off to reduce
+   memory consumption during dump capture.
+4. Optionally, user can also set 'fadump_reserve_mem=' kernel cmdline
    to specify size of the memory to reserve for boot memory dump
    preservation.
 

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

[Michael Ellerman]

Hari Bathini <hbathini@linux.vnet.ibm.com> writes:

> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 8ff0dd4..87edc7b 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -338,6 +343,36 @@ unsigned long __init arch_reserved_kernel_pages(void)
>  	return memblock_reserved_size() / PAGE_SIZE;
>  }
>  
> +static void __init parse_fadump_append_params(const char *p)
> +{
> +	static char fadump_cmdline[COMMAND_LINE_SIZE] __initdata;
> +	char *token;
> +
> +	strlcpy(fadump_cmdline, p, COMMAND_LINE_SIZE);
> +	token = strchr(fadump_cmdline, ',');
> +	while (token) {
> +		*token = ' ';
> +		token = strchr(token, ',');
> +	}
> +
> +	pr_info("enforcing additional parameters (%s) passed through "
> +		"'fadump_append=' parameter\n", fadump_cmdline);
> +	parse_early_options(fadump_cmdline);

Using parse_early_options() means it only works for parameters defined
with early_param() or __setup() doesn't it?

That might be OK but at least you need to clearly document it.

cheers

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

[Hari Bathini]

On Wednesday 26 April 2017 04:02 PM, Michael Ellerman wrote:
> Hari Bathini <hbathini@linux.vnet.ibm.com> writes:
>
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index 8ff0dd4..87edc7b 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -338,6 +343,36 @@ unsigned long __init arch_reserved_kernel_pages(void)
>>   	return memblock_reserved_size() / PAGE_SIZE;
>>   }
>>   
>> +static void __init parse_fadump_append_params(const char *p)
>> +{
>> +	static char fadump_cmdline[COMMAND_LINE_SIZE] __initdata;
>> +	char *token;
>> +
>> +	strlcpy(fadump_cmdline, p, COMMAND_LINE_SIZE);
>> +	token = strchr(fadump_cmdline, ',');
>> +	while (token) {
>> +		*token = ' ';
>> +		token = strchr(token, ',');
>> +	}
>> +
>> +	pr_info("enforcing additional parameters (%s) passed through "
>> +		"'fadump_append=' parameter\n", fadump_cmdline);
>> +	parse_early_options(fadump_cmdline);
> Using parse_early_options() means it only works for parameters defined
> with early_param() or __setup() doesn't it?

Yeah. Actually, is it better to change commandline from "$params 
fadump_append=nr_cpus=1,numa=off"
to "$params nr_cpus=1 numa=off" early in the boot process? That way, 
parameters like udev.children-max=2 &
systemd.unit=kdump.service may also be specified in fadump_append= 
parameter?

Thanks
Hari

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

[Michael Ellerman]

Hari Bathini <hbathini@linux.vnet.ibm.com> writes:

> On Wednesday 26 April 2017 04:02 PM, Michael Ellerman wrote:
>> Hari Bathini <hbathini@linux.vnet.ibm.com> writes:
>>
>>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>>> index 8ff0dd4..87edc7b 100644
>>> --- a/arch/powerpc/kernel/fadump.c
>>> +++ b/arch/powerpc/kernel/fadump.c
>>> @@ -338,6 +343,36 @@ unsigned long __init arch_reserved_kernel_pages(void)
>>>   	return memblock_reserved_size() / PAGE_SIZE;
>>>   }
>>>   
>>> +static void __init parse_fadump_append_params(const char *p)
>>> +{
>>> +	static char fadump_cmdline[COMMAND_LINE_SIZE] __initdata;
>>> +	char *token;
>>> +
>>> +	strlcpy(fadump_cmdline, p, COMMAND_LINE_SIZE);
>>> +	token = strchr(fadump_cmdline, ',');
>>> +	while (token) {
>>> +		*token = ' ';
>>> +		token = strchr(token, ',');
>>> +	}
>>> +
>>> +	pr_info("enforcing additional parameters (%s) passed through "
>>> +		"'fadump_append=' parameter\n", fadump_cmdline);
>>> +	parse_early_options(fadump_cmdline);
>> Using parse_early_options() means it only works for parameters defined
>> with early_param() or __setup() doesn't it?
>
> Yeah. Actually, is it better to change commandline from "$params 
> fadump_append=nr_cpus=1,numa=off"
> to "$params nr_cpus=1 numa=off" early in the boot process?

It's not better, it's more confusing for a user, because the command
line looks different to what they specified.

But it might be the only option because I don't know if there's an easy
way to trigger parsing for the non-early options.

cheers

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

[Michal Suchánek]

Hello,

On Wed, 26 Apr 2017 13:05:20 +0530
Hari Bathini <hbathini@linux.vnet.ibm.com> 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 as a comma separated list, to be enforced 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.

According to Linux admin guide at
Documentation/admin-guide/kernel-parameters.{rst,txt}

there are arguments like isolcpus=1,2,10-20,100-2000:2/25
baycom_ser_hdx=<io>,<irq>,<mode> or console=ttyUSB0[,options] that
include a comma. On the other hand, same guide suggests that parameters
can be quoted param="spaces in here". 

So I think it would be more sensible to document the existing quoting
mechanism in the fadump_append option documentation rather than
inventing your own that is incompatible with some existing options.

Thanks

Michal