Hi Jeremy,
On Tue, 23 May 2023 09:01:09 -0500
Jeremy Linton <jeremy.linton(a)arm.com> 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>
> ---
> 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"