[v6,0/2] arm64: kdump: simplify the reservation behaviour of crashkernel=,high

Message ID 20230515060259.830662-1-bhe@redhat.com
Headers
Series arm64: kdump: simplify the reservation behaviour of crashkernel=,high |

Message

Baoquan He May 15, 2023, 6:02 a.m. UTC
  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/20230407022419.19412-1-bhe@redhat.com/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
  

Comments

Catalin Marinas June 9, 2023, 7:30 p.m. UTC | #1
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
  
Baoquan He June 11, 2023, 12:35 a.m. UTC | #2
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!
  
Baoquan He June 11, 2023, 12:15 p.m. UTC | #3
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/202306110549.ynH2Juok-lkp@intel.com/


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
  
Catalin Marinas June 11, 2023, 2:31 p.m. UTC | #4
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/202306110549.ynH2Juok-lkp@intel.com/
> 
> 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.
  
Baoquan He June 11, 2023, 11:06 p.m. UTC | #5
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/202306110549.ynH2Juok-lkp@intel.com/
> > 
> > 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.