Hi Michal,
Sorry for taking this long to respond. I was working on a few other things.
On Monday 15 May 2017 02:59 PM, Michal Suchánek wrote:
Hello,
On Mon, 15 May 2017 12:59:46 +0530
Hari Bathini <hbath...@linux.vnet.ibm.com> wrote:
On Friday 12 May 2017 09:12 PM, Michal Suchánek wrote:
On Fri, 12 May 2017 15:15:33 +0530
Hari Bathini <hbath...@linux.vnet.ibm.com> wrote:
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..
How so?
presumably you can reuse parse_args even if you do not register with
early_param and call it yourself. Then your parsing of
fadump_append is
I wasn't aware of that. Thanks for pointing it out, Michal.
Will try to use parse_args and get back.
I was thinking a bit more about the uses of the commandline and how
fadump_append potentially breaks it.
The first thing that should be addressed and is the special -- argument
which denotes the start of init arguments that are not to be parsed by
the kernel. Incidentally the initial implementation using early_param
happened to handles that without issue. parse_args surely handles that
so adding a hook somewhere should give you location of that argument
(if any). And interesting thing that can happen is passing an -- inside
the fadump_append argument. It should be handled (or not) in some way
or other and the handling documented.
The intention with this patch is to replace
"root=/dev/sda2 ro fadump_append=nr_cpus=1,numa=off crashkernel=1024M"
with
"root=/dev/sda2 ro nr_cpus=1 numa=off crashkernel=1024M"
when kernel is booting for dump capture.
While parse_args() is making parsing relatively easy, the main idea is
to replace parameters as above when fadump capture kernel is booting.
The code is relatively complicated to replace parameters using
space-separated
quoted string even though parse_args() may parse them easily.
Comma-separated
list was easier to implement and seemed less error prone for replacing
parameters.
Want to replace fadump_append=, to avoid command line overflow issues and
also to not have to deal with special cases like --. Actually,
fadump_append=
seems to be confusing in that sense. How about
fadump_args=comma-separated-list-
of-params-for-capture-kernel ? Please share your thoughts.
The second thing that breaks is reusing content of /proc/cmdline in
kexec calls for passing arguments to the loaded kernel. It works
flawlessly passing the same arguments the currently running kernel was
started with unless fadump_append argument handler appends content of
the fadump_append argument to actual commandline that appears there. So
this would be an argument against modifying the commandline. You could
argue using fadump and kexec together is an exotic use case but I would
expect that the very machines that require fadump_append to reduce dump
memory requirement benefit from using kexec for reboots because the BIOS
probing the hardware takes quite a while as well. If rewriting the
commandline is desired then this side effect of recursively adding the
content of fadump_append on kexecs which reuse /proc/cmdline should be
documented.
fadump capture kernel (the kernel where we expand "fadump_append=x,y,z"
to "x y z") only advised to be used for saving dump (just like kdump
kernel).
We are expected to boot into production kernel immediately after saving
dump.
Only in special cases where a user configures to stay in fadump capture
kernel
will he encounter the above problem. And someone choosing such scenario
is most
likely aware of what to make of /proc/cmdline?
Thanks
Hari
