In v5 patch, Catalin helped review and acked the patch. However, an
uninitialized local varilable is warned out by static checker when Will
tried to merge the patch. And Will complained the code flow in
reserve_crashkernel() is hard to follow, required to refactor. While
when I tried to do the refactory, I feel it's not easy, the existing
several cases causes that.
To make the code easier understand, I try my best to compose a document
to introduce the background, concept and implementation strategies of
crashkernel reservation. Hope it can help people to understand the code
flow a little more easily.
[PATCH v5] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
https://lore.kernel.org/all/[email protected]/T/#u
v5->v6:
- Fix the warning reported by static checker about "uninitialized symbol
'search_base'".
- Add a document Documentation/arm64/kdump.rst to explain how to reserve
crashkernel.
Baoquan He (2):
arm64: kdump: simplify the reservation behaviour of crashkernel=,high
Documentation: add kdump.rst to present crashkernel reservation on
arm64
Documentation/arm64/kdump.rst | 103 ++++++++++++++++++++++++++++++++++
arch/arm64/mm/init.c | 44 +++++++++++----
2 files changed, 137 insertions(+), 10 deletions(-)
create mode 100644 Documentation/arm64/kdump.rst
--
2.34.1
People complained the crashkernel reservation code flow is hard to
follow, so add this document to explain the background, concepts and
implementation of crashkernel reservation on arm64. Hope this can help
people to understand it more easily.
Signed-off-by: Baoquan He <[email protected]>
---
Documentation/arm64/kdump.rst | 103 ++++++++++++++++++++++++++++++++++
1 file changed, 103 insertions(+)
create mode 100644 Documentation/arm64/kdump.rst
diff --git a/Documentation/arm64/kdump.rst b/Documentation/arm64/kdump.rst
new file mode 100644
index 000000000000..78b22017c490
--- /dev/null
+++ b/Documentation/arm64/kdump.rst
@@ -0,0 +1,103 @@
+=======================================
+crashkernel memory reservation on arm64
+=======================================
+
+Author: Baoquan He <[email protected]>
+
+Kdump mechanism is utilized to capture corrupted kernel's vmcore so
+that people can analyze it to get the root cause of corruption. In
+order to do that, a preliminarily reserved memory is needed to load
+in kdump kernel, and switch to kdump kernel to boot up and run if
+corruption happened.
+
+That reserved memory for kdump is adapted to be able to minimally
+accommodate kdump kernel to boot and run, and user space programs
+running to do the vmcore collecting.
+
+Kernel parameter
+================
+Through kernel parameter like below, memory can be reserved
+accordingly during early stage of 1st kernel's bootup so that
+continuous large chunk of memomy can be found and reserved. Meanwhile,
+the need of low memory need be considered if crashkernel is reserved
+in high memory area.
+
+- crashkernel=size@offset
+- crashkernel=size
+- crashkernel=size,high crashkernel=size,low
+
+Low memory and high memory
+===============
+What is low memory and high memory? In kdump reservation, low memory
+means the memory area under a specific limitation, and it's usually
+decided by the lowest addressing bits of PCI devices which kdump kernel
+need rely on to boot up and collect vmcore successfully. Those devices
+not related to vmcore dumping can be ignored, e.g on x86, those i2c may
+only be able to access 24bits addressing area, but kdump kernel still
+take 4G as the limitation because all known devices that kdump kernel
+cares about have 32bits addressing ability. On arm64, the low memory
+upper boundary is not fixed, it's 1G on RPi4 platform, while 4G on normal
+arm64 system. On the special system with CONFIG_ZONE_DMA|DMA32 disabled,
+the whole system RAM is low memory. Except of low memory, all the rest
+of system RAM is high memory which kernel and user space programs can
+require to allocate and use.
+
+Implementation
+==============
+1)crashkernel=size@offset
+-------------------------
+crashkernel memory must be reserved at the user specified region, otherwise
+fail if already occupied.
+
+
+2) crashkernel=size
+-------------------
+crashkernel memory region will be reserved in any available position
+according to searching order.
+
+Firstly, it searches the low memory area for an available region with specified
+size.
+
+Secondly, if searching low memory failed, fallback to search the high memory
+area with the specified size. Meanwhile, if the reservation in high memory
+succeeds, a default reservation in low memory will be done, the current default
+value is 128M which is satisfying the low memory needs, e.g pci device driver
+initialization.
+
+If both the above searching failed, the reservation will fail finally.
+
+Note: crashkernel=size is recommended option among crashkernel kernel
+parameters. With it, user doesn't need to know much about system memory
+information, just need to specify whatever memory kdump kernel needs to
+make vmcore dumping succeed.
+
+3) crashkernel=size,high crashkernel=size,low
+--------------------------------------------
+crashkernel=size,high is an important supplement to crashkernel=size. It
+allows user to precisely specify how much memory need be allocated from
+high memory, and how much memory is needed from low memory. On system
+with large memory, low memory is small and precious since some kernel
+feature and many devices can only request memory from the area, while
+requiring a large chunk of continuous memory from high memory area doesn't
+matter much and can satisfy most of kernel and almost all user space
+programs' requirement. In such case, only a small part of necessary memory
+from low memory area can satisfy needs. With it, the 1st kernel's normal
+running won't be impacted because of limited low memory resource.
+
+To reserve memory for crashkernel=size,high, firstly, searching is tried in
+high memory region. If reservation succeeds, low memory reservaton will be
+done subsequently.
+
+Secondly, if reservation in high memory failed, fallback to search the
+low memory with the specified size in crsahkernel=,high. If succeeds,
+everything is fine since no low memory is needed.
+
+Notes:
+- If crashkernel=,low is not specified, the default low memory reservation
+ will be done automically.
+
+- if crashkernel=0,low is specified, means that low memory reservation is
+ ommited intentionally.
+
+3)
+
--
2.34.1
On 2023/5/15 14:02, Baoquan He wrote:
> People complained the crashkernel reservation code flow is hard to
> follow, so add this document to explain the background, concepts and
> implementation of crashkernel reservation on arm64. Hope this can help
> people to understand it more easily.
Reviewed-by: Zhen Lei <[email protected]>
>
> Signed-off-by: Baoquan He <[email protected]>
> ---
> Documentation/arm64/kdump.rst | 103 ++++++++++++++++++++++++++++++++++
> 1 file changed, 103 insertions(+)
> create mode 100644 Documentation/arm64/kdump.rst
>
> diff --git a/Documentation/arm64/kdump.rst b/Documentation/arm64/kdump.rst
> new file mode 100644
> index 000000000000..78b22017c490
> --- /dev/null
> +++ b/Documentation/arm64/kdump.rst
> @@ -0,0 +1,103 @@
> +=======================================
> +crashkernel memory reservation on arm64
> +=======================================
> +
> +Author: Baoquan He <[email protected]>
> +
> +Kdump mechanism is utilized to capture corrupted kernel's vmcore so
> +that people can analyze it to get the root cause of corruption. In
> +order to do that, a preliminarily reserved memory is needed to load
> +in kdump kernel, and switch to kdump kernel to boot up and run if
> +corruption happened.
> +
> +That reserved memory for kdump is adapted to be able to minimally
> +accommodate kdump kernel to boot and run, and user space programs
> +running to do the vmcore collecting.
> +
> +Kernel parameter
> +================
> +Through kernel parameter like below, memory can be reserved
> +accordingly during early stage of 1st kernel's bootup so that
> +continuous large chunk of memomy can be found and reserved. Meanwhile,
> +the need of low memory need be considered if crashkernel is reserved
> +in high memory area.
> +
> +- crashkernel=size@offset
> +- crashkernel=size
> +- crashkernel=size,high crashkernel=size,low
> +
> +Low memory and high memory
> +===============
> +What is low memory and high memory? In kdump reservation, low memory
> +means the memory area under a specific limitation, and it's usually
> +decided by the lowest addressing bits of PCI devices which kdump kernel
> +need rely on to boot up and collect vmcore successfully. Those devices
> +not related to vmcore dumping can be ignored, e.g on x86, those i2c may
> +only be able to access 24bits addressing area, but kdump kernel still
> +take 4G as the limitation because all known devices that kdump kernel
> +cares about have 32bits addressing ability. On arm64, the low memory
> +upper boundary is not fixed, it's 1G on RPi4 platform, while 4G on normal
> +arm64 system. On the special system with CONFIG_ZONE_DMA|DMA32 disabled,
> +the whole system RAM is low memory. Except of low memory, all the rest
> +of system RAM is high memory which kernel and user space programs can
> +require to allocate and use.
> +
> +Implementation
> +==============
> +1)crashkernel=size@offset
> +-------------------------
> +crashkernel memory must be reserved at the user specified region, otherwise
> +fail if already occupied.
> +
> +
> +2) crashkernel=size
> +-------------------
> +crashkernel memory region will be reserved in any available position
> +according to searching order.
> +
> +Firstly, it searches the low memory area for an available region with specified
> +size.
> +
> +Secondly, if searching low memory failed, fallback to search the high memory
> +area with the specified size. Meanwhile, if the reservation in high memory
> +succeeds, a default reservation in low memory will be done, the current default
> +value is 128M which is satisfying the low memory needs, e.g pci device driver
> +initialization.
> +
> +If both the above searching failed, the reservation will fail finally.
> +
> +Note: crashkernel=size is recommended option among crashkernel kernel
> +parameters. With it, user doesn't need to know much about system memory
> +information, just need to specify whatever memory kdump kernel needs to
> +make vmcore dumping succeed.
> +
> +3) crashkernel=size,high crashkernel=size,low
> +--------------------------------------------
> +crashkernel=size,high is an important supplement to crashkernel=size. It
> +allows user to precisely specify how much memory need be allocated from
> +high memory, and how much memory is needed from low memory. On system
> +with large memory, low memory is small and precious since some kernel
> +feature and many devices can only request memory from the area, while
> +requiring a large chunk of continuous memory from high memory area doesn't
> +matter much and can satisfy most of kernel and almost all user space
> +programs' requirement. In such case, only a small part of necessary memory
> +from low memory area can satisfy needs. With it, the 1st kernel's normal
> +running won't be impacted because of limited low memory resource.
> +
> +To reserve memory for crashkernel=size,high, firstly, searching is tried in
> +high memory region. If reservation succeeds, low memory reservaton will be
> +done subsequently.
> +
> +Secondly, if reservation in high memory failed, fallback to search the
> +low memory with the specified size in crsahkernel=,high. If succeeds,
> +everything is fine since no low memory is needed.
> +
> +Notes:
> +- If crashkernel=,low is not specified, the default low memory reservation
> + will be done automically.
> +
> +- if crashkernel=0,low is specified, means that low memory reservation is
> + ommited intentionally.
ommited --> omitted
> +
> +3)
This line seems to be deleted.
> +
>
--
Regards,
Zhen Lei
On Mon, 15 May 2023 14:02:57 +0800, Baoquan He wrote:
> In v5 patch, Catalin helped review and acked the patch. However, an
> uninitialized local varilable is warned out by static checker when Will
> tried to merge the patch. And Will complained the code flow in
> reserve_crashkernel() is hard to follow, required to refactor. While
> when I tried to do the refactory, I feel it's not easy, the existing
> several cases causes that.
>
> [...]
Applied to arm64 (for-next/kdump).
I reworte some of the paragraphs in the documentation patch, removed
some sentences to make it easier to read (some details were pretty
obvious). Please have a look, if you think I missed something important,
just send a patch on top. Thanks.
[1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
https://git.kernel.org/arm64/c/6c4dcaddbd36
[2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
https://git.kernel.org/arm64/c/03dc0e05407f
--
Catalin
On 06/09/23 at 08:30pm, Catalin Marinas wrote:
> On Mon, 15 May 2023 14:02:57 +0800, Baoquan He wrote:
> > In v5 patch, Catalin helped review and acked the patch. However, an
> > uninitialized local varilable is warned out by static checker when Will
> > tried to merge the patch. And Will complained the code flow in
> > reserve_crashkernel() is hard to follow, required to refactor. While
> > when I tried to do the refactory, I feel it's not easy, the existing
> > several cases causes that.
> >
> > [...]
>
> Applied to arm64 (for-next/kdump).
>
> I reworte some of the paragraphs in the documentation patch, removed
> some sentences to make it easier to read (some details were pretty
> obvious). Please have a look, if you think I missed something important,
> just send a patch on top. Thanks.
>
> [1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
> https://git.kernel.org/arm64/c/6c4dcaddbd36
> [2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
> https://git.kernel.org/arm64/c/03dc0e05407f
Thanks a lot, Catalin. The rewriting looks great!
Hi Catalin,
On 06/09/23 at 08:30pm, Catalin Marinas wrote:
> On Mon, 15 May 2023 14:02:57 +0800, Baoquan He wrote:
> > In v5 patch, Catalin helped review and acked the patch. However, an
> > uninitialized local varilable is warned out by static checker when Will
> > tried to merge the patch. And Will complained the code flow in
> > reserve_crashkernel() is hard to follow, required to refactor. While
> > when I tried to do the refactory, I feel it's not easy, the existing
> > several cases causes that.
> >
> > [...]
>
> Applied to arm64 (for-next/kdump).
>
> I reworte some of the paragraphs in the documentation patch, removed
> some sentences to make it easier to read (some details were pretty
> obvious). Please have a look, if you think I missed something important,
> just send a patch on top. Thanks.
>
> [1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
> https://git.kernel.org/arm64/c/6c4dcaddbd36
> [2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
> https://git.kernel.org/arm64/c/03dc0e05407f
Could you help add below code change into the document patch commit? I
forgot adding it and got warning report from lkp test robot.
https://lore.kernel.org/oe-kbuild-all/[email protected]/
diff --git a/Documentation/arm64/index.rst b/Documentation/arm64/index.rst
index ae21f8118830..dcfebddb6088 100644
--- a/Documentation/arm64/index.rst
+++ b/Documentation/arm64/index.rst
@@ -25,6 +25,7 @@ ARM64 Architecture
sve
tagged-address-abi
tagged-pointers
+ kdump
features
On Sun, Jun 11, 2023 at 08:15:07PM +0800, Baoquan He wrote:
> On 06/09/23 at 08:30pm, Catalin Marinas wrote:
> > On Mon, 15 May 2023 14:02:57 +0800, Baoquan He wrote:
> > > In v5 patch, Catalin helped review and acked the patch. However, an
> > > uninitialized local varilable is warned out by static checker when Will
> > > tried to merge the patch. And Will complained the code flow in
> > > reserve_crashkernel() is hard to follow, required to refactor. While
> > > when I tried to do the refactory, I feel it's not easy, the existing
> > > several cases causes that.
> > >
> > > [...]
> >
> > Applied to arm64 (for-next/kdump).
> >
> > I reworte some of the paragraphs in the documentation patch, removed
> > some sentences to make it easier to read (some details were pretty
> > obvious). Please have a look, if you think I missed something important,
> > just send a patch on top. Thanks.
> >
> > [1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
> > https://git.kernel.org/arm64/c/6c4dcaddbd36
> > [2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
> > https://git.kernel.org/arm64/c/03dc0e05407f
>
> Could you help add below code change into the document patch commit? I
> forgot adding it and got warning report from lkp test robot.
>
> https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> diff --git a/Documentation/arm64/index.rst b/Documentation/arm64/index.rst
> index ae21f8118830..dcfebddb6088 100644
> --- a/Documentation/arm64/index.rst
> +++ b/Documentation/arm64/index.rst
> @@ -25,6 +25,7 @@ ARM64 Architecture
> sve
> tagged-address-abi
> tagged-pointers
> + kdump
I've seen the warning as well. Please send a patch fixing this as I try
to avoid rebasing. Also we keep this part in alphabetical order.
Thanks.
--
Catalin
On 06/11/23 at 03:31pm, Catalin Marinas wrote:
> On Sun, Jun 11, 2023 at 08:15:07PM +0800, Baoquan He wrote:
> > On 06/09/23 at 08:30pm, Catalin Marinas wrote:
> > > On Mon, 15 May 2023 14:02:57 +0800, Baoquan He wrote:
> > > > In v5 patch, Catalin helped review and acked the patch. However, an
> > > > uninitialized local varilable is warned out by static checker when Will
> > > > tried to merge the patch. And Will complained the code flow in
> > > > reserve_crashkernel() is hard to follow, required to refactor. While
> > > > when I tried to do the refactory, I feel it's not easy, the existing
> > > > several cases causes that.
> > > >
> > > > [...]
> > >
> > > Applied to arm64 (for-next/kdump).
> > >
> > > I reworte some of the paragraphs in the documentation patch, removed
> > > some sentences to make it easier to read (some details were pretty
> > > obvious). Please have a look, if you think I missed something important,
> > > just send a patch on top. Thanks.
> > >
> > > [1/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
> > > https://git.kernel.org/arm64/c/6c4dcaddbd36
> > > [2/2] Documentation: add kdump.rst to present crashkernel reservation on arm64
> > > https://git.kernel.org/arm64/c/03dc0e05407f
> >
> > Could you help add below code change into the document patch commit? I
> > forgot adding it and got warning report from lkp test robot.
> >
> > https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >
> > diff --git a/Documentation/arm64/index.rst b/Documentation/arm64/index.rst
> > index ae21f8118830..dcfebddb6088 100644
> > --- a/Documentation/arm64/index.rst
> > +++ b/Documentation/arm64/index.rst
> > @@ -25,6 +25,7 @@ ARM64 Architecture
> > sve
> > tagged-address-abi
> > tagged-pointers
> > + kdump
>
> I've seen the warning as well. Please send a patch fixing this as I try
> to avoid rebasing. Also we keep this part in alphabetical order.
Has sent a patch to fix that, thanks.