diff mbox series

[v1] Remove stale crashkernel= example from documentation

Message ID 20190904091423.23963-1-olaf@aepfle.de (mailing list archive)
State New, archived
Headers show
Series [v1] Remove stale crashkernel= example from documentation | expand

Commit Message

Olaf Hering Sept. 4, 2019, 9:14 a.m. UTC
A plain crashkernel=size is apparently not supported by the code
anymore. In case kdump ever worked like that, the code which removed
support for this notation did not update the documentation.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 docs/misc/kexec_and_kdump.txt | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

Comments

Andrew Cooper Sept. 4, 2019, 9:18 a.m. UTC | #1
On 04/09/2019 10:14, Olaf Hering wrote:
> A plain crashkernel=size is apparently not supported by the code
> anymore. In case kdump ever worked like that, the code which removed
> support for this notation did not update the documentation.
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

That sounds like an accidental regression in parsing of crashkernel=,
rather than a deliberate action.

~Andrew
Olaf Hering Sept. 4, 2019, 9:37 a.m. UTC | #2
Am Wed, 4 Sep 2019 10:18:41 +0100
schrieb Andrew Cooper <andrew.cooper3@citrix.com>:

> That sounds like an accidental regression in parsing of crashkernel=,
> rather than a deliberate action.

Maybe just the lack of b49225dc9df336405292dc08862b4c7c9d887bd6 in vendor binaries...
It is likely broken since 4.10. I have not tried staging.

Olaf
Jan Beulich Sept. 4, 2019, 12:19 p.m. UTC | #3
On 04.09.2019 11:37, Olaf Hering wrote:
> Am Wed, 4 Sep 2019 10:18:41 +0100
> schrieb Andrew Cooper <andrew.cooper3@citrix.com>:
> 
>> That sounds like an accidental regression in parsing of crashkernel=,
>> rather than a deliberate action.
> 
> Maybe just the lack of b49225dc9df336405292dc08862b4c7c9d887bd6 in vendor binaries...

But this change was only to deal with the bogus log message.
The handling was still correct (and the option was being
honored). I also can't see how this would be different now.
If you've really observed the option not working, please
provide more detail.

> It is likely broken since 4.10. I have not tried staging.

Because of the issue just being cosmetic, I didn't consider
in necessary to backport the change.

Jan
Olaf Hering Sept. 4, 2019, 2:13 p.m. UTC | #4
Am Wed, 4 Sep 2019 14:19:23 +0200
schrieb Jan Beulich <jbeulich@suse.com>:

> On 04.09.2019 11:37, Olaf Hering wrote:
> > Maybe just the lack of b49225dc9df336405292dc08862b4c7c9d887bd6 in vendor binaries...  
> But this change was only to deal with the bogus log message.
> The handling was still correct (and the option was being
> honored). I also can't see how this would be different now.

Is that true? My interpretation of the code path is that no colon and nothing after a size value will lead to EINVAL. With this change any unknown string will cause EINVAL.

Olaf
Jan Beulich Sept. 4, 2019, 2:22 p.m. UTC | #5
On 04.09.2019 16:13, Olaf Hering wrote:
> Am Wed, 4 Sep 2019 14:19:23 +0200
> schrieb Jan Beulich <jbeulich@suse.com>:
> 
>> On 04.09.2019 11:37, Olaf Hering wrote:
>>> Maybe just the lack of b49225dc9df336405292dc08862b4c7c9d887bd6 in vendor binaries...  
>> But this change was only to deal with the bogus log message.
>> The handling was still correct (and the option was being
>> honored). I also can't see how this would be different now.
> 
> Is that true? My interpretation of the code path is that no
> colon and nothing after a size value will lead to EINVAL.

First of all - does "the code" here mean master/staging, or any
release branch? And then, yes, on release branches there will be
EINVAL, but as said before kexec_crash_area.size will get/remain
set nevertheless (as the error path doesn't zap any of the
earlier parsing outcome).

> With this change any unknown string will cause EINVAL.

Which is what should happen for unknown (i.e. unsupported) strings,
shouldn't it?

Jan
Olaf Hering Sept. 4, 2019, 3:09 p.m. UTC | #6
Am Wed, 4 Sep 2019 16:22:37 +0200
schrieb Jan Beulich <jbeulich@suse.com>:

> First of all - does "the code" here mean master/staging, or any
> release branch? And then, yes, on release branches there will be
> EINVAL, but as said before kexec_crash_area.size will get/remain
> set nevertheless (as the error path doesn't zap any of the
> earlier parsing outcome).

The code is anything since staging-4.10, in my case 4.11 and 4.12.
And yes, the side effect is setting the values.

I have tried a number of Xen/dom0/kexec-tools/crashkernel= combinations.
SLE12SP4 works, SLE15SP1 fails. I was looking for "crash kernel"
hints in xl dmesg, only much later I spotted the "KDump:" line.
The mentioned entries are not present in /proc/iomem.
All of that led to the conclusion that crashkernel=size does not work.

It turned out the tooling is not dom0 aware, it fails to translate
"console=com1 com1=57600"+"console=hvc0" into "console=ttyS0,57600"
for the kdump kernel. This is a common pattern of ignorance.

And finally the kdump kernel fails to initialize hardware due to
low memory and SWIOTBL? errors. Once drm.ko and usbcore.ko are 
disabled, the first kdump ever was generated on this system.

> > With this change any unknown string will cause EINVAL.  
> Which is what should happen for unknown (i.e. unsupported) strings,
> shouldn't it?

Yes.

Olaf
diff mbox series

Patch

diff --git a/docs/misc/kexec_and_kdump.txt b/docs/misc/kexec_and_kdump.txt
index 0842b3d58f..fea62ffa5c 100644
--- a/docs/misc/kexec_and_kdump.txt
+++ b/docs/misc/kexec_and_kdump.txt
@@ -116,17 +116,7 @@  to run without disrupting the memory used by the first kernel. This area is
 called the crash kernel region and is reserved using the crashkernel
 command line parameter to the Xen hypervisor. It has two forms:
 
-  i) crashkernel=size
-
-     This is the simplest and recommended way to reserve the crash kernel
-     region. Just specify how large the region should be and the hypervisor
-     will find a good location for it. A good size to start with is 128Mb
-
-     e.g.
-
-     crashkernel=128M
-
-  ii) crashkernel=size@base
+  i) crashkernel=size@base
 
       In this form the base address is provided in addition to
       the size. Use this if auto-placement doesn't work for some reason.
@@ -136,7 +126,7 @@  command line parameter to the Xen hypervisor. It has two forms:
 
       e.g. crashkernel=128M@256M
 
-  iii) crashkernel=size,below=offset
+  ii) crashkernel=size,below=offset
 
       This allows us to place the crash kernel within the usuable address
       space without having to worry about a specific phyiscal address.