[Xen-devel] [PATCH v1] Remove stale crashkernel= example from documentation

[Xen-devel] [PATCH v1] Remove stale crashkernel= example from documentation

[Olaf Hering]

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

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1] Remove stale crashkernel= example from documentation

[Andrew Cooper]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1] Remove stale crashkernel= example from documentation

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 354 bytes --]

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

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1] Remove stale crashkernel= example from documentation

[Jan Beulich]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1] Remove stale crashkernel= example from documentation

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 586 bytes --]

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

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1] Remove stale crashkernel= example from documentation

[Jan Beulich]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1] Remove stale crashkernel= example from documentation

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 1390 bytes --]

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

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel