Re: [PATCH v7 1/4] powerpc/fadump: reduce memory consumption for capture kernel
On Friday 18 August 2017 05:27 PM, Michal Suchánek wrote: On Fri, 18 Aug 2017 16:20:53 +0530 Hari Bathiniwrote: Hi Michal, Thanks for the patches. I tried testing with the patches: [0.00] fadump: Firmware-assisted dump is active. [0.00] fadump: Modifying command line to enforce the additional parameters passed through 'fadump_extra_args=' [0.00] fadump: Original command line: BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M "fadump_extra_args=nr_cpus=1 numa=off udev.childern-max=2" rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap [0.00] fadump: Modified command line: BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M "fadump_extra_args nr_cpus=1 numa=off udev.childern-max=2" rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap Looks like the quotes are retained not enforcing the parameters... Hello, You are passing an argument >>"fadump_extra_args<< - that is an argument containing a quote in the name. I did not test this scenario Actually, this was not intentional.. I passed fadump_extra_args="nr_cpus=1 numa=off udev.childern-max=2" through grub loader but chosen/bootargs ended up having "fadump_extra_args=nr_cpus=1 numa=off udev.childern-max=2". Need to check why that is.. Thanks Hari assuming the argument name would not match in this case. It would probably not match if the quote was in the middle of the argument name but at the start it is skipped. Note that due to the requirement to remove quotes symmetrically which is added in the third patch this case does not break the commandline - it merely makes the arguments ineffective. The format suggested in the documentation is fadump_extra_args="nr_cpus=1 numa=off udev.childern-max=2"<< - that is quotes around the value. This format worked in my testing. Unfortunately, the format with quote before argument name would probably require another extra parameter to the callback to detect properly. Thanks Michal I am yet to test the patches in other scenarios though.. Thanks Hari On Friday 18 August 2017 01:44 AM, Michal Suchanek wrote: From: Hari Bathini With fadump (dump capture) kernel booting like a regular kernel, it needs almost 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_extra_args=' that would take regular parameters as a space separated quoted string, to be enforced when fadump is active. This 'fadump_extra_args=' 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 Signed-off-by: Hari Bathini Signed-off-by: Michal Suchanek --- Changes from v6: Correct and simplify quote handling. Ideally I would like to extend parse_args to give the length of the original quoted value to callback. However, parse_args removes at most one doubel-quote from the start and one from the end so that is easy to detect. Otherwise all other users will have to be updated to trash the new argument. --- arch/powerpc/include/asm/fadump.h | 2 + arch/powerpc/kernel/fadump.c | 109 -- arch/powerpc/kernel/prom.c| 7 +++ 3 files changed, 115 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h index ce88bbe1d809..98ae00943fb3 100644 --- a/arch/powerpc/include/asm/fadump.h +++ b/arch/powerpc/include/asm/fadump.h @@ -208,11 +208,13 @@ extern int early_init_dt_scan_fw_dump(unsigned long node, const char *uname, int depth, void *data); extern int fadump_reserve_mem(void); extern int setup_fadump(void); +extern void enforce_fadump_extra_args(char *cmdline); extern int is_fadump_active(void); extern void crash_fadump(struct pt_regs *, const char *); extern void fadump_cleanup(void); #else/* CONFIG_FA_DUMP */ +static inline void enforce_fadump_extra_args(char *cmdline) { } static inline int is_fadump_active(void) { return 0; } static inline void crash_fadump(struct pt_regs *regs, const char *str) { } #endif diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index dc0c49cfd90a..a1614d9b8a21 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -78,8 +78,10 @@ int __init early_init_dt_scan_fw_dump(unsigned long node, * dump data waiting for us.
Re: [PATCH v7 1/4] powerpc/fadump: reduce memory consumption for capture kernel
On Friday 18 August 2017 05:27 PM, Michal Suchánek wrote: On Fri, 18 Aug 2017 16:20:53 +0530 Hari Bathini wrote: Hi Michal, Thanks for the patches. I tried testing with the patches: [0.00] fadump: Firmware-assisted dump is active. [0.00] fadump: Modifying command line to enforce the additional parameters passed through 'fadump_extra_args=' [0.00] fadump: Original command line: BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M "fadump_extra_args=nr_cpus=1 numa=off udev.childern-max=2" rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap [0.00] fadump: Modified command line: BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M "fadump_extra_args nr_cpus=1 numa=off udev.childern-max=2" rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap Looks like the quotes are retained not enforcing the parameters... Hello, You are passing an argument >>"fadump_extra_args<< - that is an argument containing a quote in the name. I did not test this scenario Actually, this was not intentional.. I passed fadump_extra_args="nr_cpus=1 numa=off udev.childern-max=2" through grub loader but chosen/bootargs ended up having "fadump_extra_args=nr_cpus=1 numa=off udev.childern-max=2". Need to check why that is.. Thanks Hari assuming the argument name would not match in this case. It would probably not match if the quote was in the middle of the argument name but at the start it is skipped. Note that due to the requirement to remove quotes symmetrically which is added in the third patch this case does not break the commandline - it merely makes the arguments ineffective. The format suggested in the documentation is fadump_extra_args="nr_cpus=1 numa=off udev.childern-max=2"<< - that is quotes around the value. This format worked in my testing. Unfortunately, the format with quote before argument name would probably require another extra parameter to the callback to detect properly. Thanks Michal I am yet to test the patches in other scenarios though.. Thanks Hari On Friday 18 August 2017 01:44 AM, Michal Suchanek wrote: From: Hari Bathini With fadump (dump capture) kernel booting like a regular kernel, it needs almost 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_extra_args=' that would take regular parameters as a space separated quoted string, to be enforced when fadump is active. This 'fadump_extra_args=' 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 Signed-off-by: Hari Bathini Signed-off-by: Michal Suchanek --- Changes from v6: Correct and simplify quote handling. Ideally I would like to extend parse_args to give the length of the original quoted value to callback. However, parse_args removes at most one doubel-quote from the start and one from the end so that is easy to detect. Otherwise all other users will have to be updated to trash the new argument. --- arch/powerpc/include/asm/fadump.h | 2 + arch/powerpc/kernel/fadump.c | 109 -- arch/powerpc/kernel/prom.c| 7 +++ 3 files changed, 115 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h index ce88bbe1d809..98ae00943fb3 100644 --- a/arch/powerpc/include/asm/fadump.h +++ b/arch/powerpc/include/asm/fadump.h @@ -208,11 +208,13 @@ extern int early_init_dt_scan_fw_dump(unsigned long node, const char *uname, int depth, void *data); extern int fadump_reserve_mem(void); extern int setup_fadump(void); +extern void enforce_fadump_extra_args(char *cmdline); extern int is_fadump_active(void); extern void crash_fadump(struct pt_regs *, const char *); extern void fadump_cleanup(void); #else/* CONFIG_FA_DUMP */ +static inline void enforce_fadump_extra_args(char *cmdline) { } static inline int is_fadump_active(void) { return 0; } static inline void crash_fadump(struct pt_regs *regs, const char *str) { } #endif diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index dc0c49cfd90a..a1614d9b8a21 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -78,8 +78,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
Re: [PATCH v7 1/4] powerpc/fadump: reduce memory consumption for capture kernel
On Fri, 18 Aug 2017 16:20:53 +0530 Hari Bathiniwrote: > Hi Michal, > > > Thanks for the patches. I tried testing with the patches: > > [0.00] fadump: Firmware-assisted dump is active. > [0.00] fadump: Modifying command line to enforce the > additional parameters passed through 'fadump_extra_args=' > [0.00] fadump: Original command line: > BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root > ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M > "fadump_extra_args=nr_cpus=1 numa=off udev.childern-max=2" > rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap > [0.00] fadump: Modified command line: > BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root > ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M > "fadump_extra_args nr_cpus=1 numa=off udev.childern-max=2" > rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap > > Looks like the quotes are retained not enforcing the parameters... Hello, You are passing an argument >>"fadump_extra_args<< - that is an argument containing a quote in the name. I did not test this scenario assuming the argument name would not match in this case. It would probably not match if the quote was in the middle of the argument name but at the start it is skipped. Note that due to the requirement to remove quotes symmetrically which is added in the third patch this case does not break the commandline - it merely makes the arguments ineffective. The format suggested in the documentation is >>fadump_extra_args="nr_cpus=1 numa=off udev.childern-max=2"<< - that is quotes around the value. This format worked in my testing. Unfortunately, the format with quote before argument name would probably require another extra parameter to the callback to detect properly. Thanks Michal > > I am yet to test the patches in other scenarios though.. > > > Thanks > > Hari > > > On Friday 18 August 2017 01:44 AM, Michal Suchanek wrote: > > From: Hari Bathini > > > > With fadump (dump capture) kernel booting like a regular kernel, it > > needs almost 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_extra_args=' that would take > > regular parameters as a space separated quoted string, to be > > enforced when fadump is active. This 'fadump_extra_args=' 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 > > Signed-off-by: Hari Bathini > > Signed-off-by: Michal Suchanek > > --- > > Changes from v6: > > Correct and simplify quote handling. Ideally I would like to extend > > parse_args to give the length of the original quoted value to > > callback. However, parse_args removes at most one doubel-quote from > > the start and one from the end so that is easy to detect. Otherwise > > all other users will have to be updated to trash the new argument. > > --- > > arch/powerpc/include/asm/fadump.h | 2 + > > arch/powerpc/kernel/fadump.c | 109 > > -- > > arch/powerpc/kernel/prom.c| 7 +++ 3 files changed, 115 > > insertions(+), 3 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/fadump.h > > b/arch/powerpc/include/asm/fadump.h index > > ce88bbe1d809..98ae00943fb3 100644 --- > > a/arch/powerpc/include/asm/fadump.h +++ > > b/arch/powerpc/include/asm/fadump.h @@ -208,11 +208,13 @@ extern > > int early_init_dt_scan_fw_dump(unsigned long node, const char > > *uname, int depth, void *data); extern int fadump_reserve_mem(void); > > extern int setup_fadump(void); > > +extern void enforce_fadump_extra_args(char *cmdline); > > extern int is_fadump_active(void); > > extern void crash_fadump(struct pt_regs *, const char *); > > extern void fadump_cleanup(void); > > > > #else /* CONFIG_FA_DUMP */ > > +static inline void enforce_fadump_extra_args(char *cmdline) { } > > static inline int is_fadump_active(void) { return 0; } > > static inline void crash_fadump(struct pt_regs *regs, const char > > *str) { } #endif > > diff --git a/arch/powerpc/kernel/fadump.c > > b/arch/powerpc/kernel/fadump.c index dc0c49cfd90a..a1614d9b8a21 > > 100644 --- a/arch/powerpc/kernel/fadump.c > > +++ b/arch/powerpc/kernel/fadump.c > > @@ -78,8 +78,10 @@ int __init early_init_dt_scan_fw_dump(unsigned > > long node, > > * dump data waiting for us. > > */ > > fdm_active =
Re: [PATCH v7 1/4] powerpc/fadump: reduce memory consumption for capture kernel
On Fri, 18 Aug 2017 16:20:53 +0530 Hari Bathini wrote: > Hi Michal, > > > Thanks for the patches. I tried testing with the patches: > > [0.00] fadump: Firmware-assisted dump is active. > [0.00] fadump: Modifying command line to enforce the > additional parameters passed through 'fadump_extra_args=' > [0.00] fadump: Original command line: > BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root > ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M > "fadump_extra_args=nr_cpus=1 numa=off udev.childern-max=2" > rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap > [0.00] fadump: Modified command line: > BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root > ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M > "fadump_extra_args nr_cpus=1 numa=off udev.childern-max=2" > rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap > > Looks like the quotes are retained not enforcing the parameters... Hello, You are passing an argument >>"fadump_extra_args<< - that is an argument containing a quote in the name. I did not test this scenario assuming the argument name would not match in this case. It would probably not match if the quote was in the middle of the argument name but at the start it is skipped. Note that due to the requirement to remove quotes symmetrically which is added in the third patch this case does not break the commandline - it merely makes the arguments ineffective. The format suggested in the documentation is >>fadump_extra_args="nr_cpus=1 numa=off udev.childern-max=2"<< - that is quotes around the value. This format worked in my testing. Unfortunately, the format with quote before argument name would probably require another extra parameter to the callback to detect properly. Thanks Michal > > I am yet to test the patches in other scenarios though.. > > > Thanks > > Hari > > > On Friday 18 August 2017 01:44 AM, Michal Suchanek wrote: > > From: Hari Bathini > > > > With fadump (dump capture) kernel booting like a regular kernel, it > > needs almost 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_extra_args=' that would take > > regular parameters as a space separated quoted string, to be > > enforced when fadump is active. This 'fadump_extra_args=' 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 > > Signed-off-by: Hari Bathini > > Signed-off-by: Michal Suchanek > > --- > > Changes from v6: > > Correct and simplify quote handling. Ideally I would like to extend > > parse_args to give the length of the original quoted value to > > callback. However, parse_args removes at most one doubel-quote from > > the start and one from the end so that is easy to detect. Otherwise > > all other users will have to be updated to trash the new argument. > > --- > > arch/powerpc/include/asm/fadump.h | 2 + > > arch/powerpc/kernel/fadump.c | 109 > > -- > > arch/powerpc/kernel/prom.c| 7 +++ 3 files changed, 115 > > insertions(+), 3 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/fadump.h > > b/arch/powerpc/include/asm/fadump.h index > > ce88bbe1d809..98ae00943fb3 100644 --- > > a/arch/powerpc/include/asm/fadump.h +++ > > b/arch/powerpc/include/asm/fadump.h @@ -208,11 +208,13 @@ extern > > int early_init_dt_scan_fw_dump(unsigned long node, const char > > *uname, int depth, void *data); extern int fadump_reserve_mem(void); > > extern int setup_fadump(void); > > +extern void enforce_fadump_extra_args(char *cmdline); > > extern int is_fadump_active(void); > > extern void crash_fadump(struct pt_regs *, const char *); > > extern void fadump_cleanup(void); > > > > #else /* CONFIG_FA_DUMP */ > > +static inline void enforce_fadump_extra_args(char *cmdline) { } > > static inline int is_fadump_active(void) { return 0; } > > static inline void crash_fadump(struct pt_regs *regs, const char > > *str) { } #endif > > diff --git a/arch/powerpc/kernel/fadump.c > > b/arch/powerpc/kernel/fadump.c index dc0c49cfd90a..a1614d9b8a21 > > 100644 --- a/arch/powerpc/kernel/fadump.c > > +++ b/arch/powerpc/kernel/fadump.c > > @@ -78,8 +78,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) { > > +
Re: [PATCH v7 1/4] powerpc/fadump: reduce memory consumption for capture kernel
Hi Michal, Thanks for the patches. I tried testing with the patches: [0.00] fadump: Firmware-assisted dump is active. [0.00] fadump: Modifying command line to enforce the additional parameters passed through 'fadump_extra_args=' [0.00] fadump: Original command line: BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M "fadump_extra_args=nr_cpus=1 numa=off udev.childern-max=2" rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap [0.00] fadump: Modified command line: BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M "fadump_extra_args nr_cpus=1 numa=off udev.childern-max=2" rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap Looks like the quotes are retained not enforcing the parameters... I am yet to test the patches in other scenarios though.. Thanks Hari On Friday 18 August 2017 01:44 AM, Michal Suchanek wrote: From: Hari BathiniWith fadump (dump capture) kernel booting like a regular kernel, it needs almost 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_extra_args=' that would take regular parameters as a space separated quoted string, to be enforced when fadump is active. This 'fadump_extra_args=' 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 Signed-off-by: Hari Bathini Signed-off-by: Michal Suchanek --- Changes from v6: Correct and simplify quote handling. Ideally I would like to extend parse_args to give the length of the original quoted value to callback. However, parse_args removes at most one doubel-quote from the start and one from the end so that is easy to detect. Otherwise all other users will have to be updated to trash the new argument. --- arch/powerpc/include/asm/fadump.h | 2 + arch/powerpc/kernel/fadump.c | 109 -- arch/powerpc/kernel/prom.c| 7 +++ 3 files changed, 115 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h index ce88bbe1d809..98ae00943fb3 100644 --- a/arch/powerpc/include/asm/fadump.h +++ b/arch/powerpc/include/asm/fadump.h @@ -208,11 +208,13 @@ extern int early_init_dt_scan_fw_dump(unsigned long node, const char *uname, int depth, void *data); extern int fadump_reserve_mem(void); extern int setup_fadump(void); +extern void enforce_fadump_extra_args(char *cmdline); extern int is_fadump_active(void); extern void crash_fadump(struct pt_regs *, const char *); extern void fadump_cleanup(void); #else /* CONFIG_FA_DUMP */ +static inline void enforce_fadump_extra_args(char *cmdline) { } static inline int is_fadump_active(void) { return 0; } static inline void crash_fadump(struct pt_regs *regs, const char *str) { } #endif diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index dc0c49cfd90a..a1614d9b8a21 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -78,8 +78,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. @@ -332,8 +334,11 @@ 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" @@ -373,7 +378,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
Re: [PATCH v7 1/4] powerpc/fadump: reduce memory consumption for capture kernel
Hi Michal, Thanks for the patches. I tried testing with the patches: [0.00] fadump: Firmware-assisted dump is active. [0.00] fadump: Modifying command line to enforce the additional parameters passed through 'fadump_extra_args=' [0.00] fadump: Original command line: BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M "fadump_extra_args=nr_cpus=1 numa=off udev.childern-max=2" rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap [0.00] fadump: Modified command line: BOOT_IMAGE=/vmlinux-4.13.0-rc1-bz155783+ root=/dev/mapper/rhel-root ro crashkernel=2048M fadump=on fadump_reserve_mem=1024M "fadump_extra_args nr_cpus=1 numa=off udev.childern-max=2" rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap Looks like the quotes are retained not enforcing the parameters... I am yet to test the patches in other scenarios though.. Thanks Hari On Friday 18 August 2017 01:44 AM, Michal Suchanek wrote: From: Hari Bathini With fadump (dump capture) kernel booting like a regular kernel, it needs almost 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_extra_args=' that would take regular parameters as a space separated quoted string, to be enforced when fadump is active. This 'fadump_extra_args=' 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 Signed-off-by: Hari Bathini Signed-off-by: Michal Suchanek --- Changes from v6: Correct and simplify quote handling. Ideally I would like to extend parse_args to give the length of the original quoted value to callback. However, parse_args removes at most one doubel-quote from the start and one from the end so that is easy to detect. Otherwise all other users will have to be updated to trash the new argument. --- arch/powerpc/include/asm/fadump.h | 2 + arch/powerpc/kernel/fadump.c | 109 -- arch/powerpc/kernel/prom.c| 7 +++ 3 files changed, 115 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h index ce88bbe1d809..98ae00943fb3 100644 --- a/arch/powerpc/include/asm/fadump.h +++ b/arch/powerpc/include/asm/fadump.h @@ -208,11 +208,13 @@ extern int early_init_dt_scan_fw_dump(unsigned long node, const char *uname, int depth, void *data); extern int fadump_reserve_mem(void); extern int setup_fadump(void); +extern void enforce_fadump_extra_args(char *cmdline); extern int is_fadump_active(void); extern void crash_fadump(struct pt_regs *, const char *); extern void fadump_cleanup(void); #else /* CONFIG_FA_DUMP */ +static inline void enforce_fadump_extra_args(char *cmdline) { } static inline int is_fadump_active(void) { return 0; } static inline void crash_fadump(struct pt_regs *regs, const char *str) { } #endif diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index dc0c49cfd90a..a1614d9b8a21 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -78,8 +78,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. @@ -332,8 +334,11 @@ 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" @@ -373,7 +378,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 @@ -460,6 +464,105 @@ static int __init early_fadump_reserve_mem(char *p) }
[PATCH v7 1/4] powerpc/fadump: reduce memory consumption for capture kernel
From: Hari BathiniWith fadump (dump capture) kernel booting like a regular kernel, it needs almost 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_extra_args=' that would take regular parameters as a space separated quoted string, to be enforced when fadump is active. This 'fadump_extra_args=' 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 Signed-off-by: Hari Bathini Signed-off-by: Michal Suchanek --- Changes from v6: Correct and simplify quote handling. Ideally I would like to extend parse_args to give the length of the original quoted value to callback. However, parse_args removes at most one doubel-quote from the start and one from the end so that is easy to detect. Otherwise all other users will have to be updated to trash the new argument. --- arch/powerpc/include/asm/fadump.h | 2 + arch/powerpc/kernel/fadump.c | 109 -- arch/powerpc/kernel/prom.c| 7 +++ 3 files changed, 115 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h index ce88bbe1d809..98ae00943fb3 100644 --- a/arch/powerpc/include/asm/fadump.h +++ b/arch/powerpc/include/asm/fadump.h @@ -208,11 +208,13 @@ extern int early_init_dt_scan_fw_dump(unsigned long node, const char *uname, int depth, void *data); extern int fadump_reserve_mem(void); extern int setup_fadump(void); +extern void enforce_fadump_extra_args(char *cmdline); extern int is_fadump_active(void); extern void crash_fadump(struct pt_regs *, const char *); extern void fadump_cleanup(void); #else /* CONFIG_FA_DUMP */ +static inline void enforce_fadump_extra_args(char *cmdline) { } static inline int is_fadump_active(void) { return 0; } static inline void crash_fadump(struct pt_regs *regs, const char *str) { } #endif diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index dc0c49cfd90a..a1614d9b8a21 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -78,8 +78,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. @@ -332,8 +334,11 @@ 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" @@ -373,7 +378,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 @@ -460,6 +464,105 @@ static int __init early_fadump_reserve_mem(char *p) } early_param("fadump_reserve_mem", early_fadump_reserve_mem); +#define FADUMP_EXTRA_ARGS_PARAM"fadump_extra_args=" +#define FADUMP_EXTRA_ARGS_LEN (strlen(FADUMP_EXTRA_ARGS_PARAM) - 1) + +struct param_info { + char*cmdline; + char*tmp_cmdline; + int shortening; +}; + +static void __init fadump_update_params(struct param_info *param_info, + char *param, char *val) +{ + ptrdiff_t param_offset = param - param_info->tmp_cmdline; + size_t vallen = val ? strlen(val) : 0; + char *tgt = param_info->cmdline + param_offset + + FADUMP_EXTRA_ARGS_LEN - param_info->shortening; + int shortening = 0; + + if (!val) + return; + + /* remove '=' */ + *tgt++ = ' '; + + /* next_arg removes one leading and one trailing '"' */ + if
[PATCH v7 1/4] powerpc/fadump: reduce memory consumption for capture kernel
From: Hari Bathini With fadump (dump capture) kernel booting like a regular kernel, it needs almost 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_extra_args=' that would take regular parameters as a space separated quoted string, to be enforced when fadump is active. This 'fadump_extra_args=' 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 Signed-off-by: Hari Bathini Signed-off-by: Michal Suchanek --- Changes from v6: Correct and simplify quote handling. Ideally I would like to extend parse_args to give the length of the original quoted value to callback. However, parse_args removes at most one doubel-quote from the start and one from the end so that is easy to detect. Otherwise all other users will have to be updated to trash the new argument. --- arch/powerpc/include/asm/fadump.h | 2 + arch/powerpc/kernel/fadump.c | 109 -- arch/powerpc/kernel/prom.c| 7 +++ 3 files changed, 115 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h index ce88bbe1d809..98ae00943fb3 100644 --- a/arch/powerpc/include/asm/fadump.h +++ b/arch/powerpc/include/asm/fadump.h @@ -208,11 +208,13 @@ extern int early_init_dt_scan_fw_dump(unsigned long node, const char *uname, int depth, void *data); extern int fadump_reserve_mem(void); extern int setup_fadump(void); +extern void enforce_fadump_extra_args(char *cmdline); extern int is_fadump_active(void); extern void crash_fadump(struct pt_regs *, const char *); extern void fadump_cleanup(void); #else /* CONFIG_FA_DUMP */ +static inline void enforce_fadump_extra_args(char *cmdline) { } static inline int is_fadump_active(void) { return 0; } static inline void crash_fadump(struct pt_regs *regs, const char *str) { } #endif diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index dc0c49cfd90a..a1614d9b8a21 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -78,8 +78,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. @@ -332,8 +334,11 @@ 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" @@ -373,7 +378,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 @@ -460,6 +464,105 @@ static int __init early_fadump_reserve_mem(char *p) } early_param("fadump_reserve_mem", early_fadump_reserve_mem); +#define FADUMP_EXTRA_ARGS_PARAM"fadump_extra_args=" +#define FADUMP_EXTRA_ARGS_LEN (strlen(FADUMP_EXTRA_ARGS_PARAM) - 1) + +struct param_info { + char*cmdline; + char*tmp_cmdline; + int shortening; +}; + +static void __init fadump_update_params(struct param_info *param_info, + char *param, char *val) +{ + ptrdiff_t param_offset = param - param_info->tmp_cmdline; + size_t vallen = val ? strlen(val) : 0; + char *tgt = param_info->cmdline + param_offset + + FADUMP_EXTRA_ARGS_LEN - param_info->shortening; + int shortening = 0; + + if (!val) + return; + + /* remove '=' */ + *tgt++ = ' '; + + /* next_arg removes one leading and one trailing '"' */ + if (*tgt == '"') + shortening += 1; + if (*(tgt + vallen + shortening) == '"') +