linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Elliott, Robert (Persistent Memory)" <elliott@hpe.com>
To: James Bottomley <James.Bottomley@HansenPartnership.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Matt Fleming <matt@codeblueprint.co.uk>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	"linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-kernel @ vger . kernel . org"
	<linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap
Date: Mon, 25 Jan 2016 18:02:19 +0000	[thread overview]
Message-ID: <94D0CD8314A33A4D9D801C0FE68B40295BF3B840@G9W0745.americas.hpqcorp.net> (raw)
In-Reply-To: <1453567445.2470.24.camel@HansenPartnership.com>




> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
> Sent: Saturday, January 23, 2016 10:44 AM
> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Matt Fleming
> <matt@codeblueprint.co.uk>; Thomas Gleixner <tglx@linutronix.de>; Ingo
> Molnar <mingo@redhat.com>; H . Peter Anvin <hpa@zytor.com>; linux-
> efi@vger.kernel.org; Rasmus Villemoes <linux@rasmusvillemoes.dk>; Andrew
> Morton <akpm@linux-foundation.org>; linux-kernel @ vger . kernel . org
> <linux-kernel@vger.kernel.org>
> Cc: Elliott, Robert (Persistent Memory) <elliott@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 <elliott@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

  parent reply	other threads:[~2016-01-25 18:02 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-23 14:55 [PATCH v3 0/4] x86/efi: use binary units when printing Andy Shevchenko
2016-01-23 14:55 ` [PATCH v3 1/4] lib/string_helpers: export string_units_{2,10} for others Andy Shevchenko
2016-01-23 16:14   ` James Bottomley
2016-01-23 16:58     ` Andy Shevchenko
2016-01-23 14:55 ` [PATCH v3 2/4] lib/string_helpers: fix indentation in few places Andy Shevchenko
2016-01-23 14:55 ` [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap Andy Shevchenko
2016-01-23 16:44   ` James Bottomley
2016-01-23 17:18     ` Andy Shevchenko
2016-01-23 18:03       ` James Bottomley
2016-01-25  8:31         ` Andy Shevchenko
2016-01-25 15:25           ` James Bottomley
2016-01-23 18:12       ` James Bottomley
2016-01-23 20:29         ` One Thousand Gnomes
2016-01-23 20:43           ` H. Peter Anvin
2016-01-25 18:02     ` Elliott, Robert (Persistent Memory) [this message]
2016-01-25 18:56       ` James Bottomley
2016-01-25 19:28         ` Andy Shevchenko
2016-01-25 19:45           ` James Bottomley
2016-01-25 20:01             ` Andy Shevchenko
2016-01-25 20:18               ` James Bottomley
2016-01-25 20:37               ` Elliott, Robert (Persistent Memory)
2016-01-26 11:50                 ` Matt Fleming
2016-01-26 11:59                   ` Andy Shevchenko
2016-01-28  9:29                     ` Matt Fleming
2016-01-28 11:15                       ` Andy Shevchenko
2016-01-25 20:44               ` James Bottomley
2016-01-23 14:55 ` [PATCH v3 4/4] x86/efi: Use proper units in efi_find_mirror() Andy Shevchenko
2016-01-23 16:34 ` [PATCH v3 0/4] x86/efi: use binary units when printing James Bottomley
2016-01-23 17:20   ` Andy Shevchenko

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=94D0CD8314A33A4D9D801C0FE68B40295BF3B840@G9W0745.americas.hpqcorp.net \
    --to=elliott@hpe.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=matt@codeblueprint.co.uk \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).