Before:
BIOS-provided physical RAM map:
BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] usable
BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] reserved
BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] reserved
After:
BIOS-provided physical RAM map:
BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] 639 KB kernel usable RAM
BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] 1 KB reserved
BIOS-e820: [gap 0x00000000000a0000-0x00000000000effff] 320 KB ...
BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] 64 KB reserved
BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] 1.9 GB kernel usable RAM
BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] 144 KB reserved
BIOS-e820: [gap 0x0000000080000000-0x00000000afffffff] 768 MB ...
BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] 256 MB reserved
BIOS-e820: [gap 0x00000000c0000000-0x00000000fed1bfff] 1005.1 MB ...
BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] 16 KB reserved
BIOS-e820: [gap 0x00000000fed20000-0x00000000feffbfff] 2.8 MB ...
BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] 16 KB reserved
BIOS-e820: [gap 0x00000000ff000000-0x00000000fffbffff] 15.7 MB ...
BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] 256 KB reserved
BIOS-e820: [gap 0x0000000100000000-0x000000fcffffffff] 1008 GB ...
BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] 12 GB reserved
Note how a 1-digit precision field is printed out if a range is
fractional in its largest-enclosing natural size unit.
So the "256 MB" and "12 GB" fields above denote exactly 256 MB and
12 GB regions, while "1.9 GB" signals the region's fractional nature
and it being just below 2GB.
Printing E820 maps with such details visualizes 'weird' ranges
at a glance, and gives users a better understanding of how
large the various ranges are, without having to perform hexadecimal
subtraction in their minds.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
(cherry picked from commit d1ac6b8718575a7ea2f0a1ff347835a8879df673)
---
arch/x86/kernel/e820.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 10bd10bd5672..8ee89962fcbf 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -199,6 +199,41 @@ static void __init e820_print_type(enum e820_type type)
}
}
+/*
+ * Print out the size of a E820 region, in human-readable
+ * fashion, going from KB, MB, GB to TB units.
+ *
+ * Print out fractional sizes with a single digit of precision.
+ */
+static void e820_print_size(u64 size)
+{
+ if (size < SZ_1M) {
+ if (size & (SZ_1K-1))
+ pr_cont(" %4llu.%01llu KB", size/SZ_1K, 10*(size & (SZ_1K-1))/SZ_1K);
+ else
+ pr_cont(" %4llu KB", size/SZ_1K);
+ return;
+ }
+ if (size < SZ_1G) {
+ if (size & (SZ_1M-1))
+ pr_cont(" %4llu.%01llu MB", size/SZ_1M, 10*(size & (SZ_1M-1))/SZ_1M);
+ else
+ pr_cont(" %4llu MB", size/SZ_1M);
+ return;
+ }
+ if (size < SZ_1T) {
+ if (size & (SZ_1G-1))
+ pr_cont(" %4llu.%01llu GB", size/SZ_1G, 10*(size & (SZ_1G-1))/SZ_1G);
+ else
+ pr_cont(" %4llu GB", size/SZ_1G);
+ return;
+ }
+ if (size & (SZ_1T-1))
+ pr_cont(" %4llu.%01llu TB", size/SZ_1T, 10*(size & (SZ_1T-1))/SZ_1T);
+ else
+ pr_cont(" %4llu TB", size/SZ_1T);
+}
+
static void __init e820__print_table(const char *who)
{
u64 range_end_prev = 0;
@@ -215,14 +250,22 @@ static void __init e820__print_table(const char *who)
if (range_start < range_end_prev)
pr_info("BUG: out of order E820 entry!\n");
+ /* Print gaps, if any: */
if (range_start > range_end_prev) {
- pr_info("%s: [gap %#018Lx-%#018Lx]\n",
+ u64 gap_size = range_start - range_end_prev;
+
+ pr_info("%s: [gap %#018Lx-%#018Lx]",
who,
range_end_prev,
range_start-1);
+
+ e820_print_size(gap_size);
+ pr_cont(" ...\n");
}
- pr_info("%s: [mem %#018Lx-%#018Lx] ", who, range_start, range_end-1);
+ /* Print allocated ranges: */
+ pr_info("%s: [mem %#018Lx-%#018Lx]", who, range_start, range_end-1);
+ e820_print_size(entry->size);
e820_print_type(entry->type);
pr_cont("\n");
--
2.45.2On Mon, Apr 21, 2025 at 08:51:47PM +0200, Ingo Molnar wrote:
> Before:
>
> BIOS-provided physical RAM map:
> BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
> BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
> BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
> BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] usable
> BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] reserved
> BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
> BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
> BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
> BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
> BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] reserved
>
> After:
>
> BIOS-provided physical RAM map:
> BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] 639 KB kernel usable RAM
> BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] 1 KB reserved
> BIOS-e820: [gap 0x00000000000a0000-0x00000000000effff] 320 KB ...
> BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] 64 KB reserved
> BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] 1.9 GB kernel usable RAM
> BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] 144 KB reserved
> BIOS-e820: [gap 0x0000000080000000-0x00000000afffffff] 768 MB ...
> BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] 256 MB reserved
> BIOS-e820: [gap 0x00000000c0000000-0x00000000fed1bfff] 1005.1 MB ...
> BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] 16 KB reserved
> BIOS-e820: [gap 0x00000000fed20000-0x00000000feffbfff] 2.8 MB ...
> BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] 16 KB reserved
> BIOS-e820: [gap 0x00000000ff000000-0x00000000fffbffff] 15.7 MB ...
> BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] 256 KB reserved
> BIOS-e820: [gap 0x0000000100000000-0x000000fcffffffff] 1008 GB ...
> BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] 12 GB reserved
>
> Note how a 1-digit precision field is printed out if a range is
> fractional in its largest-enclosing natural size unit.
>
> So the "256 MB" and "12 GB" fields above denote exactly 256 MB and
> 12 GB regions, while "1.9 GB" signals the region's fractional nature
> and it being just below 2GB.
>
> Printing E820 maps with such details visualizes 'weird' ranges
> at a glance, and gives users a better understanding of how
> large the various ranges are, without having to perform hexadecimal
> subtraction in their minds.
>
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: Arnd Bergmann <arnd@kernel.org>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
> (cherry picked from commit d1ac6b8718575a7ea2f0a1ff347835a8879df673)
> ---
> arch/x86/kernel/e820.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 10bd10bd5672..8ee89962fcbf 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -199,6 +199,41 @@ static void __init e820_print_type(enum e820_type type)
> }
> }
>
> +/*
> + * Print out the size of a E820 region, in human-readable
> + * fashion, going from KB, MB, GB to TB units.
> + *
> + * Print out fractional sizes with a single digit of precision.
> + */
> +static void e820_print_size(u64 size)
> +{
> + if (size < SZ_1M) {
> + if (size & (SZ_1K-1))
> + pr_cont(" %4llu.%01llu KB", size/SZ_1K, 10*(size & (SZ_1K-1))/SZ_1K);
> + else
> + pr_cont(" %4llu KB", size/SZ_1K);
> + return;
> + }
I'd make this a helper, e.g
static void __e820_print_size(u64 size, u64 unit, const char *unit_name)
{
if (size & (unit - 1)) {
u64 fraction = 10 * (size & (unit - 1))/unit;
pr_cont(" %4llu.%01llu %s", size/unit, fraction, unit_name);
} else {
pr_cont(" %4llu %s", size/unit, unit_name);
}
}
> + if (size < SZ_1G) {
> + if (size & (SZ_1M-1))
> + pr_cont(" %4llu.%01llu MB", size/SZ_1M, 10*(size & (SZ_1M-1))/SZ_1M);
> + else
> + pr_cont(" %4llu MB", size/SZ_1M);
> + return;
> + }
> + if (size < SZ_1T) {
> + if (size & (SZ_1G-1))
> + pr_cont(" %4llu.%01llu GB", size/SZ_1G, 10*(size & (SZ_1G-1))/SZ_1G);
> + else
> + pr_cont(" %4llu GB", size/SZ_1G);
> + return;
> + }
> + if (size & (SZ_1T-1))
> + pr_cont(" %4llu.%01llu TB", size/SZ_1T, 10*(size & (SZ_1T-1))/SZ_1T);
> + else
> + pr_cont(" %4llu TB", size/SZ_1T);
> +}
> +
> static void __init e820__print_table(const char *who)
> {
> u64 range_end_prev = 0;
> @@ -215,14 +250,22 @@ static void __init e820__print_table(const char *who)
> if (range_start < range_end_prev)
> pr_info("BUG: out of order E820 entry!\n");
>
> + /* Print gaps, if any: */
> if (range_start > range_end_prev) {
> - pr_info("%s: [gap %#018Lx-%#018Lx]\n",
> + u64 gap_size = range_start - range_end_prev;
> +
> + pr_info("%s: [gap %#018Lx-%#018Lx]",
> who,
> range_end_prev,
> range_start-1);
> +
> + e820_print_size(gap_size);
> + pr_cont(" ...\n");
> }
>
> - pr_info("%s: [mem %#018Lx-%#018Lx] ", who, range_start, range_end-1);
> + /* Print allocated ranges: */
> + pr_info("%s: [mem %#018Lx-%#018Lx]", who, range_start, range_end-1);
> + e820_print_size(entry->size);
> e820_print_type(entry->type);
> pr_cont("\n");
>
> --
> 2.45.2
>
--
Sincerely yours,
Mike.
* Mike Rapoport <rppt@kernel.org> wrote:
> On Mon, Apr 21, 2025 at 08:51:47PM +0200, Ingo Molnar wrote:
> > Before:
> >
> > BIOS-provided physical RAM map:
> > BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
> > BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
> > BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
> > BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] usable
> > BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] reserved
> > BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
> > BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
> > BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
> > BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
> > BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] reserved
> >
> > After:
> >
> > BIOS-provided physical RAM map:
> > BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] 639 KB kernel usable RAM
> > BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] 1 KB reserved
> > BIOS-e820: [gap 0x00000000000a0000-0x00000000000effff] 320 KB ...
> > BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] 64 KB reserved
> > BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] 1.9 GB kernel usable RAM
> > BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] 144 KB reserved
> > BIOS-e820: [gap 0x0000000080000000-0x00000000afffffff] 768 MB ...
> > BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] 256 MB reserved
> > BIOS-e820: [gap 0x00000000c0000000-0x00000000fed1bfff] 1005.1 MB ...
> > BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] 16 KB reserved
> > BIOS-e820: [gap 0x00000000fed20000-0x00000000feffbfff] 2.8 MB ...
> > BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] 16 KB reserved
> > BIOS-e820: [gap 0x00000000ff000000-0x00000000fffbffff] 15.7 MB ...
> > BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] 256 KB reserved
> > BIOS-e820: [gap 0x0000000100000000-0x000000fcffffffff] 1008 GB ...
> > BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] 12 GB reserved
> >
> > Note how a 1-digit precision field is printed out if a range is
> > fractional in its largest-enclosing natural size unit.
> >
> > So the "256 MB" and "12 GB" fields above denote exactly 256 MB and
> > 12 GB regions, while "1.9 GB" signals the region's fractional nature
> > and it being just below 2GB.
> >
> > Printing E820 maps with such details visualizes 'weird' ranges
> > at a glance, and gives users a better understanding of how
> > large the various ranges are, without having to perform hexadecimal
> > subtraction in their minds.
> >
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> > Cc: Andy Shevchenko <andy@kernel.org>
> > Cc: Arnd Bergmann <arnd@kernel.org>
> > Cc: David Woodhouse <dwmw@amazon.co.uk>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > (cherry picked from commit d1ac6b8718575a7ea2f0a1ff347835a8879df673)
> > ---
> > arch/x86/kernel/e820.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 45 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> > index 10bd10bd5672..8ee89962fcbf 100644
> > --- a/arch/x86/kernel/e820.c
> > +++ b/arch/x86/kernel/e820.c
> > @@ -199,6 +199,41 @@ static void __init e820_print_type(enum e820_type type)
> > }
> > }
> >
> > +/*
> > + * Print out the size of a E820 region, in human-readable
> > + * fashion, going from KB, MB, GB to TB units.
> > + *
> > + * Print out fractional sizes with a single digit of precision.
> > + */
> > +static void e820_print_size(u64 size)
> > +{
> > + if (size < SZ_1M) {
> > + if (size & (SZ_1K-1))
> > + pr_cont(" %4llu.%01llu KB", size/SZ_1K, 10*(size & (SZ_1K-1))/SZ_1K);
> > + else
> > + pr_cont(" %4llu KB", size/SZ_1K);
> > + return;
> > + }
>
> I'd make this a helper, e.g
>
> static void __e820_print_size(u64 size, u64 unit, const char *unit_name)
> {
> if (size & (unit - 1)) {
> u64 fraction = 10 * (size & (unit - 1))/unit;
>
> pr_cont(" %4llu.%01llu %s", size/unit, fraction, unit_name);
> } else {
> pr_cont(" %4llu %s", size/unit, unit_name);
> }
> }
While I like the helper in principle, it doesn't work on 32-bit: it's
an u64 with u64 division, while with the open coded literals the
compiler figures it out.
With div64_u64(), or a macro, it's not nearly as obvious a cleanup
IMHO.
Thanks,
Ingo
Mon, Apr 21, 2025 at 08:51:47PM +0200, Ingo Molnar kirjoitti:
> Before:
>
> BIOS-provided physical RAM map:
> BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
> BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
> BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
> BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] usable
> BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] reserved
> BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
> BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
> BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
> BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
> BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] reserved
>
> After:
>
> BIOS-provided physical RAM map:
> BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] 639 KB kernel usable RAM
> BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] 1 KB reserved
> BIOS-e820: [gap 0x00000000000a0000-0x00000000000effff] 320 KB ...
> BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] 64 KB reserved
> BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] 1.9 GB kernel usable RAM
> BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] 144 KB reserved
> BIOS-e820: [gap 0x0000000080000000-0x00000000afffffff] 768 MB ...
> BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] 256 MB reserved
> BIOS-e820: [gap 0x00000000c0000000-0x00000000fed1bfff] 1005.1 MB ...
> BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] 16 KB reserved
> BIOS-e820: [gap 0x00000000fed20000-0x00000000feffbfff] 2.8 MB ...
> BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] 16 KB reserved
> BIOS-e820: [gap 0x00000000ff000000-0x00000000fffbffff] 15.7 MB ...
> BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] 256 KB reserved
> BIOS-e820: [gap 0x0000000100000000-0x000000fcffffffff] 1008 GB ...
> BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] 12 GB reserved
>
> Note how a 1-digit precision field is printed out if a range is
> fractional in its largest-enclosing natural size unit.
>
> So the "256 MB" and "12 GB" fields above denote exactly 256 MB and
> 12 GB regions, while "1.9 GB" signals the region's fractional nature
> and it being just below 2GB.
>
> Printing E820 maps with such details visualizes 'weird' ranges
> at a glance, and gives users a better understanding of how
> large the various ranges are, without having to perform hexadecimal
> subtraction in their minds.
...
> +/*
> + * Print out the size of a E820 region, in human-readable
> + * fashion, going from KB, MB, GB to TB units.
> + *
> + * Print out fractional sizes with a single digit of precision.
> + */
> +static void e820_print_size(u64 size)
> +{
> + if (size < SZ_1M) {
> + if (size & (SZ_1K-1))
> + pr_cont(" %4llu.%01llu KB", size/SZ_1K, 10*(size & (SZ_1K-1))/SZ_1K);
> + else
> + pr_cont(" %4llu KB", size/SZ_1K);
I would add some spaces here and there for the sake of readability.
> + return;
> + }
> + if (size < SZ_1G) {
Can be written in one line as
} else if (...) {
Ditto for the rest.
> + if (size & (SZ_1M-1))
> + pr_cont(" %4llu.%01llu MB", size/SZ_1M, 10*(size & (SZ_1M-1))/SZ_1M);
> + else
> + pr_cont(" %4llu MB", size/SZ_1M);
> + return;
> + }
> + if (size < SZ_1T) {
> + if (size & (SZ_1G-1))
> + pr_cont(" %4llu.%01llu GB", size/SZ_1G, 10*(size & (SZ_1G-1))/SZ_1G);
> + else
> + pr_cont(" %4llu GB", size/SZ_1G);
> + return;
> + }
> + if (size & (SZ_1T-1))
> + pr_cont(" %4llu.%01llu TB", size/SZ_1T, 10*(size & (SZ_1T-1))/SZ_1T);
> + else
> + pr_cont(" %4llu TB", size/SZ_1T);
> +}
Don't you want to use string_helpers.h provided API?
string_get_size().
--
With Best Regards,
Andy Shevchenko
* Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> Mon, Apr 21, 2025 at 08:51:47PM +0200, Ingo Molnar kirjoitti:
> > Before:
> >
> > BIOS-provided physical RAM map:
> > BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
> > BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
> > BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
> > BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] usable
> > BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] reserved
> > BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
> > BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
> > BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
> > BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
> > BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] reserved
> >
> > After:
> >
> > BIOS-provided physical RAM map:
> > BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] 639 KB kernel usable RAM
> > BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] 1 KB reserved
> > BIOS-e820: [gap 0x00000000000a0000-0x00000000000effff] 320 KB ...
> > BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] 64 KB reserved
> > BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] 1.9 GB kernel usable RAM
> > BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] 144 KB reserved
> > BIOS-e820: [gap 0x0000000080000000-0x00000000afffffff] 768 MB ...
> > BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] 256 MB reserved
> > BIOS-e820: [gap 0x00000000c0000000-0x00000000fed1bfff] 1005.1 MB ...
> > BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] 16 KB reserved
> > BIOS-e820: [gap 0x00000000fed20000-0x00000000feffbfff] 2.8 MB ...
> > BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] 16 KB reserved
> > BIOS-e820: [gap 0x00000000ff000000-0x00000000fffbffff] 15.7 MB ...
> > BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] 256 KB reserved
> > BIOS-e820: [gap 0x0000000100000000-0x000000fcffffffff] 1008 GB ...
> > BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] 12 GB reserved
> >
> > Note how a 1-digit precision field is printed out if a range is
> > fractional in its largest-enclosing natural size unit.
> >
> > So the "256 MB" and "12 GB" fields above denote exactly 256 MB and
> > 12 GB regions, while "1.9 GB" signals the region's fractional nature
> > and it being just below 2GB.
> >
> > Printing E820 maps with such details visualizes 'weird' ranges
> > at a glance, and gives users a better understanding of how
> > large the various ranges are, without having to perform hexadecimal
> > subtraction in their minds.
>
> ...
>
> > +/*
> > + * Print out the size of a E820 region, in human-readable
> > + * fashion, going from KB, MB, GB to TB units.
> > + *
> > + * Print out fractional sizes with a single digit of precision.
> > + */
> > +static void e820_print_size(u64 size)
> > +{
> > + if (size < SZ_1M) {
> > + if (size & (SZ_1K-1))
> > + pr_cont(" %4llu.%01llu KB", size/SZ_1K, 10*(size & (SZ_1K-1))/SZ_1K);
> > + else
> > + pr_cont(" %4llu KB", size/SZ_1K);
>
> I would add some spaces here and there for the sake of readability.
I think it's perfectly readable, skipping the whitespace for numeric
literals is standard style. Linus himself does that occasionally, see:
94a2bc0f611c ("arm64: add 'runtime constant' support")
static inline void __runtime_fixup_ptr(void *where, unsigned long val)
{
__le32 *p = lm_alias(where);
__runtime_fixup_16(p, val);
__runtime_fixup_16(p+1, val >> 16);
__runtime_fixup_16(p+2, val >> 32);
__runtime_fixup_16(p+3, val >> 48);
__runtime_fixup_caches(where, 4);
}
Or:
938df695e98d ("vsprintf: associate the format state with the format pointer")
+ unsigned int shift = 32 - size*8;
which uses visual grouping to make arithmethic expressions more
readable.
>
> > + return;
> > + }
> > + if (size < SZ_1G) {
>
> Can be written in one line as
>
> } else if (...) {
Done. (See delta patch below.)
>
> Ditto for the rest.
>
> > + if (size & (SZ_1M-1))
> > + pr_cont(" %4llu.%01llu MB", size/SZ_1M, 10*(size & (SZ_1M-1))/SZ_1M);
> > + else
> > + pr_cont(" %4llu MB", size/SZ_1M);
> > + return;
> > + }
> > + if (size < SZ_1T) {
> > + if (size & (SZ_1G-1))
> > + pr_cont(" %4llu.%01llu GB", size/SZ_1G, 10*(size & (SZ_1G-1))/SZ_1G);
> > + else
> > + pr_cont(" %4llu GB", size/SZ_1G);
> > + return;
> > + }
> > + if (size & (SZ_1T-1))
> > + pr_cont(" %4llu.%01llu TB", size/SZ_1T, 10*(size & (SZ_1T-1))/SZ_1T);
> > + else
> > + pr_cont(" %4llu TB", size/SZ_1T);
> > +}
>
> Don't you want to use string_helpers.h provided API?
> string_get_size().
I don't think string_get_size() knows the fine distinction between:
BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] 256 KB device reserved
and:
BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] 256.0 KB device reserved
"256 KB" is exactly 256 KB, while "256.0 KB" denotes a value that is a
bit larger than 256 KB but rounds down to 256 KB at 1 KB granularity.
When reading platform boot logs it's useful to know when such values
are exact, at a glance.
Thanks,
Ingo
====================>
arch/x86/kernel/e820.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 7f600d32a999..67a477203c97 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -213,22 +213,19 @@ static void e820_print_size(u64 size)
else
pr_cont(" %4llu KB", size/SZ_1K);
return;
- }
- if (size < SZ_1G) {
+ } else if (size < SZ_1G) {
if (size & (SZ_1M-1))
pr_cont(" %4llu.%01llu MB", size/SZ_1M, 10*(size & (SZ_1M-1))/SZ_1M);
else
pr_cont(" %4llu MB", size/SZ_1M);
return;
- }
- if (size < SZ_1T) {
+ } else if (size < SZ_1T) {
if (size & (SZ_1G-1))
pr_cont(" %4llu.%01llu GB", size/SZ_1G, 10*(size & (SZ_1G-1))/SZ_1G);
else
pr_cont(" %4llu GB", size/SZ_1G);
return;
- }
- if (size & (SZ_1T-1))
+ } else if (size & (SZ_1T-1))
pr_cont(" %4llu.%01llu TB", size/SZ_1T, 10*(size & (SZ_1T-1))/SZ_1T);
else
pr_cont(" %4llu TB", size/SZ_1T);
On Thu, May 15, 2025 at 12:44:06PM +0200, Ingo Molnar wrote:
> * Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > Mon, Apr 21, 2025 at 08:51:47PM +0200, Ingo Molnar kirjoitti:
...
> > > + if (size & (SZ_1T-1))
> > > + pr_cont(" %4llu.%01llu TB", size/SZ_1T, 10*(size & (SZ_1T-1))/SZ_1T);
> > > + else
> > > + pr_cont(" %4llu TB", size/SZ_1T);
> > > +}
> >
> > Don't you want to use string_helpers.h provided API?
> > string_get_size().
>
> I don't think string_get_size() knows the fine distinction between:
>
> BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] 256 KB device reserved
>
> and:
>
> BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] 256.0 KB device reserved
>
> "256 KB" is exactly 256 KB, while "256.0 KB" denotes a value that is a
> bit larger than 256 KB but rounds down to 256 KB at 1 KB granularity.
>
> When reading platform boot logs it's useful to know when such values
> are exact, at a glance.
We can patch string_size() to print precise integers without fractional part.
--
With Best Regards,
Andy Shevchenko
* Ingo Molnar <mingo@kernel.org> wrote:
> > > + }
> > > + if (size < SZ_1G) {
> >
> > Can be written in one line as
> >
> > } else if (...) {
>
> Done. (See delta patch below.)
Actually, I take this back, as there's a return in the branch above:
pr_cont(" %4llu MB", size/SZ_1M);
return;
}
if (size < SZ_1T) {
Which makes the plain 'if' more readable: the previous 'if' branches
off entirely and control flow never gets back, so mix the blocks with
'else if' looks a bit weird.
Thanks,
Ingo
© 2016 - 2025 Red Hat, Inc.