linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel
@ 2017-05-02 15:56 Michal Suchanek
  2017-05-02 15:56 ` [PATCH v4 RFT 2/2] powerpc/fadump: update documentation about 'fadump_append=' parameter Michal Suchanek
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Michal Suchanek @ 2017-05-02 15:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Jonathan Corbet, Hari Bathini, Michal Suchanek, Vlastimil Babka,
	Michal Hocko, linuxppc-dev, linux-doc, linux-kernel

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@ellerman.id.au>
Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
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);
+	}
+
+	return 0;
+}
+early_param("fadump_append", early_fadump_append_param);
+
 /* Look for fadump= cmdline option. */
 static int __init early_fadump_param(char *p)
 {
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 RFT 2/2] powerpc/fadump: update documentation about 'fadump_append=' parameter
  2017-05-02 15:56 [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel Michal Suchanek
@ 2017-05-02 15:56 ` Michal Suchanek
  2017-05-02 16:44 ` [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel Hari Bathini
  2017-05-03  7:13 ` Hari Bathini
  2 siblings, 0 replies; 7+ messages in thread
From: Michal Suchanek @ 2017-05-02 15:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Jonathan Corbet, Hari Bathini, Michal Suchanek, Vlastimil Babka,
	Michal Hocko, linuxppc-dev, linux-doc, linux-kernel

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>
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 Documentation/powerpc/firmware-assisted-dump.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/powerpc/firmware-assisted-dump.txt b/Documentation/powerpc/firmware-assisted-dump.txt
index 9cabaf8..99ab8f4 100644
--- a/Documentation/powerpc/firmware-assisted-dump.txt
+++ b/Documentation/powerpc/firmware-assisted-dump.txt
@@ -162,7 +162,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 'crashkernel=' kernel cmdline
+3. A user can pass additional kernel parameters as a comma separated list
+   through 'fadump_append=' parameter, to be be appended 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.
 
@@ -172,6 +176,8 @@ NOTE: 1. 'fadump_reserve_mem=' parameter has been deprecated. Instead
       2. If firmware-assisted dump fails to reserve memory then it
          will fallback to existing kdump mechanism if 'crashkernel='
          option is set at kernel cmdline.
+      3. 'fadump_append=' parameter con be quoted to append multiple arguments
+         as in 'fadump_append="nr_cpus=8 numa=off"'
 
 Sysfs/debugfs files:
 ------------
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel
  2017-05-02 15:56 [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel Michal Suchanek
  2017-05-02 15:56 ` [PATCH v4 RFT 2/2] powerpc/fadump: update documentation about 'fadump_append=' parameter Michal Suchanek
@ 2017-05-02 16:44 ` Hari Bathini
  2017-05-03  7:13 ` Hari Bathini
  2 siblings, 0 replies; 7+ messages in thread
From: Hari Bathini @ 2017-05-02 16:44 UTC (permalink / raw)
  To: Michal Suchanek, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Jonathan Corbet, Vlastimil Babka, Michal Hocko,
	linuxppc-dev, linux-doc, linux-kernel

Hi Michal,


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@ellerman.id.au>
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> 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);
> +	}
> +
> +	return 0;
> +}
> +early_param("fadump_append", early_fadump_append_param);
> +
>   /* Look for fadump= cmdline option. */
>   static int __init early_fadump_param(char *p)
>   {

I don't think this addresses Michael's concern in v3..
IIUC, we should actually be editing boot_command_line early
in the boot process instead of using parse_early_options() to enforce
the additional parameters..

Thanks
Hari

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel
  2017-05-02 15:56 [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel Michal Suchanek
  2017-05-02 15:56 ` [PATCH v4 RFT 2/2] powerpc/fadump: update documentation about 'fadump_append=' parameter Michal Suchanek
  2017-05-02 16:44 ` [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel Hari Bathini
@ 2017-05-03  7:13 ` Hari Bathini
  2017-05-03  8:49   ` Hari Bathini
  2017-05-10 14:58   ` Michal Suchánek
  2 siblings, 2 replies; 7+ messages in thread
From: Hari Bathini @ 2017-05-03  7:13 UTC (permalink / raw)
  To: Michal Suchanek, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Jonathan Corbet, Vlastimil Babka, Michal Hocko,
	linuxppc-dev, linux-doc, linux-kernel



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@ellerman.id.au>
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> 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
resulting in the intended way..

Thanks
Hari

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel
  2017-05-03  7:13 ` Hari Bathini
@ 2017-05-03  8:49   ` Hari Bathini
  2017-05-10 14:58   ` Michal Suchánek
  1 sibling, 0 replies; 7+ messages in thread
From: Hari Bathini @ 2017-05-03  8:49 UTC (permalink / raw)
  To: Michal Suchanek, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Jonathan Corbet, Vlastimil Babka, Michal Hocko,
	linuxppc-dev, linux-doc, linux-kernel



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@ellerman.id.au>
>> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
>> ---
>> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel
  2017-05-03  7:13 ` Hari Bathini
  2017-05-03  8:49   ` Hari Bathini
@ 2017-05-10 14:58   ` Michal Suchánek
  2017-05-10 15:10     ` Hari Bathini
  1 sibling, 1 reply; 7+ messages in thread
From: Michal Suchánek @ 2017-05-10 14:58 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Jonathan Corbet, Vlastimil Babka, Michal Hocko, linuxppc-dev,
	linux-doc, linux-kernel

On Wed, 3 May 2017 12:43:33 +0530
Hari Bathini <hbathini@linux.vnet.ibm.com> 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@ellerman.id.au>
> > Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > 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=1
> numa=off" is not resulting in the intended way..

Since yaboot arguments are quoted you will probably want something like

append = "quiet splash somearg1 somearg2 fadump_append=\"nr_cpus=1
numa=off\""

in yaboot.conf

Looking at the yaboot parser it seems to handle quoting of quotes so
unlike your parser that does not handle quoting of commas it allows
passing arbitrary arguments to both the normal kernel and the fadump
kernel. 

Thanks

Michal

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel
  2017-05-10 14:58   ` Michal Suchánek
@ 2017-05-10 15:10     ` Hari Bathini
  0 siblings, 0 replies; 7+ messages in thread
From: Hari Bathini @ 2017-05-10 15:10 UTC (permalink / raw)
  To: Michal Suchánek
  Cc: Jonathan Corbet, linux-doc, linux-kernel, Michal Hocko,
	Paul Mackerras, linuxppc-dev, Vlastimil Babka



On Wednesday 10 May 2017 08:28 PM, Michal Suchánek wrote:
> On Wed, 3 May 2017 12:43:33 +0530
> Hari Bathini <hbathini@linux.vnet.ibm.com> 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@ellerman.id.au>
>>> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
>>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
>>> ---
>>> 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=1
>> numa=off" is not resulting in the intended way..
> Since yaboot arguments are quoted you will probably want something like
>
> append = "quiet splash somearg1 somearg2 fadump_append=\"nr_cpus=1
> numa=off\""

I did try this out. But it wasn't helping either..

> in yaboot.conf
>
> Looking at the yaboot parser it seems to handle quoting of quotes so
> unlike your parser that does not handle quoting of commas it allows
> passing arbitrary arguments to both the normal kernel and the fadump
> kernel.

Will improve on v5 to ensure quotes are taken care of..

Thanks
Hari

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-05-10 15:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-02 15:56 [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel Michal Suchanek
2017-05-02 15:56 ` [PATCH v4 RFT 2/2] powerpc/fadump: update documentation about 'fadump_append=' parameter Michal Suchanek
2017-05-02 16:44 ` [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel Hari Bathini
2017-05-03  7:13 ` Hari Bathini
2017-05-03  8:49   ` Hari Bathini
2017-05-10 14:58   ` Michal Suchánek
2017-05-10 15:10     ` Hari Bathini

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