[PATCHv2 0/4] add support for 64k variant

[Pingfan Liu]

6k aarch64 has significant memory consumption more than 4k variant, and
requires an separated crashkernel formula.

This series generates the formula dynamiclly based on the system
environment.

v1->v2:
split out a common function _crashkernel_add()


Pingfan Liu (4):
  kdump-lib: Introduce a help function _crashkernel_add()
  unit tests: Add test case for _crashkernel_add()
  kdump-lib: add support for 64K aarch64
  kdump-lib: Evaluate the memory consumption by smmu and mlx5 separately

 kdump-lib.sh           | 101 ++++++++++++++++++++++++++++++++++++++++-
 kdumpctl               |  24 ++++++++++
 spec/kdump-lib_spec.sh |  17 +++++++
 3 files changed, 141 insertions(+), 1 deletion(-)

-- 
2.31.1

[PATCHv2 1/4] kdump-lib: Introduce a help function _crashkernel_add()

[Pingfan Liu]

This help function can manipulate the crashkernel cmdline by adding an
number for each item.

Credit to Philipp Rudo, who brings up the original code.

Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
---
 kdump-lib.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/kdump-lib.sh b/kdump-lib.sh
index 6b0a83d..f56a011 100755
--- a/kdump-lib.sh
+++ b/kdump-lib.sh
@@ -906,6 +906,64 @@ get_recommend_size()
 	echo "0M"
 }
 
+# $1 crashkernel=""
+# $2 delta in unit of MB
+_crashkernel_add()
+{
+	local _ck _add _entry _ret
+	local _range _size _offset
+
+	_ck="$1"
+	_add="$2"
+	_ret=""
+
+	if [[ "$_ck" == *@* ]]; then
+		_offset="@${_ck##*@}"
+		_ck=${_ck%@*}
+	elif [[ "$_ck" == *,high ]] || [[ "$_ck" == *,low ]]; then
+		_offset=",${_ck##*,}"
+		_ck=${_ck%,*}
+	else
+		_offset=''
+	fi
+
+	while read -d , -r _entry; do
+		[[ -n "$_entry" ]] || continue
+		if [[ "$_entry" == *:* ]]; then
+			_range=${_entry%:*}
+			_size=${_entry#*:}
+		else
+			_range=""
+			_size=${_entry}
+		fi
+
+		case "${_size: -1}" in
+			K)
+				_size=${_size::-1}
+				_size="$((_size + (_add * 1024)))K"
+				;;
+			M)
+				_size=${_size::-1}
+				_size="$((_size + _add))M"
+				;;
+			G)
+				_size=${_size::-1}
+				_size="$((_size + (_add / 1024)))G"
+				;;
+			*)
+				_size="$((_size + (_add * 1024 * 1024)))"
+				;;
+		esac
+
+		[[ -n "$_range" ]] && _ret+="$_range:"
+		_ret+="$_size,"
+	done <<< "$_ck,"
+
+	_ret=${_ret%,}
+	[[ -n "$_offset" ]] && _ret+=$_offset
+	echo "$_ret"
+}
+
 # get default crashkernel
 # $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable
 kdump_get_arch_recommend_crashkernel()
-- 
2.31.1

[PATCHv2 1/4] kdump-lib: Introduce a help function _crashkernel_add()

[Philipp Rudo]

On Thu, 18 May 2023 13:26:40 +0800
Pingfan Liu <piliu(a)redhat.com&gt; wrote:

>  This help function can manipulate the crashkernel cmdline by adding
> an
>  number for each item.
>  
>  Credit to Philipp Rudo, who brings up the original code.
>  
>  Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt; 
@Coiby, could you please take a look as I've provided the original code.

BTW, this function has one quirk I'm not sure if it is desired. In
particular by keeping the unit of an entry it will ignore any increase
smaller than 1024M to entries with size G. E.g.

_crashkernel_add 1G 1024 -> 2G

but

_crashkernel_add 1G 1023 -> 1G

Alternatively, we could convert everything to bytes first and then use
the best matching unit, e.g.

_crashkernel_add 1G 1 -> 1025M
_crashkernel_add 1024K 1 -> 2M

But that will add more complexity and I'm not sure if it will over
engineered for our use case.

Thanks
Philipp

>  ---
>   kdump-lib.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 58 insertions(+)
>  
>  diff --git a/kdump-lib.sh b/kdump-lib.sh
>  index 6b0a83d..f56a011 100755
>  --- a/kdump-lib.sh
>  +++ b/kdump-lib.sh
>  @@ -906,6 +906,64 @@ get_recommend_size()
>   	echo "0M"
>   }
>   
>  +# $1 crashkernel=""
>  +# $2 delta in unit of MB
>  +_crashkernel_add()
>  +{
>  +	local _ck _add _entry _ret
>  +	local _range _size _offset
>  +
>  +	_ck="$1"
>  +	_add="$2"
>  +	_ret=""
>  +
>  +	if [[ "$_ck" == *@* ]]; then
>  +		_offset="@${_ck##*@}"
>  +		_ck=${_ck%@*}
>  +	elif [[ "$_ck" == *,high ]] || [[ "$_ck" == *,low ]]; then
>  +		_offset=",${_ck##*,}"
>  +		_ck=${_ck%,*}
>  +	else
>  +		_offset=''
>  +	fi
>  +
>  +	while read -d , -r _entry; do
>  +		[[ -n "$_entry" ]] || continue
>  +		if [[ "$_entry" == *:* ]]; then
>  +			_range=${_entry%:*}
>  +			_size=${_entry#*:}
>  +		else
>  +			_range=""
>  +			_size=${_entry}
>  +		fi
>  +
>  +		case "${_size: -1}" in
>  +			K)
>  +				_size=${_size::-1}
>  +				_size="$((_size + (_add * 1024)))K"
>  +				;;
>  +			M)
>  +				_size=${_size::-1}
>  +				_size="$((_size + _add))M"
>  +				;;
>  +			G)
>  +				_size=${_size::-1}
>  +				_size="$((_size + (_add / 1024)))G"
>  +				;;
>  +			*)
>  +				_size="$((_size + (_add * 1024 * 1024)))"
>  +				;;
>  +		esac
>  +
>  +		[[ -n "$_range" ]] && _ret+="$_range:"
>  +		_ret+="$_size,"
>  +	done <<< "$_ck,"
>  +
>  +	_ret=${_ret%,}
>  +	[[ -n "$_offset" ]] && _ret+=$_offset
>  +	echo "$_ret"
>  +}
>  +
>   # get default crashkernel
>   # $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable
>   kdump_get_arch_recommend_crashkernel()

[PATCHv2 1/4] kdump-lib: Introduce a help function _crashkernel_add()

[Pingfan Liu]

On Mon, May 22, 2023 at 10:59 PM Philipp Rudo <prudo(a)redhat.com&gt; wrote:
>  On Thu, 18 May 2023 13:26:40 +0800
>  Pingfan Liu <piliu(a)redhat.com&gt; wrote:
>
>  > This help function can manipulate the crashkernel cmdline by adding an
>  > number for each item.
>  >
>  > Credit to Philipp Rudo, who brings up the original code.
>  >
>  > Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
>
>  @Coiby, could you please take a look as I've provided the original code.
>
>  BTW, this function has one quirk I'm not sure if it is desired. In
>  particular by keeping the unit of an entry it will ignore any increase
>  smaller than 1024M to entries with size G. E.g.
>
>  _crashkernel_add 1G 1024 -> 2G
>
>  but
>
>  _crashkernel_add 1G 1023 -> 1G
>
>  Alternatively, we could convert everything to bytes first and then use
>  the best matching unit, e.g.
>  
Since in practice, the delta is in the unit of MB, I suggest
converting it to MB if the unit is bigger than that.

Thanks,

Pingfan


>  _crashkernel_add 1G 1 -> 1025M
>  _crashkernel_add 1024K 1 -> 2M
>
>  But that will add more complexity and I'm not sure if it will over
>  engineered for our use case.
>
>  Thanks
>  Philipp
>
>  > ---
>  >  kdump-lib.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  >  1 file changed, 58 insertions(+)
>  >
>  > diff --git a/kdump-lib.sh b/kdump-lib.sh
>  > index 6b0a83d..f56a011 100755
>  > --- a/kdump-lib.sh
>  > +++ b/kdump-lib.sh
>  > @@ -906,6 +906,64 @@ get_recommend_size()
>  >       echo "0M"
>  >  }
>  >
>  > +# $1 crashkernel=""
>  > +# $2 delta in unit of MB
>  > +_crashkernel_add()
>  > +{
>  > +     local _ck _add _entry _ret
>  > +     local _range _size _offset
>  > +
>  > +     _ck="$1"
>  > +     _add="$2"
>  > +     _ret=""
>  > +
>  > +     if [[ "$_ck" == *@* ]]; then
>  > +             _offset="@${_ck##*@}"
>  > +             _ck=${_ck%@*}
>  > +     elif [[ "$_ck" == *,high ]] || [[ "$_ck" == *,low ]];
> then
>  > +             _offset=",${_ck##*,}"
>  > +             _ck=${_ck%,*}
>  > +     else
>  > +             _offset=''
>  > +     fi
>  > +
>  > +     while read -d , -r _entry; do
>  > +             [[ -n "$_entry" ]] || continue
>  > +             if [[ "$_entry" == *:* ]]; then
>  > +                     _range=${_entry%:*}
>  > +                     _size=${_entry#*:}
>  > +             else
>  > +                     _range=""
>  > +                     _size=${_entry}
>  > +             fi
>  > +
>  > +             case "${_size: -1}" in
>  > +                     K)
>  > +                             _size=${_size::-1}
>  > +                             _size="$((_size + (_add * 1024)))K"
>  > +                             ;;
>  > +                     M)
>  > +                             _size=${_size::-1}
>  > +                             _size="$((_size + _add))M"
>  > +                             ;;
>  > +                     G)
>  > +                             _size=${_size::-1}
>  > +                             _size="$((_size + (_add / 1024)))G"
>  > +                             ;;
>  > +                     *)
>  > +                             _size="$((_size + (_add * 1024 * 1024)))"
>  > +                             ;;
>  > +             esac
>  > +
>  > +             [[ -n "$_range" ]] && _ret+="$_range:"
>  > +             _ret+="$_size,"
>  > +     done <<< "$_ck,"
>  > +
>  > +     _ret=${_ret%,}
>  > +     [[ -n "$_offset" ]] && _ret+=$_offset
>  > +     echo "$_ret"
>  > +}
>  > +
>  >  # get default crashkernel
>  >  # $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable
>  >  kdump_get_arch_recommend_crashkernel()
>

[PATCHv2 1/4] kdump-lib: Introduce a help function _crashkernel_add()

[Philipp Rudo]

On Wed, 24 May 2023 11:48:45 +0800
Pingfan Liu <piliu(a)redhat.com&gt; wrote:

>  On Mon, May 22, 2023 at 10:59 PM Philipp Rudo
> <prudo(a)redhat.com&gt; wrote:
>  >
>  > On Thu, 18 May 2023 13:26:40 +0800
>  > Pingfan Liu <piliu(a)redhat.com&gt; wrote:
>  >  
>  > > This help function can manipulate the crashkernel cmdline by adding an
>  > > number for each item.
>  > >
>  > > Credit to Philipp Rudo, who brings up the original code.
>  > >
>  > > Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;  
>  >
>  > @Coiby, could you please take a look as I've provided the original code.
>  >
>  > BTW, this function has one quirk I'm not sure if it is desired. In
>  > particular by keeping the unit of an entry it will ignore any increase
>  > smaller than 1024M to entries with size G. E.g.
>  >
>  > _crashkernel_add 1G 1024 -> 2G
>  >
>  > but
>  >
>  > _crashkernel_add 1G 1023 -> 1G
>  >
>  > Alternatively, we could convert everything to bytes first and then use
>  > the best matching unit, e.g.
>  >  
>  
>  Since in practice, the delta is in the unit of MB, I suggest
>  converting it to MB if the unit is bigger than that. 
That's fine with me as well.

Thanks
Philipp


>  
>  Thanks,
>  
>  Pingfan
>  
>  
>  > _crashkernel_add 1G 1 -> 1025M
>  > _crashkernel_add 1024K 1 -> 2M
>  >
>  > But that will add more complexity and I'm not sure if it will over
>  > engineered for our use case.
>  >
>  > Thanks
>  > Philipp
>  >  
>  > > ---
>  > >  kdump-lib.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  > >  1 file changed, 58 insertions(+)
>  > >
>  > > diff --git a/kdump-lib.sh b/kdump-lib.sh
>  > > index 6b0a83d..f56a011 100755
>  > > --- a/kdump-lib.sh
>  > > +++ b/kdump-lib.sh
>  > > @@ -906,6 +906,64 @@ get_recommend_size()
>  > >       echo "0M"
>  > >  }
>  > >
>  > > +# $1 crashkernel=""
>  > > +# $2 delta in unit of MB
>  > > +_crashkernel_add()
>  > > +{
>  > > +     local _ck _add _entry _ret
>  > > +     local _range _size _offset
>  > > +
>  > > +     _ck="$1"
>  > > +     _add="$2"
>  > > +     _ret=""
>  > > +
>  > > +     if [[ "$_ck" == *@* ]]; then
>  > > +             _offset="@${_ck##*@}"
>  > > +             _ck=${_ck%@*}
>  > > +     elif [[ "$_ck" == *,high ]] || [[ "$_ck" == *,low ]];
> then
>  > > +             _offset=",${_ck##*,}"
>  > > +             _ck=${_ck%,*}
>  > > +     else
>  > > +             _offset=''
>  > > +     fi
>  > > +
>  > > +     while read -d , -r _entry; do
>  > > +             [[ -n "$_entry" ]] || continue
>  > > +             if [[ "$_entry" == *:* ]]; then
>  > > +                     _range=${_entry%:*}
>  > > +                     _size=${_entry#*:}
>  > > +             else
>  > > +                     _range=""
>  > > +                     _size=${_entry}
>  > > +             fi
>  > > +
>  > > +             case "${_size: -1}" in
>  > > +                     K)
>  > > +                             _size=${_size::-1}
>  > > +                             _size="$((_size + (_add * 1024)))K"
>  > > +                             ;;
>  > > +                     M)
>  > > +                             _size=${_size::-1}
>  > > +                             _size="$((_size + _add))M"
>  > > +                             ;;
>  > > +                     G)
>  > > +                             _size=${_size::-1}
>  > > +                             _size="$((_size + (_add / 1024)))G"
>  > > +                             ;;
>  > > +                     *)
>  > > +                             _size="$((_size + (_add * 1024 *
> 1024)))"
>  > > +                             ;;
>  > > +             esac
>  > > +
>  > > +             [[ -n "$_range" ]] &&
> _ret+="$_range:"
>  > > +             _ret+="$_size,"
>  > > +     done <<< "$_ck,"
>  > > +
>  > > +     _ret=${_ret%,}
>  > > +     [[ -n "$_offset" ]] && _ret+=$_offset
>  > > +     echo "$_ret"
>  > > +}
>  > > +
>  > >  # get default crashkernel
>  > >  # $1 dump mode, if not specified, dump_mode will be judged by
> is_fadump_capable
>  > >  kdump_get_arch_recommend_crashkernel()  
>  >  
>

[PATCHv2 2/4] unit tests: Add test case for _crashkernel_add()

[Pingfan Liu]

Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
---
 spec/kdump-lib_spec.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/spec/kdump-lib_spec.sh b/spec/kdump-lib_spec.sh
index 3d10006..d477b2b 100644
--- a/spec/kdump-lib_spec.sh
+++ b/spec/kdump-lib_spec.sh
@@ -48,6 +48,23 @@ Describe 'kdump-lib'
 		End
 	End
 
+	Describe "_crashkernel_add()"
+		Context "when the input parameter is
'1G-4G:256M,4G-64G:320M,64G-:576M'"
+			delta=100
+			Parameters
+				"1G-4G:256M,4G-64G:320M,64G-:576M"
"1G-4G:356M,4G-64G:420M,64G-:676M"
+				"1G-4G:256M,4G-64G:320M,64G-:576M@4G"
"1G-4G:356M,4G-64G:420M,64G-:676M@4G"
+				"300M,high" "400M,high"
+				"500M@1G" "600M@1G"
+			End
+			It "should add delta to the values after ':'"
+
+				When call _crashkernel_add "$1" "$delta"
+				The output should equal "$2"
+			End
+		End
+	End
+
 	Describe 'prepare_cmdline()'
 		get_bootcpu_apicid() {
 			echo 1
-- 
2.31.1

[PATCHv2 2/4] unit tests: Add test case for _crashkernel_add()

[Philipp Rudo]

Hi Pingfan,

personally I would squash this patch with the previous one. In my
opinion functions and test cases for them belong together.

On Thu, 18 May 2023 13:26:41 +0800
Pingfan Liu <piliu(a)redhat.com&gt; wrote:

>  Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
>  ---
>   spec/kdump-lib_spec.sh | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>  
>  diff --git a/spec/kdump-lib_spec.sh b/spec/kdump-lib_spec.sh
>  index 3d10006..d477b2b 100644
>  --- a/spec/kdump-lib_spec.sh
>  +++ b/spec/kdump-lib_spec.sh
>  @@ -48,6 +48,23 @@ Describe 'kdump-lib'
>   		End
>   	End
>   
>  +	Describe "_crashkernel_add()"
>  +		Context "when the input parameter is
> '1G-4G:256M,4G-64G:320M,64G-:576M'"
>  +			delta=100
>  +			Parameters
>  +				"1G-4G:256M,4G-64G:320M,64G-:576M"
> "1G-4G:356M,4G-64G:420M,64G-:676M"
>  +				"1G-4G:256M,4G-64G:320M,64G-:576M@4G"
> "1G-4G:356M,4G-64G:420M,64G-:676M@4G"
>  +				"300M,high" "400M,high"
>  +				"500M@1G" "600M@1G" 
For a complete test you should also add cases where the size is
not in M, but K and G (please also see my comment for patch 1).
As well as one test with ',low'.

Thanks
Philipp

>  +			End
>  +			It "should add delta to the values after ':'"
>  +
>  +				When call _crashkernel_add "$1" "$delta"
>  +				The output should equal "$2"
>  +			End
>  +		End
>  +	End
>  +
>   	Describe 'prepare_cmdline()'
>   		get_bootcpu_apicid() {
>   			echo 1

[PATCHv2 2/4] unit tests: Add test case for _crashkernel_add()

[Pingfan Liu]

On Mon, May 22, 2023 at 10:59 PM Philipp Rudo <prudo(a)redhat.com&gt; wrote:
>  Hi Pingfan,
>
>  personally I would squash this patch with the previous one. In my
>  opinion functions and test cases for them belong together.
>
>  On Thu, 18 May 2023 13:26:41 +0800
>  Pingfan Liu <piliu(a)redhat.com&gt; wrote:
>
>  > Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
>  > ---
>  >  spec/kdump-lib_spec.sh | 17 +++++++++++++++++
>  >  1 file changed, 17 insertions(+)
>  >
>  > diff --git a/spec/kdump-lib_spec.sh b/spec/kdump-lib_spec.sh
>  > index 3d10006..d477b2b 100644
>  > --- a/spec/kdump-lib_spec.sh
>  > +++ b/spec/kdump-lib_spec.sh
>  > @@ -48,6 +48,23 @@ Describe 'kdump-lib'
>  >               End
>  >       End
>  >
>  > +     Describe "_crashkernel_add()"
>  > +             Context "when the input parameter is
> '1G-4G:256M,4G-64G:320M,64G-:576M'"
>  > +                     delta=100
>  > +                     Parameters
>  > +                             "1G-4G:256M,4G-64G:320M,64G-:576M"
> "1G-4G:356M,4G-64G:420M,64G-:676M"
>  > +                             "1G-4G:256M,4G-64G:320M,64G-:576M@4G"
> "1G-4G:356M,4G-64G:420M,64G-:676M@4G"
>  > +                             "300M,high" "400M,high"
>  > +                             "500M@1G" "600M@1G"
>
>  For a complete test you should also add cases where the size is
>  not in M, but K and G (please also see my comment for patch 1).
>  As well as one test with ',low'.
>  
OK, I will supplement them.

Thanks,

Pingfan

>  Thanks
>  Philipp
>
>  > +                     End
>  > +                     It "should add delta to the values after
> ':'"
>  > +
>  > +                             When call _crashkernel_add "$1"
> "$delta"
>  > +                             The output should equal "$2"
>  > +                     End
>  > +             End
>  > +     End
>  > +
>  >       Describe 'prepare_cmdline()'
>  >               get_bootcpu_apicid() {
>  >                       echo 1
>

[PATCHv2 3/4] kdump-lib: add support for 64K aarch64

[Pingfan Liu]

From: Pingfan Liu <piliu at redhat.com>

On aarch64, both 4K and 64K kernel can be installed, while they demand
different size reserved memory for kdump kernel.

'get_conf PAGE_SIZE' can not work if installing a 64K kernel when
running a 4K kernel. Hence resorting to the kernel release naming rules.
At present, the 64K kernel has the keyword '64k' in its suffix.

The base line for 64K is decided based on 4K. The diff 120M is picked up
since on a high end machine without smmu enabled, the diff of MemFree is
82M.

As for the smmu case, a huge difference in the memory consumption lies
between 64k and 4k driver. And it should be calculated separatedly.

Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
---
 kdump-lib.sh | 21 ++++++++++++++++++++-
 kdumpctl     | 24 ++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/kdump-lib.sh b/kdump-lib.sh
index f56a011..e1d8da9 100755
--- a/kdump-lib.sh
+++ b/kdump-lib.sh
@@ -966,6 +966,7 @@ _crashkernel_add()
 
 # get default crashkernel
 # $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable
+# $2 kernel-release, if not specified, got by _get_kdump_kernel_version
 kdump_get_arch_recommend_crashkernel()
 {
 	local _arch _ck_cmdline _dump_mode
@@ -985,8 +986,26 @@ kdump_get_arch_recommend_crashkernel()
 	if [[ $_arch == "x86_64" ]] || [[ $_arch == "s390x" ]]; then
 		_ck_cmdline="1G-4G:192M,4G-64G:256M,64G-:512M"
 	elif [[ $_arch == "aarch64" ]]; then
-		# For 4KB page size, the formula is based on x86 plus extra = 64M
+		local _running_kernel
+		local _delta="0"
+
+		# Base line for 4K variant kernel. The formula is based on x86 plus extra = 64M
 		_ck_cmdline="1G-4G:256M,4G-64G:320M,64G-:576M"
+		if [[ -z "$2" ]]; then
+			_running_kernel=$(_get_kdump_kernel_version)
+		else
+			_running_kernel=$2
+		fi
+
+		# the naming convention of 64k variant suffixes with +64k, e.g.
"vmlinuz-5.14.0-312.el9.aarch64+64k"
+		if echo "$_running_kernel" | grep 64k; then
+			# Without smmu, the diff of MemFree between 4K and 64K measured on a high end aarch64
machine is 82M.
+			# Picking up 100M to cover this diff. And finally, we have
"1G-4G:356M;4G-64G:420M;64G-:676M"
+			_delta+=100
+		else
+			_delta+=0
+		fi
+		_ck_cmdline=$(_crashkernel_add "$_ck_cmdline" "$_delta")
 	elif [[ $_arch == "ppc64le" ]]; then
 		if [[ $_dump_mode == "fadump" ]]; then

			_ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"
diff --git a/kdumpctl b/kdumpctl
index 2bec428..cbdc45a 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -1623,6 +1623,7 @@ reset_crashkernel_for_installed_kernel()
 {
 	local _installed_kernel _running_kernel _crashkernel _crashkernel_running
 	local _dump_mode_running _fadump_val_running
+	local _arch
 
 	# During package install, only try to reset crashkernel for osbuild
 	# thus to avoid calling grubby when installing os via anaconda
@@ -1651,6 +1652,29 @@ reset_crashkernel_for_installed_kernel()
 	_dump_mode_running=$(get_dump_mode_by_kernel "$_running_kernel")
 	_fadump_val_running=$(get_grub_kernel_boot_parameter "$_kernel" fadump)
 
+	_arch=$(uname -m)
+	# On aarch64, there are 4K and 64K kernel, which demands different crashkernel value
+	if [[ $_arch == "aarch64" ]]; then
+		local _installed_64K _running_64K
+
+		_installed_64K=0
+		#echo "$_installed_kernel" | grep 64k
+		if echo "$_installed_kernel" | grep 64k; then
+			_installed_64K=1
+		fi
+		if echo "$_running_kernel" | grep 64k; then
+			_running_64K=1
+		fi
+
+		# Otherwise, fall through to common case
+		if [[ $_installed_64K != "$_running_64K" ]]; then
+			# if the difference is 4K VS 64K
+			_crashkernel_running=$(kdump_get_arch_recommend_crashkernel "kdump"
"$_installed_kernel")
+			_update_kernel_cmdline "$_installed_kernel"
"$_crashkernel_running" "$_dump_mode_running"
"$_fadump_val_running"
+			return
+		fi
+	fi
+
 	if [[ $_crashkernel != "$_crashkernel_running" ]]; then
 		if _update_kernel_cmdline "$_installed_kernel"
"$_crashkernel_running" "$_dump_mode_running"
"$_fadump_val_running"; then
 			echo "kexec-tools has reset $_installed_kernel to use the new default crashkernel
value $_crashkernel_running"
-- 
2.31.1

[PATCHv2 3/4] kdump-lib: add support for 64K aarch64

[Philipp Rudo]

Hi Pingfan,

On Thu, 18 May 2023 13:26:42 +0800
Pingfan Liu <piliu(a)redhat.com&gt; wrote:

>  From: Pingfan Liu <piliu at redhat.com>
>  
>  On aarch64, both 4K and 64K kernel can be installed, while they demand
>  different size reserved memory for kdump kernel.
>  
>  'get_conf PAGE_SIZE' can not work if installing a 64K kernel when
>  running a 4K kernel. Hence resorting to the kernel release naming rules.
>  At present, the 64K kernel has the keyword '64k' in its suffix.
>  
>  The base line for 64K is decided based on 4K. The diff 120M is picked up
>  since on a high end machine without smmu enabled, the diff of MemFree is
>  82M.
>  
>  As for the smmu case, a huge difference in the memory consumption lies
>  between 64k and 4k driver. And it should be calculated separatedly.
>  
>  Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
>  ---
>   kdump-lib.sh | 21 ++++++++++++++++++++-
>   kdumpctl     | 24 ++++++++++++++++++++++++
>   2 files changed, 44 insertions(+), 1 deletion(-)
>  
>  diff --git a/kdump-lib.sh b/kdump-lib.sh
>  index f56a011..e1d8da9 100755
>  --- a/kdump-lib.sh
>  +++ b/kdump-lib.sh
>  @@ -966,6 +966,7 @@ _crashkernel_add()
>   
>   # get default crashkernel
>   # $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable
>  +# $2 kernel-release, if not specified, got by _get_kdump_kernel_version
>   kdump_get_arch_recommend_crashkernel()
>   {
>   	local _arch _ck_cmdline _dump_mode
>  @@ -985,8 +986,26 @@ kdump_get_arch_recommend_crashkernel()
>   	if [[ $_arch == "x86_64" ]] || [[ $_arch == "s390x" ]]; then
>   		_ck_cmdline="1G-4G:192M,4G-64G:256M,64G-:512M"
>   	elif [[ $_arch == "aarch64" ]]; then
>  -		# For 4KB page size, the formula is based on x86 plus extra = 64M
>  +		local _running_kernel
>  +		local _delta="0"
>  +
>  +		# Base line for 4K variant kernel. The formula is based on x86 plus extra = 64M
>   		_ck_cmdline="1G-4G:256M,4G-64G:320M,64G-:576M"
>  +		if [[ -z "$2" ]]; then
>  +			_running_kernel=$(_get_kdump_kernel_version)
>  +		else
>  +			_running_kernel=$2
>  +		fi
>  +
>  +		# the naming convention of 64k variant suffixes with +64k, e.g.
> "vmlinuz-5.14.0-312.el9.aarch64+64k"
>  +		if echo "$_running_kernel" | grep 64k; then 
					     ^^^^
					     grep -q ? Otherwise the match will be printed to stdout.

>  +			# Without smmu, the diff of MemFree between 4K and 64K measured
> on a high end aarch64 machine is 82M.
>  +			# Picking up 100M to cover this diff. And finally, we have
> "1G-4G:356M;4G-64G:420M;64G-676M"
>  +			_delta+=100 
				^^^
				In the patch description you wrote that
				the value gets increased by 120M. Which
				one is correct?

Furthermore, I was sloppy when providing the example. While the line
above is valid bash code it won't work as intended. In particular when
you are using += in bash it evaluates both sides as strings, _not_ as
mathematical expressions. For example

$ a=1
$ a+=1
$ echo $a

will give '11', _not_ '2'. The correct syntax for what you want is 

$ (( _delta += 100 ))

(Note: the spaces after/before (( / )) are required. The other ones are
optional.)

>  +		else
>  +			_delta+=0
>  +		fi
>  +		_ck_cmdline=$(_crashkernel_add "$_ck_cmdline" "$_delta")
>   	elif [[ $_arch == "ppc64le" ]]; then
>   		if [[ $_dump_mode == "fadump" ]]; then
>  
> 			_ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"
>  diff --git a/kdumpctl b/kdumpctl
>  index 2bec428..cbdc45a 100755
>  --- a/kdumpctl
>  +++ b/kdumpctl
>  @@ -1623,6 +1623,7 @@ reset_crashkernel_for_installed_kernel()
>   {
>   	local _installed_kernel _running_kernel _crashkernel _crashkernel_running
>   	local _dump_mode_running _fadump_val_running
>  +	local _arch
>   
>   	# During package install, only try to reset crashkernel for osbuild
>   	# thus to avoid calling grubby when installing os via anaconda
>  @@ -1651,6 +1652,29 @@ reset_crashkernel_for_installed_kernel()
>   	_dump_mode_running=$(get_dump_mode_by_kernel "$_running_kernel")
>   	_fadump_val_running=$(get_grub_kernel_boot_parameter "$_kernel" fadump)
>   
>  +	_arch=$(uname -m)
>  +	# On aarch64, there are 4K and 64K kernel, which demands different crashkernel value
>  +	if [[ $_arch == "aarch64" ]]; then
>  +		local _installed_64K _running_64K
>  +
>  +		_installed_64K=0
>  +		#echo "$_installed_kernel" | grep 64k
>  +		if echo "$_installed_kernel" | grep 64k; then
>  +			_installed_64K=1
>  +		fi
>  +		if echo "$_running_kernel" | grep 64k; then
>  +			_running_64K=1
>  +		fi
>  +
>  +		# Otherwise, fall through to common case
>  +		if [[ $_installed_64K != "$_running_64K" ]]; then
>  +			# if the difference is 4K VS 64K
>  +			_crashkernel_running=$(kdump_get_arch_recommend_crashkernel "kdump"
> "$_installed_kernel")
>  +			_update_kernel_cmdline "$_installed_kernel"
> "$_crashkernel_running" "$_dump_mode_running"
> "$_fadump_val_running"
>  +			return
>  +		fi
>  +	fi
>  +
>   	if [[ $_crashkernel != "$_crashkernel_running" ]]; then
>   		if _update_kernel_cmdline "$_installed_kernel"
> "$_crashkernel_running" "$_dump_mode_running"
> "$_fadump_val_running"; then
>   			echo "kexec-tools has reset $_installed_kernel to use the new default
> crashkernel value $_crashkernel_running" 
Coiby sent the patch "Simplify the management of the kernel parameter
crashkernel" to the mailing list where he removes this part of
reset_crashkernel_for_installed_kernel. I believe you can drop
the changes to kdumpctl.

Thanks
Philipp

[PATCHv2 3/4] kdump-lib: add support for 64K aarch64

[Pingfan Liu]

On Mon, May 22, 2023 at 10:59 PM Philipp Rudo <prudo(a)redhat.com&gt; wrote:
>  Hi Pingfan,
>
>  On Thu, 18 May 2023 13:26:42 +0800
>  Pingfan Liu <piliu(a)redhat.com&gt; wrote:
>
>  > From: Pingfan Liu <piliu at redhat.com>
>  >
>  > On aarch64, both 4K and 64K kernel can be installed, while they demand
>  > different size reserved memory for kdump kernel.
>  >
>  > 'get_conf PAGE_SIZE' can not work if installing a 64K kernel when
>  > running a 4K kernel. Hence resorting to the kernel release naming rules.
>  > At present, the 64K kernel has the keyword '64k' in its suffix.
>  >
>  > The base line for 64K is decided based on 4K. The diff 120M is picked up
>  > since on a high end machine without smmu enabled, the diff of MemFree is
>  > 82M.
>  >
>  > As for the smmu case, a huge difference in the memory consumption lies
>  > between 64k and 4k driver. And it should be calculated separatedly.
>  >
>  > Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
>  > ---
>  >  kdump-lib.sh | 21 ++++++++++++++++++++-
>  >  kdumpctl     | 24 ++++++++++++++++++++++++
>  >  2 files changed, 44 insertions(+), 1 deletion(-)
>  >
>  > diff --git a/kdump-lib.sh b/kdump-lib.sh
>  > index f56a011..e1d8da9 100755
>  > --- a/kdump-lib.sh
>  > +++ b/kdump-lib.sh
>  > @@ -966,6 +966,7 @@ _crashkernel_add()
>  >
>  >  # get default crashkernel
>  >  # $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable
>  > +# $2 kernel-release, if not specified, got by _get_kdump_kernel_version
>  >  kdump_get_arch_recommend_crashkernel()
>  >  {
>  >       local _arch _ck_cmdline _dump_mode
>  > @@ -985,8 +986,26 @@ kdump_get_arch_recommend_crashkernel()
>  >       if [[ $_arch == "x86_64" ]] || [[ $_arch == "s390x" ]];
> then
>  >               _ck_cmdline="1G-4G:192M,4G-64G:256M,64G-:512M"
>  >       elif [[ $_arch == "aarch64" ]]; then
>  > -             # For 4KB page size, the formula is based on x86 plus extra = 64M
>  > +             local _running_kernel
>  > +             local _delta="0"
>  > +
>  > +             # Base line for 4K variant kernel. The formula is based on x86 plus
> extra = 64M
>  >               _ck_cmdline="1G-4G:256M,4G-64G:320M,64G-:576M"
>  > +             if [[ -z "$2" ]]; then
>  > +                     _running_kernel=$(_get_kdump_kernel_version)
>  > +             else
>  > +                     _running_kernel=$2
>  > +             fi
>  > +
>  > +             # the naming convention of 64k variant suffixes with +64k, e.g.
> "vmlinuz-5.14.0-312.el9.aarch64+64k"
>  > +             if echo "$_running_kernel" | grep 64k; then
>                                               ^^^^
>                                               grep -q ? Otherwise the match will be
> printed to stdout.
>  
Adopt, thanks.

>  > +                     # Without smmu, the diff of MemFree
> between 4K and 64K measured on a high end aarch64 machine is 82M.
>  > +                     # Picking up 100M to cover this diff. And finally, we have
> "1G-4G:356M;4G-64G:420M;64G-676M"
>  > +                     _delta+=100
>                                  ^^^
>                                  In the patch description you wrote that
>                                  the value gets increased by 120M. Which
>                                  one is correct?
>  
Nice catch. I will fix the description.

>  Furthermore, I was sloppy when providing the example. While the line
>  above is valid bash code it won't work as intended. In particular when
>  you are using += in bash it evaluates both sides as strings, _not_ as
>  mathematical expressions. For example
>
>  $ a=1
>  $ a+=1
>  $ echo $a
>
>  will give '11', _not_ '2'. The correct syntax for what you want is
>  
Oops, the C grammar is so deeply rooted in my mind that I am not aware
of it either, although I read such a topic just a few days ago.

>  $ (( _delta += 100 ))
>
>  (Note: the spaces after/before (( / )) are required. The other ones are
>  optional.) 
Sorry but I can not find any format restrictions around ((...)), and I
have done some test.

>  > +             else
>  > +                     _delta+=0
>  > +             fi
>  > +             _ck_cmdline=$(_crashkernel_add "$_ck_cmdline"
> "$_delta")
>  >       elif [[ $_arch == "ppc64le" ]]; then
>  >               if [[ $_dump_mode == "fadump" ]]; then
>  >                      
> _ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"
>  > diff --git a/kdumpctl b/kdumpctl
>  > index 2bec428..cbdc45a 100755
>  > --- a/kdumpctl
>  > +++ b/kdumpctl
>  > @@ -1623,6 +1623,7 @@ reset_crashkernel_for_installed_kernel()
>  >  {
>  >       local _installed_kernel _running_kernel _crashkernel _crashkernel_running
>  >       local _dump_mode_running _fadump_val_running
>  > +     local _arch
>  >
>  >       # During package install, only try to reset crashkernel for osbuild
>  >       # thus to avoid calling grubby when installing os via anaconda
>  > @@ -1651,6 +1652,29 @@ reset_crashkernel_for_installed_kernel()
>  >       _dump_mode_running=$(get_dump_mode_by_kernel "$_running_kernel")
>  >       _fadump_val_running=$(get_grub_kernel_boot_parameter "$_kernel"
> fadump)
>  >
>  > +     _arch=$(uname -m)
>  > +     # On aarch64, there are 4K and 64K kernel, which demands different crashkernel
> value
>  > +     if [[ $_arch == "aarch64" ]]; then
>  > +             local _installed_64K _running_64K
>  > +
>  > +             _installed_64K=0
>  > +             #echo "$_installed_kernel" | grep 64k
>  > +             if echo "$_installed_kernel" | grep 64k; then
>  > +                     _installed_64K=1
>  > +             fi
>  > +             if echo "$_running_kernel" | grep 64k; then
>  > +                     _running_64K=1
>  > +             fi
>  > +
>  > +             # Otherwise, fall through to common case
>  > +             if [[ $_installed_64K != "$_running_64K" ]]; then
>  > +                     # if the difference is 4K VS 64K
>  > +                     _crashkernel_running=$(kdump_get_arch_recommend_crashkernel
> "kdump" "$_installed_kernel")
>  > +                     _update_kernel_cmdline "$_installed_kernel"
> "$_crashkernel_running" "$_dump_mode_running"
> "$_fadump_val_running"
>  > +                     return
>  > +             fi
>  > +     fi
>  > +
>  >       if [[ $_crashkernel != "$_crashkernel_running" ]]; then
>  >               if _update_kernel_cmdline "$_installed_kernel"
> "$_crashkernel_running" "$_dump_mode_running"
> "$_fadump_val_running"; then
>  >                       echo "kexec-tools has reset $_installed_kernel to use the
> new default crashkernel value $_crashkernel_running"
>
>  Coiby sent the patch "Simplify the management of the kernel parameter
>  crashkernel" to the mailing list where he removes this part of
>  reset_crashkernel_for_installed_kernel. I believe you can drop
>  the changes to kdumpctl.
>  
It is a great relief. I will rebase onto it.


Again, appreciate for your help.

Regards,

Pingfan

[PATCHv2 3/4] kdump-lib: add support for 64K aarch64

[Philipp Rudo]

On Wed, 24 May 2023 11:58:30 +0800
Pingfan Liu <piliu(a)redhat.com&gt; wrote:

>  On Mon, May 22, 2023 at 10:59 PM Philipp Rudo
> <prudo(a)redhat.com&gt; wrote: 
[[...]]

>  
>  > Furthermore, I was sloppy when providing the example. While the line
>  > above is valid bash code it won't work as intended. In particular when
>  > you are using += in bash it evaluates both sides as strings, _not_ as
>  > mathematical expressions. For example
>  >
>  > $ a=1
>  > $ a+=1
>  > $ echo $a
>  >
>  > will give '11', _not_ '2'. The correct syntax for what you want is
>  >  
>  
>  Oops, the C grammar is so deeply rooted in my mind that I am not aware
>  of it either, although I read such a topic just a few days ago. 
Don't worry. Bash is really weird when it comes to arithmetic
expressions...

>  
>  > $ (( _delta += 100 ))
>  >
>  > (Note: the spaces after/before (( / )) are required. The other ones are
>  > optional.)  
>  
>  Sorry but I can not find any format restrictions around ((...)), and I
>  have done some test. 
True, to be honest I cannot remember why I added this. My tests without
spaces work as well...

/me scratching my head

Sorry for the noise
Philipp

>  >  
>  > > +             else
>  > > +                     _delta+=0
>  > > +             fi
>  > > +             _ck_cmdline=$(_crashkernel_add "$_ck_cmdline"
> "$_delta")
>  > >       elif [[ $_arch == "ppc64le" ]]; then
>  > >               if [[ $_dump_mode == "fadump" ]]; then
>  > >                      
> _ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"
>  > > diff --git a/kdumpctl b/kdumpctl
>  > > index 2bec428..cbdc45a 100755
>  > > --- a/kdumpctl
>  > > +++ b/kdumpctl
>  > > @@ -1623,6 +1623,7 @@ reset_crashkernel_for_installed_kernel()
>  > >  {
>  > >       local _installed_kernel _running_kernel _crashkernel
> _crashkernel_running
>  > >       local _dump_mode_running _fadump_val_running
>  > > +     local _arch
>  > >
>  > >       # During package install, only try to reset crashkernel for osbuild
>  > >       # thus to avoid calling grubby when installing os via anaconda
>  > > @@ -1651,6 +1652,29 @@ reset_crashkernel_for_installed_kernel()
>  > >       _dump_mode_running=$(get_dump_mode_by_kernel
> "$_running_kernel")
>  > >       _fadump_val_running=$(get_grub_kernel_boot_parameter "$_kernel"
> fadump)
>  > >
>  > > +     _arch=$(uname -m)
>  > > +     # On aarch64, there are 4K and 64K kernel, which demands different
> crashkernel value
>  > > +     if [[ $_arch == "aarch64" ]]; then
>  > > +             local _installed_64K _running_64K
>  > > +
>  > > +             _installed_64K=0
>  > > +             #echo "$_installed_kernel" | grep 64k
>  > > +             if echo "$_installed_kernel" | grep 64k; then
>  > > +                     _installed_64K=1
>  > > +             fi
>  > > +             if echo "$_running_kernel" | grep 64k; then
>  > > +                     _running_64K=1
>  > > +             fi
>  > > +
>  > > +             # Otherwise, fall through to common case
>  > > +             if [[ $_installed_64K != "$_running_64K" ]]; then
>  > > +                     # if the difference is 4K VS 64K
>  > > +                    
> _crashkernel_running=$(kdump_get_arch_recommend_crashkernel "kdump"
> "$_installed_kernel")
>  > > +                     _update_kernel_cmdline "$_installed_kernel"
> "$_crashkernel_running" "$_dump_mode_running"
> "$_fadump_val_running"
>  > > +                     return
>  > > +             fi
>  > > +     fi
>  > > +
>  > >       if [[ $_crashkernel != "$_crashkernel_running" ]]; then
>  > >               if _update_kernel_cmdline "$_installed_kernel"
> "$_crashkernel_running" "$_dump_mode_running"
> "$_fadump_val_running"; then
>  > >                       echo "kexec-tools has reset $_installed_kernel to
> use the new default crashkernel value $_crashkernel_running"  
>  >
>  > Coiby sent the patch "Simplify the management of the kernel parameter
>  > crashkernel" to the mailing list where he removes this part of
>  > reset_crashkernel_for_installed_kernel. I believe you can drop
>  > the changes to kdumpctl.
>  >  
>  
>  It is a great relief. I will rebase onto it.
>  
>  
>  Again, appreciate for your help.
>  
>  Regards,
>  
>  Pingfan
>

[PATCHv2 3/4] kdump-lib: add support for 64K aarch64

[Jeremy Linton]

Hi,

Quick comment below:


On 5/18/23 00:26, Pingfan Liu wrote:
>  From: Pingfan Liu <piliu at redhat.com>
>  
>  On aarch64, both 4K and 64K kernel can be installed, while they demand
>  different size reserved memory for kdump kernel.
>  
>  'get_conf PAGE_SIZE' can not work if installing a 64K kernel when
>  running a 4K kernel. Hence resorting to the kernel release naming rules.
>  At present, the 64K kernel has the keyword '64k' in its suffix.
>  
>  The base line for 64K is decided based on 4K. The diff 120M is picked up
>  since on a high end machine without smmu enabled, the diff of MemFree is
>  82M.
>  
>  As for the smmu case, a huge difference in the memory consumption lies
>  between 64k and 4k driver. And it should be calculated separatedly.
>  
>  Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
>  ---
>    kdump-lib.sh | 21 ++++++++++++++++++++-
>    kdumpctl     | 24 ++++++++++++++++++++++++
>    2 files changed, 44 insertions(+), 1 deletion(-)
>  
>  diff --git a/kdump-lib.sh b/kdump-lib.sh
>  index f56a011..e1d8da9 100755
>  --- a/kdump-lib.sh
>  +++ b/kdump-lib.sh
>  @@ -966,6 +966,7 @@ _crashkernel_add()
>    
>    # get default crashkernel
>    # $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable
>  +# $2 kernel-release, if not specified, got by _get_kdump_kernel_version
>    kdump_get_arch_recommend_crashkernel()
>    {
>    	local _arch _ck_cmdline _dump_mode
>  @@ -985,8 +986,26 @@ kdump_get_arch_recommend_crashkernel()
>    	if [[ $_arch == "x86_64" ]] || [[ $_arch == "s390x" ]]; then
>    		_ck_cmdline="1G-4G:192M,4G-64G:256M,64G-:512M"
>    	elif [[ $_arch == "aarch64" ]]; then
>  -		# For 4KB page size, the formula is based on x86 plus extra = 64M
>  +		local _running_kernel
>  +		local _delta="0"
>  +
>  +		# Base line for 4K variant kernel. The formula is based on x86 plus extra = 64M
>    		_ck_cmdline="1G-4G:256M,4G-64G:320M,64G-:576M"
>  +		if [[ -z "$2" ]]; then
>  +			_running_kernel=$(_get_kdump_kernel_version)
>  +		else
>  +			_running_kernel=$2
>  +		fi
>  +
>  +		# the naming convention of 64k variant suffixes with +64k, e.g.
> "vmlinuz-5.14.0-312.el9.aarch64+64k"
>  +		if echo "$_running_kernel" | grep 64k; then 
Is this the best way to determine if the current kernel is 64k? Isn't 
`getconf PAGESIZE` less dependent on the kernel naming?


>  +			# Without smmu, the diff of MemFree between 4K and 64K measured
> on a high end aarch64 machine is 82M.
>  +			# Picking up 100M to cover this diff. And finally, we have
> "1G-4G:356M;4G-64G:420M;64G-:676M"
>  +			_delta+=100
>  +		else
>  +			_delta+=0
>  +		fi
>  +		_ck_cmdline=$(_crashkernel_add "$_ck_cmdline" "$_delta")
>    	elif [[ $_arch == "ppc64le" ]]; then
>    		if [[ $_dump_mode == "fadump" ]]; then
>   
> 			_ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"
>  diff --git a/kdumpctl b/kdumpctl
>  index 2bec428..cbdc45a 100755
>  --- a/kdumpctl
>  +++ b/kdumpctl
>  @@ -1623,6 +1623,7 @@ reset_crashkernel_for_installed_kernel()
>    {
>    	local _installed_kernel _running_kernel _crashkernel _crashkernel_running
>    	local _dump_mode_running _fadump_val_running
>  +	local _arch
>    
>    	# During package install, only try to reset crashkernel for osbuild
>    	# thus to avoid calling grubby when installing os via anaconda
>  @@ -1651,6 +1652,29 @@ reset_crashkernel_for_installed_kernel()
>    	_dump_mode_running=$(get_dump_mode_by_kernel "$_running_kernel")
>    	_fadump_val_running=$(get_grub_kernel_boot_parameter "$_kernel" fadump)
>    
>  +	_arch=$(uname -m)
>  +	# On aarch64, there are 4K and 64K kernel, which demands different crashkernel value
>  +	if [[ $_arch == "aarch64" ]]; then
>  +		local _installed_64K _running_64K
>  +
>  +		_installed_64K=0
>  +		#echo "$_installed_kernel" | grep 64k
>  +		if echo "$_installed_kernel" | grep 64k; then
>  +			_installed_64K=1
>  +		fi
>  +		if echo "$_running_kernel" | grep 64k; then
>  +			_running_64K=1
>  +		fi
>  +
>  +		# Otherwise, fall through to common case
>  +		if [[ $_installed_64K != "$_running_64K" ]]; then
>  +			# if the difference is 4K VS 64K
>  +			_crashkernel_running=$(kdump_get_arch_recommend_crashkernel "kdump"
> "$_installed_kernel")
>  +			_update_kernel_cmdline "$_installed_kernel"
> "$_crashkernel_running" "$_dump_mode_running"
> "$_fadump_val_running"
>  +			return
>  +		fi
>  +	fi
>  +
>    	if [[ $_crashkernel != "$_crashkernel_running" ]]; then
>    		if _update_kernel_cmdline "$_installed_kernel"
> "$_crashkernel_running" "$_dump_mode_running"
> "$_fadump_val_running"; then
>    			echo "kexec-tools has reset $_installed_kernel to use the new default
> crashkernel value $_crashkernel_running"

[PATCHv2 3/4] kdump-lib: add support for 64K aarch64

[Philipp Rudo]

Hi Jeremy,

On Tue, 23 May 2023 09:01:09 -0500
Jeremy Linton <jeremy.linton(a)arm.com&gt; wrote:

>  Hi,
>  
>  Quick comment below:
>  
>  
>  On 5/18/23 00:26, Pingfan Liu wrote:
>  > From: Pingfan Liu <piliu at redhat.com>
>  > 
>  > On aarch64, both 4K and 64K kernel can be installed, while they demand
>  > different size reserved memory for kdump kernel.
>  > 
>  > 'get_conf PAGE_SIZE' can not work if installing a 64K kernel when
>  > running a 4K kernel. Hence resorting to the kernel release naming rules.
>  > At present, the 64K kernel has the keyword '64k' in its suffix.
>  > 
>  > The base line for 64K is decided based on 4K. The diff 120M is picked up
>  > since on a high end machine without smmu enabled, the diff of MemFree is
>  > 82M.
>  > 
>  > As for the smmu case, a huge difference in the memory consumption lies
>  > between 64k and 4k driver. And it should be calculated separatedly.
>  > 
>  > Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
>  > ---
>  >   kdump-lib.sh | 21 ++++++++++++++++++++-
>  >   kdumpctl     | 24 ++++++++++++++++++++++++
>  >   2 files changed, 44 insertions(+), 1 deletion(-)
>  > 
>  > diff --git a/kdump-lib.sh b/kdump-lib.sh
>  > index f56a011..e1d8da9 100755
>  > --- a/kdump-lib.sh
>  > +++ b/kdump-lib.sh
>  > @@ -966,6 +966,7 @@ _crashkernel_add()
>  >   
>  >   # get default crashkernel
>  >   # $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable
>  > +# $2 kernel-release, if not specified, got by _get_kdump_kernel_version
>  >   kdump_get_arch_recommend_crashkernel()
>  >   {
>  >   	local _arch _ck_cmdline _dump_mode
>  > @@ -985,8 +986,26 @@ kdump_get_arch_recommend_crashkernel()
>  >   	if [[ $_arch == "x86_64" ]] || [[ $_arch == "s390x" ]]; then
>  >   		_ck_cmdline="1G-4G:192M,4G-64G:256M,64G-:512M"
>  >   	elif [[ $_arch == "aarch64" ]]; then
>  > -		# For 4KB page size, the formula is based on x86 plus extra = 64M
>  > +		local _running_kernel
>  > +		local _delta="0"
>  > +
>  > +		# Base line for 4K variant kernel. The formula is based on x86 plus extra = 64M
>  >   		_ck_cmdline="1G-4G:256M,4G-64G:320M,64G-:576M"
>  > +		if [[ -z "$2" ]]; then
>  > +			_running_kernel=$(_get_kdump_kernel_version)
>  > +		else
>  > +			_running_kernel=$2
>  > +		fi
>  > +
>  > +		# the naming convention of 64k variant suffixes with +64k, e.g.
> "vmlinuz-5.14.0-312.el9.aarch64+64k"
>  > +		if echo "$_running_kernel" | grep 64k; then  
>  
>  Is this the best way to determine if the current kernel is 64k? Isn't 
>  `getconf PAGESIZE` less dependent on the kernel naming? 
Pingfan described the reason in the patch description. The problem is
that the function is called when installing a new kernel. With the
default kernel on still being 4k this will lead to situations where you
are running a 4k kernel but installing a 64k one. In that case getconf
PAGE_SIZE will fail.

But true, relying on the naming convention isn't great. Let us know when
you have a better idea to solve the problem. We'd be happy to adopt it.

Thanks
Philipp

>  > +			# Without smmu, the diff of MemFree between 4K and 64K
> measured on a high end aarch64 machine is 82M.
>  > +			# Picking up 100M to cover this diff. And finally, we have
> "1G-4G:356M;4G-64G:420M;64G-:676M"
>  > +			_delta+=100
>  > +		else
>  > +			_delta+=0
>  > +		fi
>  > +		_ck_cmdline=$(_crashkernel_add "$_ck_cmdline" "$_delta")
>  >   	elif [[ $_arch == "ppc64le" ]]; then
>  >   		if [[ $_dump_mode == "fadump" ]]; then
>  >  
> 			_ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"
>  > diff --git a/kdumpctl b/kdumpctl
>  > index 2bec428..cbdc45a 100755
>  > --- a/kdumpctl
>  > +++ b/kdumpctl
>  > @@ -1623,6 +1623,7 @@ reset_crashkernel_for_installed_kernel()
>  >   {
>  >   	local _installed_kernel _running_kernel _crashkernel _crashkernel_running
>  >   	local _dump_mode_running _fadump_val_running
>  > +	local _arch
>  >   
>  >   	# During package install, only try to reset crashkernel for osbuild
>  >   	# thus to avoid calling grubby when installing os via anaconda
>  > @@ -1651,6 +1652,29 @@ reset_crashkernel_for_installed_kernel()
>  >   	_dump_mode_running=$(get_dump_mode_by_kernel "$_running_kernel")
>  >   	_fadump_val_running=$(get_grub_kernel_boot_parameter "$_kernel"
> fadump)
>  >   
>  > +	_arch=$(uname -m)
>  > +	# On aarch64, there are 4K and 64K kernel, which demands different crashkernel
> value
>  > +	if [[ $_arch == "aarch64" ]]; then
>  > +		local _installed_64K _running_64K
>  > +
>  > +		_installed_64K=0
>  > +		#echo "$_installed_kernel" | grep 64k
>  > +		if echo "$_installed_kernel" | grep 64k; then
>  > +			_installed_64K=1
>  > +		fi
>  > +		if echo "$_running_kernel" | grep 64k; then
>  > +			_running_64K=1
>  > +		fi
>  > +
>  > +		# Otherwise, fall through to common case
>  > +		if [[ $_installed_64K != "$_running_64K" ]]; then
>  > +			# if the difference is 4K VS 64K
>  > +			_crashkernel_running=$(kdump_get_arch_recommend_crashkernel "kdump"
> "$_installed_kernel")
>  > +			_update_kernel_cmdline "$_installed_kernel"
> "$_crashkernel_running" "$_dump_mode_running"
> "$_fadump_val_running"
>  > +			return
>  > +		fi
>  > +	fi
>  > +
>  >   	if [[ $_crashkernel != "$_crashkernel_running" ]]; then
>  >   		if _update_kernel_cmdline "$_installed_kernel"
> "$_crashkernel_running" "$_dump_mode_running"
> "$_fadump_val_running"; then
>  >   			echo "kexec-tools has reset $_installed_kernel to use the new default
> crashkernel value $_crashkernel_running"  
>

[PATCHv2 4/4] kdump-lib: Evaluate the memory consumption by smmu and mlx5 separately

[Pingfan Liu]

On 4k and 64k kernels, the typical consumption values for SMMU are 36MB
and 384MB, respectively. Hence for 64k kernel, the consumption by smmu
should be taken into account carefully.

To do it by adding the extra 384MB value if installing a 64k kernel.
The upper limit value 384MB is calculated according to the formula in
the kernel smmu driver.

As for mlx5 network cards, it is measured by a pratical test, 200M for
64k variant, 150M for 4k variant

Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
---
 kdump-lib.sh | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/kdump-lib.sh b/kdump-lib.sh
index e1d8da9..8826841 100755
--- a/kdump-lib.sh
+++ b/kdump-lib.sh
@@ -906,6 +906,22 @@ get_recommend_size()
 	echo "0M"
 }
 
+has_mlx5()
+{
+	if [ -d /sys/bus/pci/drivers/mlx5_core ]; then
+		return 0
+	fi
+	return 1
+}
+
+has_aarch64_smmu()
+{
+	if ls /sys/devices/platform/arm-smmu-* 1> /dev/null 2>&1; then
+		return 0
+	fi
+	return 1
+}
+
 # $1 crashkernel=""
 # $2 delta in unit of MB
 _crashkernel_add()
@@ -1002,8 +1018,14 @@ kdump_get_arch_recommend_crashkernel()
 			# Without smmu, the diff of MemFree between 4K and 64K measured on a high end aarch64
machine is 82M.
 			# Picking up 100M to cover this diff. And finally, we have
"1G-4G:356M;4G-64G:420M;64G-:676M"
 			_delta+=100
+			# On a 64K system, the extra 384MB is calculated by: cmdq_num * 16 bytes + evtq_num *
32B + priq_num * 16B
+			# While on a 4K system, it is negligible
+			has_aarch64_smmu && _delta+=384
+			#64k kernel, mlx5 consumes extra 188M memory, and choose 200M
+			has_mlx5 && _delta+=200
 		else
-			_delta+=0
+			#4k kernel, mlx5 consumes extra 124M memory, and choose 150M
+			has_mlx5 && _delta+=150
 		fi
 		_ck_cmdline=$(_crashkernel_add "$_ck_cmdline" "$_delta")
 	elif [[ $_arch == "ppc64le" ]]; then
-- 
2.31.1

[PATCHv2 4/4] kdump-lib: Evaluate the memory consumption by smmu and mlx5 separately

[Philipp Rudo]

Hi Pingfan,

On Thu, 18 May 2023 13:26:43 +0800
Pingfan Liu <piliu(a)redhat.com&gt; wrote:

>  On 4k and 64k kernels, the typical consumption values for SMMU are
> 36MB
>  and 384MB, respectively. Hence for 64k kernel, the consumption by smmu
>  should be taken into account carefully.
>  
>  To do it by adding the extra 384MB value if installing a 64k kernel.
>  The upper limit value 384MB is calculated according to the formula in
>  the kernel smmu driver.
>  
>  As for mlx5 network cards, it is measured by a pratical test, 200M for
>  64k variant, 150M for 4k variant
>  
>  Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt;
>  ---
>   kdump-lib.sh | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
>  
>  diff --git a/kdump-lib.sh b/kdump-lib.sh
>  index e1d8da9..8826841 100755
>  --- a/kdump-lib.sh
>  +++ b/kdump-lib.sh
>  @@ -906,6 +906,22 @@ get_recommend_size()
>   	echo "0M"
>   }
>   
>  +has_mlx5()
>  +{
>  +	if [ -d /sys/bus/pci/drivers/mlx5_core ]; then
>  +		return 0
>  +	fi
>  +	return 1
>  +}
>  +
>  +has_aarch64_smmu()
>  +{
>  +	if ls /sys/devices/platform/arm-smmu-* 1> /dev/null 2>&1; then
>  +		return 0
>  +	fi
>  +	return 1
>  +}
>  +
>   # $1 crashkernel=""
>   # $2 delta in unit of MB
>   _crashkernel_add()
>  @@ -1002,8 +1018,14 @@ kdump_get_arch_recommend_crashkernel()
>   			# Without smmu, the diff of MemFree between 4K and 64K measured on a high end aarch64
> machine is 82M.
>   			# Picking up 100M to cover this diff. And finally, we have
> "1G-4G:356M;4G-64G:420M;64G-:676M"
>   			_delta+=100
>  +			# On a 64K system, the extra 384MB is calculated by: cmdq_num * 16 bytes + evtq_num *
> 32B + priq_num * 16B
>  +			# While on a 4K system, it is negligible
>  +			has_aarch64_smmu && _delta+=384
>  +			#64k kernel, mlx5 consumes extra 188M memory, and choose 200M
>  +			has_mlx5 && _delta+=200 
same like the last patch, i.e. you need to add (( ... )).
Otherwise the patch looks fine.

Thanks
Philipp

>   		else
>  -			_delta+=0
>  +			#4k kernel, mlx5 consumes extra 124M memory, and choose 150M
>  +			has_mlx5 && _delta+=150
>   		fi
>   		_ck_cmdline=$(_crashkernel_add "$_ck_cmdline" "$_delta")
>   	elif [[ $_arch == "ppc64le" ]]; then