[PATCH] Exclude the extra memory requirement when comparing the estimated value with the baseline value
The current estimated crashkernel is too small and leads to OOM for the LUKS case forboth aarch64 4k and 64k kernels. Since the baseline value doesn't take special cases like LUKS into consideration, the extra memory requirement should be included when comparing the estimated value with the baseline value.
Signed-off-by: Coiby Xu coxu@redhat.com --- kdumpctl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kdumpctl b/kdumpctl index 58c18d21..12401042 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1314,13 +1314,15 @@ do_estimate() echo -e "Encrypted kdump target requires extra memory, assuming using the keyslot with maximum memory requirement\n" fi
- estimated_size=$((kernel_size + mod_size + initrd_size + runtime_size + crypt_size)) + estimated_size=$((kernel_size + mod_size + initrd_size + runtime_size)) if [[ $baseline_size -gt $estimated_size ]]; then recommended_size=$baseline_size else recommended_size=$estimated_size fi
+ recommended_size=$((recommended_size + crypt_size)) + echo "Reserved crashkernel: $((reserved_size / size_mb))M" echo "Recommended crashkernel: $((recommended_size / size_mb))M" echo
Hi Coiby,
On Thu, 15 Jun 2023 13:06:21 +0800 Coiby Xu coxu@redhat.com wrote:
The current estimated crashkernel is too small and leads to OOM for the LUKS case forboth aarch64 4k and 64k kernels. Since the baseline value
s/forboth/for both/
doesn't take special cases like LUKS into consideration, the extra memory requirement should be included when comparing the estimated value
s/should/shouldn't/ ?
with the baseline value.
The code change looks good to me but IMHO the description needs some improvement. The way I understand it the problem is that the crypt_size is rather large. Which means that for encrypted devices the estimated_size will always be chosen even when the "estimated baseline" (i.e. estimated_size - crypt_size) is smaller than the baseline_size. While the crypt_size is rather accurate but the "estimated baseline" isn't this can cause OOM when the estimated_size is used. Is that correct?
Thanks Philipp
Signed-off-by: Coiby Xu coxu@redhat.com
kdumpctl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kdumpctl b/kdumpctl index 58c18d21..12401042 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1314,13 +1314,15 @@ do_estimate() echo -e "Encrypted kdump target requires extra memory, assuming using the keyslot with maximum memory requirement\n" fi
- estimated_size=$((kernel_size + mod_size + initrd_size + runtime_size + crypt_size))
estimated_size=$((kernel_size + mod_size + initrd_size + runtime_size)) if [[ $baseline_size -gt $estimated_size ]]; then recommended_size=$baseline_size else recommended_size=$estimated_size fi
recommended_size=$((recommended_size + crypt_size))
echo "Reserved crashkernel: $((reserved_size / size_mb))M" echo "Recommended crashkernel: $((recommended_size / size_mb))M" echo
The current estimated crashkernel is too small and leads to OOM for the LUKS case for both aarch64 4k and 64k kernels. For example, "kdumpctl estimate" gave the following output on a system, # kdumpctl estimate Encrypted kdump target requires extra memory, assuming using the keyslot with maximum memory requirement
Recommended crashkernel: 1217M
Kernel image size: 47M Kernel modules size: 3M Initramfs size: 28M Runtime reservation: 64M LUKS required size: 1074M Large modules: xfs: 2002944
If we subtract the LUKS required size 1074M from the estimated size 1217M, we have only 143M. The estimation about LUKS required size is accurate so it means "kdumpctl estimate" significantly underestimate the baseline value. "kdumpctl estimate" will compare the baseline value with the default crashkernel value and choose the bigger one. Since the default value doesn't take special cases like LUKS into consideration, the extra memory requirement should be excluded when comparing the baseline value with the default value.
Signed-off-by: Coiby Xu coxu@redhat.com --- v2 - improve commit msg [Philipp] --- kdumpctl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kdumpctl b/kdumpctl index 58c18d21..12401042 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1314,13 +1314,15 @@ do_estimate() echo -e "Encrypted kdump target requires extra memory, assuming using the keyslot with maximum memory requirement\n" fi
- estimated_size=$((kernel_size + mod_size + initrd_size + runtime_size + crypt_size)) + estimated_size=$((kernel_size + mod_size + initrd_size + runtime_size)) if [[ $baseline_size -gt $estimated_size ]]; then recommended_size=$baseline_size else recommended_size=$estimated_size fi
+ recommended_size=$((recommended_size + crypt_size)) + echo "Reserved crashkernel: $((reserved_size / size_mb))M" echo "Recommended crashkernel: $((recommended_size / size_mb))M" echo
Hi Coiby,
much better. Thanks!
Personally would also mention our recommended default value as that would emphasize on how significantly the size gets underestimated. But that is more me being pedantic. If you like you can simply ignore my comment.
Thanks Reviewed-by: Philipp Rudo prudo@redhat.com
On Wed, 28 Jun 2023 11:12:21 +0800 Coiby Xu coxu@redhat.com wrote:
The current estimated crashkernel is too small and leads to OOM for the LUKS case for both aarch64 4k and 64k kernels. For example, "kdumpctl estimate" gave the following output on a system, # kdumpctl estimate Encrypted kdump target requires extra memory, assuming using the keyslot with maximum memory requirement
Recommended crashkernel: 1217M Kernel image size: 47M Kernel modules size: 3M Initramfs size: 28M Runtime reservation: 64M LUKS required size: 1074M Large modules: xfs: 2002944If we subtract the LUKS required size 1074M from the estimated size 1217M, we have only 143M. The estimation about LUKS required size is accurate so it means "kdumpctl estimate" significantly underestimate the baseline value. "kdumpctl estimate" will compare the baseline value with the default crashkernel value and choose the bigger one. Since the default value doesn't take special cases like LUKS into consideration, the extra memory requirement should be excluded when comparing the baseline value with the default value.
Signed-off-by: Coiby Xu coxu@redhat.com
v2
- improve commit msg [Philipp]
kdumpctl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kdumpctl b/kdumpctl index 58c18d21..12401042 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1314,13 +1314,15 @@ do_estimate() echo -e "Encrypted kdump target requires extra memory, assuming using the keyslot with maximum memory requirement\n" fi
- estimated_size=$((kernel_size + mod_size + initrd_size + runtime_size + crypt_size))
estimated_size=$((kernel_size + mod_size + initrd_size + runtime_size)) if [[ $baseline_size -gt $estimated_size ]]; then recommended_size=$baseline_size else recommended_size=$estimated_size fi
recommended_size=$((recommended_size + crypt_size))
echo "Reserved crashkernel: $((reserved_size / size_mb))M" echo "Recommended crashkernel: $((recommended_size / size_mb))M" echo
On Mon, Jun 19, 2023 at 06:06:00PM +0200, Philipp Rudo wrote:
Hi Coiby,
On Thu, 15 Jun 2023 13:06:21 +0800 Coiby Xu coxu@redhat.com wrote:
The current estimated crashkernel is too small and leads to OOM for the LUKS case forboth aarch64 4k and 64k kernels. Since the baseline value
s/forboth/for both/
doesn't take special cases like LUKS into consideration, the extra memory requirement should be included when comparing the estimated value
s/should/shouldn't/ ?
Thanks for catching the two typos!
with the baseline value.
k
The code change looks good to me but IMHO the description needs some improvement. The way I understand it the problem is that the crypt_size is rather large. Which means that for encrypted devices the estimated_size will always be chosen even when the "estimated baseline" (i.e. estimated_size - crypt_size) is smaller than the baseline_size. While the crypt_size is rather accurate but the "estimated baseline" isn't this can cause OOM when the estimated_size is used. Is that correct?
Yes, your understanding is correct. I've sent a new version with the improved msg. Thanks!
Thanks Philipp
Signed-off-by: Coiby Xu coxu@redhat.com
kdumpctl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kdumpctl b/kdumpctl index 58c18d21..12401042 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1314,13 +1314,15 @@ do_estimate() echo -e "Encrypted kdump target requires extra memory, assuming using the keyslot with maximum memory requirement\n" fi
- estimated_size=$((kernel_size + mod_size + initrd_size + runtime_size + crypt_size))
estimated_size=$((kernel_size + mod_size + initrd_size + runtime_size)) if [[ $baseline_size -gt $estimated_size ]]; then recommended_size=$baseline_size else recommended_size=$estimated_size fi
recommended_size=$((recommended_size + crypt_size))
echo "Reserved crashkernel: $((reserved_size / size_mb))M" echo "Recommended crashkernel: $((recommended_size / size_mb))M" echo
-
Coiby Xu -
Philipp Rudo