RE: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: James Bottomley [mailto:james.bottom...@hansenpartnership.com]
> Sent: Saturday, January 23, 2016 10:44 AM
> To: Andy Shevchenko <andriy.shevche...@linux.intel.com>; Matt Fleming
> <m...@codeblueprint.co.uk>; Thomas Gleixner <t...@linutronix.de>; Ingo
> Molnar <mi...@redhat.com>; H . Peter Anvin <h...@zytor.com>; linux-
> e...@vger.kernel.org; Rasmus Villemoes <li...@rasmusvillemoes.dk>; Andrew
> Morton <a...@linux-foundation.org>; linux-kernel @ vger . kernel . org
> <linux-kernel@vger.kernel.org>
> Cc: Elliott, Robert (Persistent Memory) <elli...@hpe.com>
> Subject: Re: [PATCH v3 3/4] x86/efi: print size in binary units in
> efi_print_memmap
> 
> On Sat, 2016-01-23 at 16:55 +0200, Andy Shevchenko wrote:
> > From: Robert Elliott <elli...@hpe.com>
> >
> > Print the size in the best-fit B, KiB, MiB, etc. units rather than
> > always MiB. This avoids rounding, which can be misleading.
> >

...
> 
> What if size is zero, which might happen on a UEFI screw up?  

> Also it gives really odd results for non power of two memory sizes. 
> 16384MB prints as 16GiB but 16385 prints as 16385MiB.
> If the goal is to have a clean interface reporting only the first four
> significant figures and a size exponent, then a helper would be much
> better than trying to open code this ad hoc.

An impetus for the patch was to stop rounding the sub-MiB values,
which is misleading and can hide bugs.  For my systems, the
minimum size of a range happens to be 4 KiB, so I wanted at least
that resolution. However, I don't want to print everything as KiB,
because that makes big sizes less clear.

Example - old output:
efi: mem00: [Conventional Memory...] 
range=[0x0000000000000000-0x0000000000001000) (0MB)
efi: mem01: [Loader Data        ...] 
range=[0x0000000000001000-0x0000000000002000) (0MB)
efi: mem02: [Conventional Memory...] 
range=[0x0000000000002000-0x0000000000093000) (0MB)
efi: mem03: [Reserved           ...] 
range=[0x0000000000093000-0x0000000000094000) (0MB)

Proposed output:
efi: mem00: [Conventional Memory...] 
range=[0x0000000000000000-0x0000000000092fff] (588 KiB @ 0 B)
efi: mem01: [Reserved           ...] 
range=[0x0000000000093000-0x0000000000093fff] (4 KiB @ 588 KiB)
efi: mem02: [Conventional Memory...] 
range=[0x0000000000094000-0x000000000009ffff] (48 KiB @ 592 KiB)
efi: mem03: [Loader Data        ...] 
range=[0x0000000000100000-0x00000000013e8fff] (19364 KiB @ 1 MiB)
(notes:
 - from a different system
 - including both base and size
 - Matt didn't like printing the base so that's been removed)

With persistent memory (NVDIMMs) bringing storage device capacities
into the memory subsystem, MiB is too small.  Seeing a 1 TiB NVDIMM
as 1 TiB is a lot clearer than having to recognize 1048576 MiB as
the same value (especially since these power-of-two quantities
don't just chop off zeros on the right).

Examples:
efi: mem50: [Runtime Data       ...] 
range=[0x00000000784ff000-0x00000000788fefff] (4 MiB @ 1971196 KiB)
efi: mem56: [Conventional Memory...] 
range=[0x0000000100000000-0x000000087fffffff] (30 GiB @ 4 GiB)
efi: mem58: [Memory Mapped I/O  ...] 
range=[0x0000000080000000-0x000000008fffffff] (256 MiB @ 2 GiB)
efi: mem60: [Persistent Memory  ...] 
range=[0x0000001480000000-0x0000001a7fffffff] (24 GiB @ 82 GiB)


---
Robert Elliott, HPE Persistent Memory