Xen devel - Dec 2011 - Re: [PATCH 3 of 3] KEXEC: Allocate crash structures in low memory

>>> Andrew Cooper  12/23/11 12:59 PM >>>
>On 23/12/11 09:06, Jan Beulich wrote:
>>>>> On 22.12.11 at 18:36, Andrew Cooper  wrote:
>>> On 64bit Xen with 32bit dom0 and crashkernel, xmalloc''ing
items such
>>> as the CPU crash notes will go into the xenheap, which tends to be
in
>>> upper memory.  This causes problems on machines with more than 64GB
>>> (or 4GB if no PAE support) of ram as the crashkernel physically
cant
>>> access the crash notes.
>> What use is a crash dump taken with a 32-bit kernel on a machine
>> with more than 64G (or more than 4G is the crash kernel
doesn''t
>> support PAE)?
>
>Very little use at all, which is the reason for this change.
With this change, the usefulness doesn''t significantly increase imo.
>The correct solution is indeed to use a 64bit dom0 kernel, but while
>politics is preventing that from happening, another solution has to be
>found.  I doubt that XenServer is the only product which would benifit,
>which is why the changes are presented for upstreaming.
>
>>>  3) Change the conring buffer to use lower memory when instructed.
>> Why does this one need moving, but none of the other Xen data
>> structures? If anything, I would have expected you to limit the Xen
>> heap altogether, so that automatically all allocations get done in a
>> way so that at least Xen''s data structures are available in
the
>> (truncated) dump.
>
>This is part of the "min" option which is trying to have the least
>possible impact.  The idea is to have this in low memory, use the
>"console_to_ring" boot option to copy dom0 dmesg into conring, and
pass
>its physical address and size in a crash note, so that the crash kernel
>environment grab it all.
Why is the console ring *that* important? I would have thought
that proper register values and stack contents are much more
significant for analysis of a crash.

Jan

At 15:51 +0000 on 29 Dec (1325173875), Jan Beulich
wrote:
> >This is part of the "min" option which is trying to have the
least
> >possible impact.  The idea is to have this in low memory, use the
> >"console_to_ring" boot option to copy dom0 dmesg into
conring, and pass
> >its physical address and size in a crash note, so that the crash kernel
> >environment grab it all.
> 
> Why is the console ring *that* important? I would have thought
> that proper register values and stack contents are much more
> significant for analysis of a crash.
The console ring has been _very_ useful in diagnosing bugs from field
reports, especially things like guest-level watchdog timeouts and
refcounting errors that cause a crash after the interesting event has
passed.  If nothing else it lets you verify the user''s description of
the system, and saves a round-trip of ''please try to set up a serial
logger and reproduce the bug'').

(I agree, though, that using a 64-bit crash kernel would be a much
better idea).

Tim.

On 29/12/2011 15:51, Jan Beulich wrote:
>>>> Andrew Cooper  12/23/11 12:59 PM >>>
>> On 23/12/11 09:06, Jan Beulich wrote:
>>>>>> On 22.12.11 at 18:36, Andrew Cooper  wrote:
>>>> On 64bit Xen with 32bit dom0 and crashkernel,
xmalloc''ing items such
>>>> as the CPU crash notes will go into the xenheap, which tends to
be in
>>>> upper memory.  This causes problems on machines with more than
64GB
>>>> (or 4GB if no PAE support) of ram as the crashkernel physically
cant
>>>> access the crash notes.
>>> What use is a crash dump taken with a 32-bit kernel on a machine
>>> with more than 64G (or more than 4G is the crash kernel
doesn''t
>>> support PAE)?
>> Very little use at all, which is the reason for this change.
> With this change, the usefulness doesn''t significantly increase
imo.
But it does make it possible for a 32bit crashkernel to get information
useful for debugging a problem, which is the point of the patch.  If you
honestly thing that this change is not worth the effort then that is
fine, but as far as I am aware, there are still several good reasons to
use a 32bit dom0 over a 64bit dom0.  Following this reasoning is why I
believe that this feature may be useful to other projects, as well as
XenServer.
>> The correct solution is indeed to use a 64bit dom0 kernel, but while
>> politics is preventing that from happening, another solution has to be
>> found.  I doubt that XenServer is the only product which would benifit,
>> which is why the changes are presented for upstreaming.
>>
>>>>  3) Change the conring buffer to use lower memory when
instructed.
>>> Why does this one need moving, but none of the other Xen data
>>> structures? If anything, I would have expected you to limit the Xen
>>> heap altogether, so that automatically all allocations get done in
a
>>> way so that at least Xen''s data structures are available
in the
>>> (truncated) dump.
>> This is part of the "min" option which is trying to have the
least
>> possible impact.  The idea is to have this in low memory, use the
>> "console_to_ring" boot option to copy dom0 dmesg into
conring, and pass
>> its physical address and size in a crash note, so that the crash kernel
>> environment grab it all.
> Why is the console ring *that* important? I would have thought
> that proper register values and stack contents are much more
> significant for analysis of a crash.
>
> Jan
In combination with "console_to_ring" on the Xen command line, the
dom0
serial console will be copied into xen console ring.  In the case of a
crash, it is highly likely that one or other of them have panic()''d
into
their respective console rings.  It is not a foolproof solution, but in
will work in the general case.

The register contents of the pcpus which were running will be available
in the PR_STATUS notes which are also deliberately allocated in low
memory by this patch.  To the best of my understanding; to get to the
dom0 vcpu state, the crashkernel needs access to the domain structs and
vcpu structs (which I believe are actually allocated below 4GiB) and the
Xen page tables which dom0 uses (which inspecting CR3 from the crash
notes is certainly not in lower memory).  I suspect that there is also
more which needs to be allocated in lower memory to get a full register
dump, stack dump, stack trace etc.

The plan is to also have the "all" option from the command line which
will also allocate the page tables (and other structures where relevant)
in lower memory, but this is rather more of an overhead than just the
console ring and crash notes, which will have a more visible impact to
customers running 32bit PV guests.  This is the reason for separating
the two via a command line argument.

~Andrew

>>> Andrew Cooper <andrew.cooper3@citrix.com> 12/31/11 1:12 AM
>>>
>The register contents of the pcpus which were running will be available
>in the PR_STATUS notes which are also deliberately allocated in low
>memory by this patch.  To the best of my understanding; to get to the
>dom0 vcpu state, the crashkernel needs access to the domain structs and
>vcpu structs (which I believe are actually allocated below 4GiB) and the
The vCPU ones are, while the domain one isn''t.
>Xen page tables which dom0 uses (which inspecting CR3 from the crash
>notes is certainly not in lower memory).  I suspect that there is also
>more which needs to be allocated in lower memory to get a full register
>dump, stack dump, stack trace etc.
>
>The plan is to also have the "all" option from the command line
which
>will also allocate the page tables (and other structures where relevant)
>in lower memory, but this is rather more of an overhead than just the
>console ring and crash notes, which will have a more visible impact to
>customers running 32bit PV guests.  This is the reason for separating
>the two via a command line argument.
I''m wondering whether, with much less impact on existing code, and as
already pointed out, restricting the allocation range of
alloc_xenheap_pages() (by way of a command line option) wouldn''t get
you what you want.

Jan

On 31/12/11 07:38, Jan Beulich wrote:
>>>> Andrew Cooper <andrew.cooper3@citrix.com> 12/31/11 1:12
AM >>>
>> The register contents of the pcpus which were running will be available
>> in the PR_STATUS notes which are also deliberately allocated in low
>> memory by this patch.  To the best of my understanding; to get to the
>> dom0 vcpu state, the crashkernel needs access to the domain structs and
>> vcpu structs (which I believe are actually allocated below 4GiB) and
the
> The vCPU ones are, while the domain one isn''t.
Right - that is useful to know.
>> Xen page tables which dom0 uses (which inspecting CR3 from the crash
>> notes is certainly not in lower memory).  I suspect that there is also
>> more which needs to be allocated in lower memory to get a full register
>> dump, stack dump, stack trace etc.
>>
>> The plan is to also have the "all" option from the command
line which
>> will also allocate the page tables (and other structures where
relevant)
>> in lower memory, but this is rather more of an overhead than just the
>> console ring and crash notes, which will have a more visible impact to
>> customers running 32bit PV guests.  This is the reason for separating
>> the two via a command line argument.
> I''m wondering whether, with much less impact on existing code, and
as
> already pointed out, restricting the allocation range of
> alloc_xenheap_pages() (by way of a command line option) wouldn''t
get
> you what you want.
>
> Jan
Assuming that all relevant structures do in fact get allocated by
alloc_xenheap_pages() (which is probably a safe assumption looking at
the code, but I am not certain) then yes, it would have much less impact
on the existing code.  However, it would have a much bigger impact on
the other memory restricted items.  It might be possible (and perhaps a
good idea when extreme debugging is needed) to have a 3rd command line
option for low_crashinfo= which does this, but I dont like it as the
general solution to this problem.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com