From: Baoquan He <bhe@redhat.com>
To: dyoung@redhat.com
Cc: corbet@lwn.net, kexec@lists.infradead.org,
linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
yinghai@kernel.org, vgoyal@redhat.com
Subject: Re: [PATCH 3/3] kdump: round up the total memory size to 128M for crashkernel reservation
Date: Tue, 24 Oct 2017 13:57:22 +0800 [thread overview]
Message-ID: <20171024055722.GG27757@x1> (raw)
In-Reply-To: <20171024053901.813846011@redhat.com>
Hi Dave,
On 10/24/17 at 01:31pm, Dave Young wrote:
> The total memory size we get in kernel is usually slightly less than 2G with a
> 2G memory module machine. The main reason is bios/firmware reserve some area
> it will not export all memory as usable to Linux.
>
> 2G memory X86 kvm guest test result of the total_mem value:
> UEFI boot with ovmf: 0x7ef10000
> Legacy boot kvm guest: 0x7ff7cc00
>
> An option is to use dmi/smbios to get physical memory size, but it's not
> reliable as well. According to Prarit hardware vendors sometimes screw this up.
> Thus we choose to round up total size to 128M to workaround this problem.
> This is a best effort workaround, will improve it when we have better way
> in the future.
Thanks for this posting. While I don't get the point of this patch. So
firmware take piece of memory, then why we need to count it into the
total memory which we want to calculate a crashkernel memory based on.
Not counting that, is there anyting incorrect?
Thanks
Baoquan
>
> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
> kernel/crash_core.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> --- linux.orig/kernel/crash_core.c
> +++ linux/kernel/crash_core.c
> @@ -42,6 +42,15 @@ static int __init parse_crashkernel_mem(
> {
> char *cur = cmdline, *tmp;
> bool infinite_end = false;
> + unsigned long long total_mem = system_ram;
> +
> + /*
> + * Firmware usually reserves some memory regions for it's own use.
> + * so we get less than actual system memory size.
> + * We workaround this by round up the total size to 128M which is
> + * enough for most test cases.
> + */
> + total_mem = roundup(total_mem, 0x8000000);
>
> /* for each entry of the comma-separated list */
> do {
> @@ -86,13 +95,13 @@ static int __init parse_crashkernel_mem(
> return -EINVAL;
> }
> cur = tmp;
> - if (size >= system_ram) {
> + if (size >= total_mem) {
> pr_warn("crashkernel: invalid size\n");
> return -EINVAL;
> }
>
> /* match ? */
> - if (system_ram >= start && system_ram < end) {
> + if (total_mem >= start && total_mem < end) {
> *crash_size = size;
> if (end == ULLONG_MAX)
> infinite_end = true;
> @@ -126,9 +135,9 @@ static int __init parse_crashkernel_mem(
> pr_warn("Memory reservation scale order expected after '^'\n");
> return -EINVAL;
> }
> - size = (system_ram - *crash_size) >> shift;
> + size = (total_mem - *crash_size) >> shift;
> size = *crash_size + roundup(size, 1ULL << 20);
> - if (size < system_ram)
> + if (size < total_mem)
> *crash_size = size;
> cur = tmp;
> } else
>
>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
WARNING: multiple messages have this Message-ID (diff)
From: Baoquan He <bhe@redhat.com>
To: dyoung@redhat.com
Cc: kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
akpm@linux-foundation.org, vgoyal@redhat.com, yinghai@kernel.org,
corbet@lwn.net
Subject: Re: [PATCH 3/3] kdump: round up the total memory size to 128M for crashkernel reservation
Date: Tue, 24 Oct 2017 13:57:22 +0800 [thread overview]
Message-ID: <20171024055722.GG27757@x1> (raw)
In-Reply-To: <20171024053901.813846011@redhat.com>
Hi Dave,
On 10/24/17 at 01:31pm, Dave Young wrote:
> The total memory size we get in kernel is usually slightly less than 2G with a
> 2G memory module machine. The main reason is bios/firmware reserve some area
> it will not export all memory as usable to Linux.
>
> 2G memory X86 kvm guest test result of the total_mem value:
> UEFI boot with ovmf: 0x7ef10000
> Legacy boot kvm guest: 0x7ff7cc00
>
> An option is to use dmi/smbios to get physical memory size, but it's not
> reliable as well. According to Prarit hardware vendors sometimes screw this up.
> Thus we choose to round up total size to 128M to workaround this problem.
> This is a best effort workaround, will improve it when we have better way
> in the future.
Thanks for this posting. While I don't get the point of this patch. So
firmware take piece of memory, then why we need to count it into the
total memory which we want to calculate a crashkernel memory based on.
Not counting that, is there anyting incorrect?
Thanks
Baoquan
>
> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
> kernel/crash_core.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> --- linux.orig/kernel/crash_core.c
> +++ linux/kernel/crash_core.c
> @@ -42,6 +42,15 @@ static int __init parse_crashkernel_mem(
> {
> char *cur = cmdline, *tmp;
> bool infinite_end = false;
> + unsigned long long total_mem = system_ram;
> +
> + /*
> + * Firmware usually reserves some memory regions for it's own use.
> + * so we get less than actual system memory size.
> + * We workaround this by round up the total size to 128M which is
> + * enough for most test cases.
> + */
> + total_mem = roundup(total_mem, 0x8000000);
>
> /* for each entry of the comma-separated list */
> do {
> @@ -86,13 +95,13 @@ static int __init parse_crashkernel_mem(
> return -EINVAL;
> }
> cur = tmp;
> - if (size >= system_ram) {
> + if (size >= total_mem) {
> pr_warn("crashkernel: invalid size\n");
> return -EINVAL;
> }
>
> /* match ? */
> - if (system_ram >= start && system_ram < end) {
> + if (total_mem >= start && total_mem < end) {
> *crash_size = size;
> if (end == ULLONG_MAX)
> infinite_end = true;
> @@ -126,9 +135,9 @@ static int __init parse_crashkernel_mem(
> pr_warn("Memory reservation scale order expected after '^'\n");
> return -EINVAL;
> }
> - size = (system_ram - *crash_size) >> shift;
> + size = (total_mem - *crash_size) >> shift;
> size = *crash_size + roundup(size, 1ULL << 20);
> - if (size < system_ram)
> + if (size < total_mem)
> *crash_size = size;
> cur = tmp;
> } else
>
>
next prev parent reply other threads:[~2017-10-24 5:57 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-24 5:31 [PATCH 3/3] kdump: round up the total memory size to 128M for crashkernel reservation dyoung
2017-10-24 5:31 ` dyoung
2017-10-24 5:57 ` Baoquan He [this message]
2017-10-24 5:57 ` Baoquan He
2017-10-24 6:09 ` Dave Young
2017-10-24 6:09 ` Dave Young
2017-11-01 7:21 ` Baoquan He
2017-11-01 7:21 ` Baoquan He
2017-11-03 2:46 ` [PATCH 3/3 update] " Dave Young
2017-11-03 2:46 ` Dave Young
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171024055722.GG27757@x1 \
--to=bhe@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=corbet@lwn.net \
--cc=dyoung@redhat.com \
--cc=kexec@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vgoyal@redhat.com \
--cc=yinghai@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.