On Thursday 11 May 2017 06:46 PM, Michal Suchánek wrote:
On Thu, 11 May 2017 02:00:11 +0530
Hari Bathini <hbath...@linux.vnet.ibm.com> wrote:

Hello Michal,

On Wednesday 10 May 2017 09:31 PM, Michal Suchánek wrote:
Hello,

On Wed, 03 May 2017 23:52:52 +0530
Hari Bathini <hbath...@linux.vnet.ibm.com> wrote:
With the introduction of 'fadump_append=' parameter to pass
additional parameters to fadump (capture) kernel, update
documentation about it.

Signed-off-by: Hari Bathini <hbath...@linux.vnet.ibm.com>
---

Changes from v4:
* Based on top of patchset that includes
    
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=akpm&id=05f383cdfba8793240e73f9a9fbff4e25d66003f


   Documentation/powerpc/firmware-assisted-dump.txt |   10
+++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/powerpc/firmware-assisted-dump.txt
b/Documentation/powerpc/firmware-assisted-dump.txt index
8394bc8..6327193 100644 ---
a/Documentation/powerpc/firmware-assisted-dump.txt +++
b/Documentation/powerpc/firmware-assisted-dump.txt @@ -162,7
+162,15 @@ 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 command line parameters as a comma
+   separated list through 'fadump_append=' parameter, to be
enforced
+   when fadump is active. For example, if parameters like
nr_cpus=1,
+   numa=off & udev.children-max=2 are to be enforced when fadump
is
+   active, 'fadump_append=nr_cpus=1,numa=off,udev.children-max=2'
+   can be passed in command line, which will be replaced with
+   "nr_cpus=1 numa=off udev.children-max=2" when fadump is active.
+   This helps in reducing memory consumption during dump capture.
+4. Optionally, user can also set 'crashkernel=' kernel cmdline
      to specify size of the memory to reserve for boot memory dump
      preservation.
Writing your own deficient parser for comma separated arguments when
perfectly fine parser for space separated quoted arguments exists in
the kernel and the bootloader does not seem like a good idea to
me.

Couple of things that prompted me for v5 are:
    1. Using parse_early_options() limits the kind of parameters
       that can be passed to fadump capture kernel. Passing parameters
       like systemd.unit= & udev.childern.max= has no effect with v4.
Updating
       boot_command_line parameter, when fadump is active, seems a
better alternative.

    2. Passing space-separated quoted arguments is not working
       as intended with lilo. Updating bootloader with the below
       entry in /etc/lilo.conf file results in a missing append
       entry in /etc/yaboot.conf file.

         append = "quiet sysrq=1 insmod=sym53c8xx insmod=ipr
crashkernel=512M-:256M fadump_append=\"nr_cpus=1 numa=off
udev.children-max=2\""
Meaning that a script that emulates LILO semantics on top of yaboot
which is completely capable of passing qouted space separated arguments
fails. IMHO it is more reasonable to fix the script or whatever
adaptation layer or use yaboot directly than working around bug in said
script by introducing a new argument parser in the kernel.



Hmmm.. while trying to implement space-separated parameter list with quotes
as syntax for fadump_append parameter, noticed that it can make implemenation more vulnerable. Here are some problems I am facing while implementing this..

As the boot_command_line needs to be updated before parse_early_param,
we still need some kind of parser. Now this parser has to look for quotes.
But grub2 is updating fadump_append=\"nr_cpus=1 numa=off\" in
/etc/default/grub to "fadump_append=nr_cpus=1 numa=off" cmdline.
This may make parsing complicated in cases where only a single parameter
is passed without quotes like fadump_append=nr_cpus=1. It can get more
complicated, if there is some other parameter after fadump_append with
quotes like below:

fadump_append=nr_cpus=1 paramx paramy "another_param=something with spaces"

Can't say the same about a comma-separated parameter list. Though I won't
dare say it is the best way but maybe the lesser of two evils..

Thanks
Hari

Reply via email to