Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1315625 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-23 16:00 +0100
    Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-01-23 17:50 +0100
      Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-01-23 18:20 +0100
        Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-01-23 19:10 +0100
          Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-25 09:40 +0100
            Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-01-25 16:30 +0100
        Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-01-23 19:20 +0100
          Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-23 21:40 +0100
            Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap "H. Peter Anvin" <hpa@zytor.com> - 2016-01-23 21:50 +0100
      RE: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap "Elliott, Robert (Persistent Memory)" <elliott@hpe.com> - 2016-01-25 19:10 +0100
        Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-01-25 20:00 +0100
          Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-01-25 20:30 +0100
            Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-01-25 20:50 +0100
              Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-01-25 21:10 +0100
                Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-01-25 21:20 +0100
                RE: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap "Elliott, Robert (Persistent Memory)" <elliott@hpe.com> - 2016-01-25 21:40 +0100
                  Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap Matt Fleming <matt@codeblueprint.co.uk> - 2016-01-26 13:00 +0100
                    Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-26 13:00 +0100
                Re: [PATCH v3 3/4] x86/efi: print size in binary units in  efi_print_memmap James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-01-25 21:50 +0100

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

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.

Use proper IEC binary units (KiB, MiB, etc.) rather than misuse SI
decimal units (KB, MB, etc.).

old:
    efi: mem61: [Persistent Memory  |   |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x0000000880000000-0x0000000c7fffffff) (16384MB)

new:
    efi: mem61: [Persistent Memory  |   |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x0000000880000000-0x0000000c7fffffff] (16 GiB)

Signed-off-by: Robert Elliott <elliott@hpe.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/platform/efi/efi.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e0846b5..3badc8a 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -35,10 +35,12 @@
 #include <linux/efi.h>
 #include <linux/efi-bgrt.h>
 #include <linux/export.h>
+#include <linux/bitops.h>
 #include <linux/bootmem.h>
 #include <linux/slab.h>
 #include <linux/memblock.h>
 #include <linux/spinlock.h>
+#include <linux/string_helpers.h>
 #include <linux/uaccess.h>
 #include <linux/time.h>
 #include <linux/io.h>
@@ -117,6 +119,14 @@ void efi_get_time(struct timespec *now)
 	now->tv_nsec = 0;
 }
 
+static char * __init efi_size_format(char *buf, size_t size, u64 bytes)
+{
+	unsigned long i = bytes ? __ffs64(bytes) / 10 : 0;
+
+	snprintf(buf, size, "%llu %s", bytes >> (i * 10), string_units_2[i]);
+	return buf;
+}
+
 void __init efi_find_mirror(void)
 {
 	void *p;
@@ -225,21 +235,20 @@ int __init efi_memblock_x86_reserve_range(void)
 void __init efi_print_memmap(void)
 {
 #ifdef EFI_DEBUG
-	efi_memory_desc_t *md;
 	void *p;
 	int i;
 
 	for (p = memmap.map, i = 0;
 	     p < memmap.map_end;
 	     p += memmap.desc_size, i++) {
-		char buf[64];
+		efi_memory_desc_t *md = p;
+		u64 size = md->num_pages << EFI_PAGE_SHIFT;
+		char buf[64], buf3[32];
 
-		md = p;
-		pr_info("mem%02u: %s range=[0x%016llx-0x%016llx] (%lluMB)\n",
+		pr_info("mem%02u: %s range=[0x%016llx-0x%016llx] (%s)\n",
 			i, efi_md_typeattr_format(buf, sizeof(buf), md),
-			md->phys_addr,
-			md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - 1,
-			(md->num_pages >> (20 - EFI_PAGE_SHIFT)));
+			md->phys_addr, md->phys_addr + size - 1,
+			efi_size_format(buf3, sizeof(buf3), size));
 	}
 #endif  /*  EFI_DEBUG  */
 }
-- 
2.7.0.rc3

[toc] | [next] | [standalone]

#1315695 — 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.
> 
> Use proper IEC binary units (KiB, MiB, etc.) rather than misuse SI
> decimal units (KB, MB, etc.).
> 
> old:
>     efi: mem61: [Persistent Memory  |   |  |  |  |  |  |  
>  |WB|WT|WC|UC] range=[0x0000000880000000-0x0000000c7fffffff)
> (16384MB)
> 
> new:
>     efi: mem61: [Persistent Memory  |   |  |  |  |  |  |  
>  |WB|WT|WC|UC] range=[0x0000000880000000-0x0000000c7fffffff] (16 GiB)
> 
> Signed-off-by: Robert Elliott <elliott@hpe.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/platform/efi/efi.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/platform/efi/efi.c
> b/arch/x86/platform/efi/efi.c
> index e0846b5..3badc8a 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -35,10 +35,12 @@
>  #include <linux/efi.h>
>  #include <linux/efi-bgrt.h>
>  #include <linux/export.h>
> +#include <linux/bitops.h>
>  #include <linux/bootmem.h>
>  #include <linux/slab.h>
>  #include <linux/memblock.h>
>  #include <linux/spinlock.h>
> +#include <linux/string_helpers.h>
>  #include <linux/uaccess.h>
>  #include <linux/time.h>
>  #include <linux/io.h>
> @@ -117,6 +119,14 @@ void efi_get_time(struct timespec *now)
>  	now->tv_nsec = 0;
>  }
>  
> +static char * __init efi_size_format(char *buf, size_t size, u64
> bytes)
> +{
> +	unsigned long i = bytes ? __ffs64(bytes) / 10 : 0;

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.

Not an attack on you patch per-se, but I really hate the IEC convention
that was essentially a ploy by disk manufacturers to inflate their disk
sizes by 10% simply by relabelling them.  Everyone was happy when a GB
was 2^30, now everyone's simply confused whenever they see GB.  We had
to pander to this in block devices because people got annoyed when we
reported a size that was different from the label but are you sure we
have to extend the madness to memory?

James


> +	snprintf(buf, size, "%llu %s", bytes >> (i * 10),
> string_units_2[i]);
>
> +	return buf;
> +}
> +
>  void __init efi_find_mirror(void)
>  {
>  	void *p;
> @@ -225,21 +235,20 @@ int __init efi_memblock_x86_reserve_range(void)
>  void __init efi_print_memmap(void)
>  {
>  #ifdef EFI_DEBUG
> -	efi_memory_desc_t *md;
>  	void *p;
>  	int i;
>  
>  	for (p = memmap.map, i = 0;
>  	     p < memmap.map_end;
>  	     p += memmap.desc_size, i++) {
> -		char buf[64];
> +		efi_memory_desc_t *md = p;
> +		u64 size = md->num_pages << EFI_PAGE_SHIFT;
> +		char buf[64], buf3[32];
>  
> -		md = p;
> -		pr_info("mem%02u: %s range=[0x%016llx-0x%016llx]
> (%lluMB)\n",
> +		pr_info("mem%02u: %s range=[0x%016llx-0x%016llx]
> (%s)\n",
>  			i, efi_md_typeattr_format(buf, sizeof(buf),
> md),
> -			md->phys_addr,
> -			md->phys_addr + (md->num_pages <<
> EFI_PAGE_SHIFT) - 1,
> -			(md->num_pages >> (20 - EFI_PAGE_SHIFT)));
> +			md->phys_addr, md->phys_addr + size - 1,
> +			efi_size_format(buf3, sizeof(buf3), size));
>  	}
>  #endif  /*  EFI_DEBUG  */
>  }

[toc] | [prev] | [next] | [standalone]

#1315703

On Sat, Jan 23, 2016 at 6:44 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Sat, 2016-01-23 at 16:55 +0200, Andy Shevchenko wrote:

>> +static char * __init efi_size_format(char *buf, size_t size, u64
>> bytes)
>> +{
>> +     unsigned long i = bytes ? __ffs64(bytes) / 10 : 0;
>
> What if size is zero, which might happen on a UEFI screw up?

size of what? Of input buffer?

>  Also it
> gives really odd results for non power of two memory sizes. 16384MB
> prints as 16GiB but 16385 prints as 16385MiB.

Adaptive precision. I don't think the idea is to print a nearby numbers here.

> 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.

No. You get it wrong. The initial idea was (actually not mine, see
authorship) to print an exact number with units and reduce whenever
it's possible, i.e number is a multiplication of certain unit.

> Not an attack on you patch per-se, but I really hate the IEC convention
> that was essentially a ploy by disk manufacturers to inflate their disk
> sizes by 10% simply by relabelling them.

>  Everyone was happy when a GB was 2^30,

No, not everyone. It was a misspelling done by some first storage
producer. Try to look at the problem from physics point of view. Units
are essential part of a value. There is an agreement how to use
multipliers and their code names. 1000 x Unit means kiloUnit. As per
agreement.

> now everyone's simply confused whenever they see GB.  We had
> to pander to this in block devices because people got annoyed when we
> reported a size that was different from the label but are you sure we
> have to extend the madness to memory?

I actually don't know who is from us is being more conservative. I
could call a madness to mess things from ancient (classical
mathematics and physics) with something which has less than hundred
years in development.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1315713 — Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

On Sat, 2016-01-23 at 19:18 +0200, Andy Shevchenko wrote:
> On Sat, Jan 23, 2016 at 6:44 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Sat, 2016-01-23 at 16:55 +0200, Andy Shevchenko wrote:
> 
> > > +static char * __init efi_size_format(char *buf, size_t size, u64
> > > bytes)
> > > +{
> > > +     unsigned long i = bytes ? __ffs64(bytes) / 10 : 0;
> > 
> > What if size is zero, which might happen on a UEFI screw up?
> 
> size of what? Of input buffer?

I mean when bytes == 0 ffs is undefined.

> >  Also it gives really odd results for non power of two memory 
> > sizes. 16384MB prints as 16GiB but 16385 prints as 16385MiB.
> 
> Adaptive precision. I don't think the idea is to print a nearby
> numbers here.

Well either there's a point to reducing to the nearest exponent or we
simply print everything in MB as the original did.  Doing it
inconsistently is asking for trouble ... and lots of user queries.  I
mean, supposing there's a range off by one ... now we print a huge
number in B.

I really advise against hacking around like this.  In any event if efi
must have this, please don't involve the parts of the kernel that try
to do this correctly, like lib/string_helpers.h

> > 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.
> 
> No. You get it wrong. The initial idea was (actually not mine, see
> authorship) to print an exact number with units and reduce whenever
> it's possible, i.e number is a multiplication of certain unit.

so you must implement the original idea no matter how inconsistent it
leads us to be?  Is it wrong to try to do better?

James

[toc] | [prev] | [next] | [standalone]

#1316274 — Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

On Sat, 2016-01-23 at 10:03 -0800, James Bottomley wrote:
> On Sat, 2016-01-23 at 19:18 +0200, Andy Shevchenko wrote:
> > On Sat, Jan 23, 2016 at 6:44 PM, James Bottomley
> > <James.Bottomley@hansenpartnership.com> wrote:
> > > On Sat, 2016-01-23 at 16:55 +0200, Andy Shevchenko wrote:
> > 
> > > > +static char * __init efi_size_format(char *buf, size_t size,
> > > > u64
> > > > bytes)
> > > > +{
> > > > +     unsigned long i = bytes ? __ffs64(bytes) / 10 : 0;
> > > 
> > > What if size is zero, which might happen on a UEFI screw up?
> > 
> > size of what? Of input buffer?
> 
> I mean when bytes == 0 ffs is undefined.

Well, someone misread the above code ;-)

There is ternary operator exactly to serve this purpose.

> 
> > >  Also it gives really odd results for non power of two memory 
> > > sizes. 16384MB prints as 16GiB but 16385 prints as 16385MiB.
> > 
> > Adaptive precision. I don't think the idea is to print a nearby
> > numbers here.
> 
> Well either there's a point to reducing to the nearest exponent or we
> simply print everything in MB as the original did.  Doing it
> inconsistently is asking for trouble ... and lots of user queries.  I
> mean, supposing there's a range off by one ... now we print a huge
> number in B.

> I really advise against hacking around like this.  In any event if
> efi
> must have this, please don't involve the parts of the kernel that try
> to do this correctly, like lib/string_helpers.h

> 
> > > 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.
> > 
> > No. You get it wrong. The initial idea was (actually not mine, see
> > authorship) to print an exact number with units and reduce whenever
> > it's possible, i.e number is a multiplication of certain unit.
> 
> so you must implement the original idea no matter how inconsistent it
> leads us to be?  Is it wrong to try to do better?

For both comments I prefer to hear Matt's opinion as he is maintainer
of EFI stuff.

My role in this all is to reduce the code base by avoiding 'not
invented here' syndrome.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[toc] | [prev] | [next] | [standalone]

#1316814 — Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

On Mon, 2016-01-25 at 10:31 +0200, Andy Shevchenko wrote:
> On Sat, 2016-01-23 at 10:03 -0800, James Bottomley wrote:
> > On Sat, 2016-01-23 at 19:18 +0200, Andy Shevchenko wrote:
> > > > 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.
> > > 
> > > No. You get it wrong. The initial idea was (actually not mine,
> > > see
> > > authorship) to print an exact number with units and reduce
> > > whenever
> > > it's possible, i.e number is a multiplication of certain unit.
> > 
> > so you must implement the original idea no matter how inconsistent
> > it
> > leads us to be?  Is it wrong to try to do better?
> 
> For both comments I prefer to hear Matt's opinion as he is maintainer
> of EFI stuff.
> 
> My role in this all is to reduce the code base by avoiding 'not
> invented here' syndrome.

To get up to the starting blocks on that one, you need a usable
interface.  Neither of the series has that for lib/string_helper.c, so
this is a NAK to both of those parts.

I already told you how to do it in a way that creates a maintainable
interface.  It looks obvious to me and I was politely waiting for you
to produce it, but if you're saying your not able to, I suppose I can
produce the patch.

James

[toc] | [prev] | [next] | [standalone]

#1315718 — Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

On Sat, 2016-01-23 at 19:18 +0200, Andy Shevchenko wrote:
> On Sat, Jan 23, 2016 at 6:44 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > Not an attack on you patch per-se, but I really hate the IEC
> > convention
> > that was essentially a ploy by disk manufacturers to inflate their
> > disk
> > sizes by 10% simply by relabelling them.
> 
> >  Everyone was happy when a GB was 2^30,
> 
> No, not everyone. It was a misspelling done by some first storage
> producer.

No, it was the adopted convention in computer science to use units in
2^10 since all sizes were usually binary.  It actually began with
memory sizes.  So a GB never meant 10^9 bytes before 2007 because the
classical unit users had no idea what a byte was.  The first bytes were
counted in powers of 2.

>  Try to look at the problem from physics point of view. Units
> are essential part of a value. There is an agreement how to use
> multipliers and their code names. 1000 x Unit means kiloUnit. As per
> agreement.

It's not about physics (and certainly astronomers, who often take pi to
be 1 would be happy with 2^10 = 10^3) it's about politics: disk
manufacturers wanted a way to report bigger sizes, so they made a huge
fuss about the "inconsistency" of computer science using units in
increments of 2^10 and forced through this IEC standard.

> > now everyone's simply confused whenever they see GB.  We had
> > to pander to this in block devices because people got annoyed when 
> > we reported a size that was different from the label but are you 
> > sure we have to extend the madness to memory?
> 
> I actually don't know who is from us is being more conservative. I
> could call a madness to mess things from ancient (classical
> mathematics and physics) with something which has less than hundred
> years in development.

In engineering terms, counting in powers of 2 makes a lot of sense for
quantites using binary address busses and the IEC even recognised that
by inventing a new unit for it.  Having GB mean 2^30 up to 2007 and
10^9 after it is confusing for everyone born before about 1990.

James

[toc] | [prev] | [next] | [standalone]

#1315749 — Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

All a bit revisionist. Everyone else on the planet was upset about it
because it broke things like calculating bit density because the prefixes
for the bit capacity are not in metric form. BIPM (keeper of the SI
units) never approved powers of two as an interpretation. IEC came into
line in 1999, ISO followed.

Disk sizes have been decimal since at least the 1970s. The original IBM
10MB hard disc for example was 10MB not 10MiB.

Powers of two are only validly referred to as KiB, MiB, GiB as of all
current standard body positions. Powers of 10 based units are kB, MB, GB)

(The best one is CD and DVD: DVD uses the proper definition, CD uses MiB,
although given the multiple sector sizes and encodings on CD it's all
manure anyway)

Alan

[toc] | [prev] | [next] | [standalone]

#1315752

On January 23, 2016 12:29:26 PM PST, One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> wrote:
>All a bit revisionist. Everyone else on the planet was upset about it
>because it broke things like calculating bit density because the
>prefixes
>for the bit capacity are not in metric form. BIPM (keeper of the SI
>units) never approved powers of two as an interpretation. IEC came into
>line in 1999, ISO followed.
>
>Disk sizes have been decimal since at least the 1970s. The original IBM
>10MB hard disc for example was 10MB not 10MiB.
>
>Powers of two are only validly referred to as KiB, MiB, GiB as of all
>current standard body positions. Powers of 10 based units are kB, MB,
>GB)
>
>(The best one is CD and DVD: DVD uses the proper definition, CD uses
>MiB,
>although given the multiple sector sizes and encodings on CD it's all
>manure anyway)
>
>Alan

Then there are oddball definitions like 1 MB = 1,024,000, which IBM used for disk for a long time.  At least IEC tried to come up with a unambiguous way to denote these prefixes.  It was less of an issue for kilo- since the binary prefix was always capitalized.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

[toc] | [prev] | [next] | [standalone]

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

> -----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

[toc] | [prev] | [next] | [standalone]

#1317197 — Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

On Mon, 2016-01-25 at 18:02 +0000, Elliott, Robert (Persistent Memory)
wrote:
> 
> 
> > -----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)

OK, this is getting a bit out of hand: I didn't say your aim was bad
... I think it's a reasonable desire; I said the proposed
implementation was bad.  Using ffs leads to precision runaway and
exporting an array from string_helpers.c is simply the wrong way to do
it.

Since we've now spent more time arguing about this than it would take
to do a correct patch, this is what I was thinking.  It extracts the
precision reduction core from string_helpers.c and exposes it to all
users who want to convert to units.  I added a nozeros option becuase I
think you want it to print 1 GiB rather than 1.00 GiB for exact powers
of two.  (OK, and I fixed a bug where it will report small amounts as
1.00 B instead of whole number of bytes).  Absent the nozero option,
you could simply have used string_get_size(), with a block size of 1.

James

---

diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index dabe643..78935fae 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -10,6 +10,8 @@ enum string_size_units {
 	STRING_UNITS_2,		/* use binary powers of 2^10 */
 };
 
+void string_get_units(u64 size, const enum string_size_units units,
+		      char *buf, int len, bool nozeros);
 void string_get_size(u64 size, u64 blk_size, enum string_size_units units,
 		     char *buf, int len);
 
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 5c88204..ab6b332 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -13,21 +13,13 @@
 #include <linux/string.h>
 #include <linux/string_helpers.h>
 
-/**
- * string_get_size - get the size in the specified units
- * @size:	The size to be converted in blocks
- * @blk_size:	Size of the block (use 1 for size in bytes)
- * @units:	units to use (powers of 1000 or 1024)
- * @buf:	buffer to format to
- * @len:	length of buffer
- *
- * This function returns a string formatted to 3 significant figures
- * giving the size in the required units.  @buf should have room for
- * at least 9 bytes and will always be zero terminated.
- *
- */
-void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
-		     char *buf, int len)
+static const unsigned int divisor[] = {
+	[STRING_UNITS_10] = 1000,
+	[STRING_UNITS_2] = 1024,
+};
+
+static void string_reduce(u64 size, int log, const enum string_size_units units,
+			  char *buf, int len, bool nozeros)
 {
 	static const char *const units_10[] = {
 		"B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
@@ -39,52 +31,23 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
 		[STRING_UNITS_10] = units_10,
 		[STRING_UNITS_2] = units_2,
 	};
-	static const unsigned int divisor[] = {
-		[STRING_UNITS_10] = 1000,
-		[STRING_UNITS_2] = 1024,
-	};
 	static const unsigned int rounding[] = { 500, 50, 5 };
-	int i = 0, j;
-	u32 remainder = 0, sf_cap;
+	char zeros[] = ".00";
+
+	int j;
+	u32 sf_cap, remainder = 0;
 	char tmp[8];
 	const char *unit;
 
 	tmp[0] = '\0';
 
-	if (blk_size == 0)
-		size = 0;
 	if (size == 0)
 		goto out;
 
-	/* This is Napier's algorithm.  Reduce the original block size to
-	 *
-	 * coefficient * divisor[units]^i
-	 *
-	 * we do the reduction so both coefficients are just under 32 bits so
-	 * that multiplying them together won't overflow 64 bits and we keep
-	 * as much precision as possible in the numbers.
-	 *
-	 * Note: it's safe to throw away the remainders here because all the
-	 * precision is in the coefficients.
-	 */
-	while (blk_size >> 32) {
-		do_div(blk_size, divisor[units]);
-		i++;
-	}
-
-	while (size >> 32) {
-		do_div(size, divisor[units]);
-		i++;
-	}
-
-	/* now perform the actual multiplication keeping i as the sum of the
-	 * two logarithms */
-	size *= blk_size;
-
-	/* and logarithmically reduce it until it's just under the divisor */
+	/* Logarithmically reduce it until it's just under the divisor */
 	while (size >= divisor[units]) {
 		remainder = do_div(size, divisor[units]);
-		i++;
+		log++;
 	}
 
 	/* work out in j how many digits of precision we need from the
@@ -109,21 +72,93 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
 		size += 1;
 	}
 
-	if (j) {
+	if (j && log) {
 		snprintf(tmp, sizeof(tmp), ".%03u", remainder);
 		tmp[j+1] = '\0';
+		zeros[j+1] = '\0';
+		if (nozeros && strcmp(tmp, zeros) == 0)
+			tmp[0]='\0';
 	}
 
  out:
-	if (i >= ARRAY_SIZE(units_2))
+	if (log >= ARRAY_SIZE(units_2))
 		unit = "UNK";
 	else
-		unit = units_str[units][i];
+		unit = units_str[units][log];
 
 	snprintf(buf, len, "%u%s %s", (u32)size,
 		 tmp, unit);
 }
-EXPORT_SYMBOL(string_get_size);
+
+/**
+ * string_get_units - convert size to specified units
+ * @size:	The quantity to be converted
+ * @units:	units to use (powers of 1000 or 1024)
+ * @buf:	buffer to format to
+ * @len:	length of buffer
+ * @nozereos:	eliminate zeros after the decimal point if true
+ *
+ * This function returns a string formatted to 3 significant figures
+ * giving the size in the required units.  @buf should have room for
+ * at least 9 bytes and will always be zero terminated.
+ */
+void string_get_units(u64 size, const enum string_size_units units,
+		      char *buf, int len, bool nozeros)
+{
+	string_reduce(size, 0, units, buf, len, nozeros);
+}
+
+/**
+ * string_get_size - get the size in the specified units
+ * @size:	The size to be converted in blocks
+ * @blk_size:	Size of the block (use 1 for size in bytes)
+ * @units:	units to use (powers of 1000 or 1024)
+ * @buf:	buffer to format to
+ * @len:	length of buffer
+ *
+ * This function returns a string formatted to 3 significant figures
+ * giving the size in the required units.  @buf should have room for
+ * at least 9 bytes and will always be zero terminated.
+ *
+ */
+void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
+		     char *buf, int len)
+{
+	int i = 0;
+
+	if (blk_size == 0)
+		size = 0;
+	if (size == 0)
+		goto out;
+
+	/* This is Napier's algorithm.  Reduce the original block size to
+	 *
+	 * coefficient * divisor[units]^i
+	 *
+	 * we do the reduction so both coefficients are just under 32 bits so
+	 * that multiplying them together won't overflow 64 bits and we keep
+	 * as much precision as possible in the numbers.
+	 *
+	 * Note: it's safe to throw away the remainders here because all the
+	 * precision is in the coefficients.
+	 */
+	while (blk_size >> 32) {
+		do_div(blk_size, divisor[units]);
+		i++;
+	}
+
+	while (size >> 32) {
+		do_div(size, divisor[units]);
+		i++;
+	}
+
+	/* now perform the actual multiplication keeping i as the sum of the
+	 * two logarithms */
+	size *= blk_size;
+
+ out:
+	string_reduce(size, i, units, buf, len, false);
+}
 
 static bool unescape_space(char **src, char **dst)
 {

[toc] | [prev] | [next] | [standalone]

#1317233

On Mon, Jan 25, 2016 at 8:56 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Mon, 2016-01-25 at 18:02 +0000, Elliott, Robert (Persistent Memory)
> wrote:

>  Using ffs leads to precision runaway

How exactly?!

> and
> exporting an array from string_helpers.c is simply the wrong way to do
> it.

This part I didn't object.

> Since we've now spent more time arguing about this than it would take
> to do a correct patch, this is what I was thinking.  It extracts the
> precision reduction core from string_helpers.c and exposes it to all
> users who want to convert to units.  I added a nozeros option becuase I
> think you want it to print 1 GiB rather than 1.00 GiB for exact powers
> of two.  (OK, and I fixed a bug where it will report small amounts as
> 1.00 B instead of whole number of bytes).  Absent the nozero option,
> you could simply have used string_get_size(), with a block size of 1.

It's good you are doing this better, but I still vote for __ffs64(),
since it would be faster on binary units.

Also, in one version I tried to convert couple of other users which
are using only KM (in general whatever range it would be) units. Any
ideas how to modify to support them?

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1317243 — Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

On Mon, 2016-01-25 at 21:28 +0200, Andy Shevchenko wrote:
> On Mon, Jan 25, 2016 at 8:56 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Mon, 2016-01-25 at 18:02 +0000, Elliott, Robert (Persistent
> > Memory)
> > wrote:
> 
> >  Using ffs leads to precision runaway
> 
> How exactly?!

Off by one.  A size of 0xffffffffffffffff prints 18446744073709551615 B
rather than 20 GiB.

> > and
> > exporting an array from string_helpers.c is simply the wrong way to
> > do
> > it.
> 
> This part I didn't object.
> 
> > Since we've now spent more time arguing about this than it would
> > take
> > to do a correct patch, this is what I was thinking.  It extracts
> > the
> > precision reduction core from string_helpers.c and exposes it to
> > all
> > users who want to convert to units.  I added a nozeros option
> > becuase I
> > think you want it to print 1 GiB rather than 1.00 GiB for exact
> > powers
> > of two.  (OK, and I fixed a bug where it will report small amounts
> > as
> > 1.00 B instead of whole number of bytes).  Absent the nozero
> > option,
> > you could simply have used string_get_size(), with a block size of
> > 1.
> 
> It's good you are doing this better, but I still vote for __ffs64(),
> since it would be faster on binary units.

Is speed of a start of day print a particular concern?

> Also, in one version I tried to convert couple of other users which
> are using only KM (in general whatever range it would be) units. Any
> ideas how to modify to support them?

You mean units in odd increments of 6 digits (so K, M, T ...)? no.  The
logarithmic reduction is done to the base of the unit increment (1000
or 1024) so it doesn't really fit this case and it would be hard to
adjust because we don't have enough precision in the remainder. 
 However, unless there's a huge need to keep it, I'd just fit to the
closest 3 digit increment and then everything would work.

James

[toc] | [prev] | [next] | [standalone]

#1317267

On Mon, Jan 25, 2016 at 9:45 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Mon, 2016-01-25 at 21:28 +0200, Andy Shevchenko wrote:
>> On Mon, Jan 25, 2016 at 8:56 PM, James Bottomley
>> <James.Bottomley@hansenpartnership.com> wrote:
>> > On Mon, 2016-01-25 at 18:02 +0000, Elliott, Robert (Persistent
>> > Memory)
>> > wrote:
>>
>> >  Using ffs leads to precision runaway
>>
>> How exactly?!
>
> Off by one.  A size of 0xffffffffffffffff prints 18446744073709551615 B
> rather than 20 GiB.

Because it's not a 20 GiB. It's exactly 20 GiB - 1 B.

AFAIU, the intention was to show _exact_ size.

>> It's good you are doing this better, but I still vote for __ffs64(),
>> since it would be faster on binary units.
>
> Is speed of a start of day print a particular concern?

If it's cheap to do, why not to do?

>> Also, in one version I tried to convert couple of other users which
>> are using only KM (in general whatever range it would be) units. Any
>> ideas how to modify to support them?
>
> You mean units in odd increments of 6 digits (so K, M, T ...)? no.  The
> logarithmic reduction is done to the base of the unit increment (1000
> or 1024) so it doesn't really fit this case and it would be hard to
> adjust because we don't have enough precision in the remainder.
>  However, unless there's a huge need to keep it, I'd just fit to the
> closest 3 digit increment and then everything would work.

KM case:
K) if 1 MiB > value >= 0 — prints in KiB
M) if ∞ > value >= 1 MiB — prints in MiB.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1317273 — Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

On Mon, 2016-01-25 at 22:01 +0200, Andy Shevchenko wrote:
> On Mon, Jan 25, 2016 at 9:45 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Mon, 2016-01-25 at 21:28 +0200, Andy Shevchenko wrote:
> > > On Mon, Jan 25, 2016 at 8:56 PM, James Bottomley
> > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > On Mon, 2016-01-25 at 18:02 +0000, Elliott, Robert (Persistent
> > > > Memory)
> > > > wrote:
> > > 
> > > >  Using ffs leads to precision runaway
> > > 
> > > How exactly?!
> > 
> > Off by one.  A size of 0xffffffffffffffff prints
> > 18446744073709551615 B
> > rather than 20 GiB.
> 
> Because it's not a 20 GiB. It's exactly 20 GiB - 1 B.
> 
> AFAIU, the intention was to show _exact_ size.

I think that's a bad idea:  The range shows you the exact stuff in hex,
so all the information is present if you want precision.  What's
printed in brackets is for humans to read.  I'd have to reach for a
calculator to work out that  18446744073709551615 is actually around 20
GiB.

James

[toc] | [prev] | [next] | [standalone]

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

---
Robert Elliott, HPE Persistent Memory


> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Monday, January 25, 2016 2:01 PM
> To: James Bottomley <James.Bottomley@hansenpartnership.com>
> Cc: Elliott, Robert (Persistent Memory) <elliott@hpe.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; 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
> 
> On Mon, Jan 25, 2016 at 9:45 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Mon, 2016-01-25 at 21:28 +0200, Andy Shevchenko wrote:
> >> On Mon, Jan 25, 2016 at 8:56 PM, James Bottomley
> >> <James.Bottomley@hansenpartnership.com> wrote:
> >> > On Mon, 2016-01-25 at 18:02 +0000, Elliott, Robert (Persistent
> >> > Memory)
> >> > wrote:
> >>
> >> >  Using ffs leads to precision runaway
> >>
> >> How exactly?!
> >
> > Off by one.  A size of 0xffffffffffffffff prints 18446744073709551615 B
> > rather than 20 GiB.
> 
> Because it's not a 20 GiB. It's exactly 20 GiB - 1 B.
> 
> AFAIU, the intention was to show _exact_ size.

For the UEFI memory map, that was indeed my intention.  I
don't want it silently round to "20 GiB".  Even rounding
to "19.999 GiB" is imprecise.

Another option could be to use a "~" prefix for imperfect
values, like "~20 GiB".  That would serve as a warning
that something's not quite right.

[toc] | [prev] | [next] | [standalone]

#1317783 — Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

On Mon, 25 Jan, at 08:37:58PM, Elliott, Robert (Persistent Memory) wrote:
> 
> For the UEFI memory map, that was indeed my intention.  I
> don't want it silently round to "20 GiB".  Even rounding
> to "19.999 GiB" is imprecise.

OK, let's just go with your original patch Robert (minus the @ addr
bit) since it's pretty small and does what we want for this specific
case.

[toc] | [prev] | [next] | [standalone]

#1317786 — Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

On Tue, 2016-01-26 at 11:50 +0000, Matt Fleming wrote:
> On Mon, 25 Jan, at 08:37:58PM, Elliott, Robert (Persistent Memory)
> wrote:
> > 
> > For the UEFI memory map, that was indeed my intention.  I
> > don't want it silently round to "20 GiB".  Even rounding
> > to "19.999 GiB" is imprecise.
> 
> OK, let's just go with your original patch Robert (minus the @ addr
> bit) since it's pretty small and does what we want for this specific
> case.

However I am against this, but seems reviewers do not leave a chance to
us, I would propose to copy-and-paste table of binary prefixes and use
__ffs64().

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[toc] | [prev] | [next] | [standalone]

#1317293 — Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap

On Mon, 2016-01-25 at 22:01 +0200, Andy Shevchenko wrote:
> On Mon, Jan 25, 2016 at 9:45 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Mon, 2016-01-25 at 21:28 +0200, Andy Shevchenko wrote:
> > > Also, in one version I tried to convert couple of other users
> > > which
> > > are using only KM (in general whatever range it would be) units.
> > > Any
> > > ideas how to modify to support them?
> > 
> > You mean units in odd increments of 6 digits (so K, M, T ...)? no. 
> >  The logarithmic reduction is done to the base of the unit 
> > increment (1000 or 1024) so it doesn't really fit this case and it 
> > would be hard to adjust because we don't have enough precision in 
> > the remainder.  However, unless there's a huge need to keep it, I'd 
> > just fit to the closest 3 digit increment and then everything would
> > work.
> 
> KM case:
> K) if 1 MiB > value >= 0 — prints in KiB
> M) if ∞ > value >= 1 MiB — prints in MiB.

Actually there is a way to do this: add a fixed precision argument that
would stop the logarithmic reduction when the desired precision were
reached.  You'd still have to do the precision discrimination in the
call, so something like

num > 1Mib ? call for precision of 6 : call for precision of 3

James

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web