[PATCH v2 1/4] lib/string_helpers: export string_units_{2,10} for others

[PATCH v2 1/4] lib/string_helpers: export string_units_{2,10} for others

[Andy Shevchenko]

There is one user coming which would like to use those string arrays. It might
be useful for any other user in the future.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/string_helpers.h |  6 ++++++
 lib/string_helpers.c           | 21 ++++++++++++---------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index dabe643..a55c9cc 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -10,6 +10,12 @@ enum string_size_units {
 	STRING_UNITS_2,		/* use binary powers of 2^10 */
 };
 
+#define STRING_UNITS_10_NUM	9
+#define STRING_UNITS_2_NUM	9
+
+extern const char *const string_units_10[STRING_UNITS_10_NUM];
+extern const char *const string_units_2[STRING_UNITS_2_NUM];
+
 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 5939f63..7ee4644 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -13,6 +13,15 @@
 #include <linux/string.h>
 #include <linux/string_helpers.h>
 
+const char * const string_units_10[STRING_UNITS_10_NUM] = {
+	"B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB",
+};
+EXPORT_SYMBOL(string_units_10);
+const char * const string_units_2[STRING_UNITS_2_NUM] = {
+	"B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB",
+};
+EXPORT_SYMBOL(string_units_2);
+
 /**
  * string_get_size - get the size in the specified units
  * @size:	The size to be converted in blocks
@@ -29,15 +38,9 @@
 void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
 		     char *buf, int len)
 {
-	static const char *const units_10[] = {
-		"B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
-	};
-	static const char *const units_2[] = {
-		"B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"
-	};
 	static const char *const *const units_str[] = {
-		[STRING_UNITS_10] = units_10,
-		[STRING_UNITS_2] = units_2,
+		[STRING_UNITS_10] = string_units_10,
+		[STRING_UNITS_2] = string_units_2,
 	};
 	static const unsigned int divisor[] = {
 		[STRING_UNITS_10] = 1000,
@@ -92,7 +95,7 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
 	}
 
  out:
-	if (i >= ARRAY_SIZE(units_2))
+	if (i >= STRING_UNITS_2_NUM)
 		unit = "UNK";
 	else
 		unit = units_str[units][i];
-- 
2.7.0.rc3

[PATCH v2 2/4] lib/string_helpers: fix indentation in few places

[Andy Shevchenko]

Fix the indentation of label and put snprintf() to one line.

There is no functional change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 lib/string_helpers.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 7ee4644..51fcea2 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -94,14 +94,13 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
 		tmp[j+1] = '\0';
 	}
 
- out:
+out:
 	if (i >= STRING_UNITS_2_NUM)
 		unit = "UNK";
 	else
 		unit = units_str[units][i];
 
-	snprintf(buf, len, "%u%s %s", (u32)size,
-		 tmp, unit);
+	snprintf(buf, len, "%u%s %s", (u32)size, tmp, unit);
 }
 EXPORT_SYMBOL(string_get_size);
 
-- 
2.7.0.rc3

Re: [PATCH v2 1/4] lib/string_helpers: export string_units_{2,10} for others

[James Bottomley]

On Thu, 2016-01-21 at 17:22 +0200, Andy Shevchenko wrote:
> There is one user coming which would like to use those string arrays.
> It might
> be useful for any other user in the future.

Well, let's not do it until we have an actual consumer because that
will help us get the interface correct.

> Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>  include/linux/string_helpers.h |  6 ++++++
>  lib/string_helpers.c           | 21 ++++++++++++---------
>  2 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/string_helpers.h
> b/include/linux/string_helpers.h
> index dabe643..a55c9cc 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -10,6 +10,12 @@ enum string_size_units {
>  	STRING_UNITS_2,		/* use binary powers of 2^10
> */
>  };
>  
> +#define STRING_UNITS_10_NUM	9
> +#define STRING_UNITS_2_NUM	9
> +
> +extern const char *const string_units_10[STRING_UNITS_10_NUM];
> +extern const char *const string_units_2[STRING_UNITS_2_NUM];
> +
>  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 5939f63..7ee4644 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -13,6 +13,15 @@
>  #include <linux/string.h>
>  #include <linux/string_helpers.h>
>  
> +const char * const string_units_10[STRING_UNITS_10_NUM] = {
> +	"B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB",
> +};
> +EXPORT_SYMBOL(string_units_10);
> +const char * const string_units_2[STRING_UNITS_2_NUM] = {
> +	"B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB",
> +};
> +EXPORT_SYMBOL(string_units_2);
> +

This is a pretty silly thing to do; how does someone who adds a unit to
one of the string_units know to increment STRING_UNITS_X_NUM?  Even if
you add a comment admonishing them to do it, it's far better to have
this calculated at compile time like it was before this patch.

James

Re: [PATCH v2 1/4] lib/string_helpers: export string_units_{2,10} for others

[Andy Shevchenko]

On Sat, Jan 23, 2016 at 7:13 AM, James Bottomley
<James.Bottomley-JuX6DAaQMKPCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> wrote:
> On Thu, 2016-01-21 at 17:22 +0200, Andy Shevchenko wrote:
>> There is one user coming which would like to use those string arrays.
>> It might
>> be useful for any other user in the future.
>
> Well, let's not do it until we have an actual consumer because that
> will help us get the interface correct.

First consumer is in patch 3.

>> +#define STRING_UNITS_10_NUM  9
>> +#define STRING_UNITS_2_NUM   9
>> +
>> +extern const char *const string_units_10[STRING_UNITS_10_NUM];
>> +extern const char *const string_units_2[STRING_UNITS_2_NUM];
>> +

>> --- a/lib/string_helpers.c
>> +++ b/lib/string_helpers.c
>> @@ -13,6 +13,15 @@
>>  #include <linux/string.h>
>>  #include <linux/string_helpers.h>
>>
>> +const char * const string_units_10[STRING_UNITS_10_NUM] = {
>> +     "B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB",
>> +};
>> +EXPORT_SYMBOL(string_units_10);
>> +const char * const string_units_2[STRING_UNITS_2_NUM] = {
>> +     "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB",
>> +};
>> +EXPORT_SYMBOL(string_units_2);
>> +
>
> This is a pretty silly thing to do; how does someone who adds a unit to
> one of the string_units know to increment STRING_UNITS_X_NUM?  Even if
> you add a comment admonishing them to do it, it's far better to have
> this calculated at compile time like it was before this patch.

Okay, I will think how to do that.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 3/4] x86/efi: print size and base in binary units in efi_print_memmap

[Andy Shevchenko]

From: Robert Elliott <elliott@hpe.com>

Print the base 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

Re: [PATCH v2 3/4] x86/efi: print size and base in binary units in efi_print_memmap

[Andrew Morton]

On Thu, 21 Jan 2016 17:22:31 +0200 Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:

> From: Robert Elliott <elliott-ZPxbGqLxI0U@public.gmane.org>
> 
> Print the base 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)

hm,

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

Where did this " - 1" come from?  I can't find a tree which has this.

> -			(md->num_pages >> (20 - EFI_PAGE_SHIFT)));
> +			md->phys_addr, md->phys_addr + size - 1,

So I did s/ - 1// here, but worried.

> +			efi_size_format(buf3, sizeof(buf3), size));
>  	}
>  #endif  /*  EFI_DEBUG  */
>  }

Re: [PATCH v2 3/4] x86/efi: print size and base in binary units in efi_print_memmap

[Andy Shevchenko]

On Fri, Jan 22, 2016 at 12:35 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Thu, 21 Jan 2016 17:22:31 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
>> From: Robert Elliott <elliott@hpe.com>
>>
>> Print the base 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)
>
> hm,
>
>> @@ -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,
>
> Where did this " - 1" come from?  I can't find a tree which has this.

http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=next&id=b324ee15566d9de933e7926c37a4d091904a513b

>
>> -                     (md->num_pages >> (20 - EFI_PAGE_SHIFT)));
>> +                     md->phys_addr, md->phys_addr + size - 1,
>
> So I did s/ - 1// here, but worried.

I'm pretty sure the series should go via efi tree.

>
>> +                     efi_size_format(buf3, sizeof(buf3), size));
>>       }
>>  #endif  /*  EFI_DEBUG  */
>>  }
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/4] x86/efi: print size and base in binary units in efi_print_memmap

[Matt Fleming]

On Fri, 22 Jan, at 09:08:00AM, Andy Shevchenko wrote:
> On Fri, Jan 22, 2016 at 12:35 AM, Andrew Morton
> <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
> > On Thu, 21 Jan 2016 17:22:31 +0200 Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> >
> >> From: Robert Elliott <elliott-ZPxbGqLxI0U@public.gmane.org>
> >>
> >> Print the base 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)
> >
> > hm,
> >
> >> @@ -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,
> >
> > Where did this " - 1" come from?  I can't find a tree which has this.
> 
> http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=next&id=b324ee15566d9de933e7926c37a4d091904a513b
> 
> >
> >> -                     (md->num_pages >> (20 - EFI_PAGE_SHIFT)));
> >> +                     md->phys_addr, md->phys_addr + size - 1,
> >
> > So I did s/ - 1// here, but worried.
> 
> I'm pretty sure the series should go via efi tree.

Right, because of the prerequisite patches sat in my tree it makes
sense for them to all be grouped together.

Andrew, would you mind dropping these from your tree and I'll take
everything through the EFI tree?

Additionally, if you and Rasmus wanted to throw a couple of Acked-by
or Reviewed-by tags my way, I'd feel better about the string_helpers
patches.

Re: [PATCH v2 3/4] x86/efi: print size and base in binary units in efi_print_memmap

[Andrew Morton]

On Fri, 22 Jan 2016 15:11:00 +0000 Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:

> > >> +                     md->phys_addr, md->phys_addr + size - 1,
> > >
> > > So I did s/ - 1// here, but worried.
> > 
> > I'm pretty sure the series should go via efi tree.
> 
> Right, because of the prerequisite patches sat in my tree it makes
> sense for them to all be grouped together.
> 
> Andrew, would you mind dropping these from your tree and I'll take
> everything through the EFI tree?
> 
> Additionally, if you and Rasmus wanted to throw a couple of Acked-by
> or Reviewed-by tags my way, I'd feel better about the string_helpers
> patches.

No probs.  Please add Acked-by: Andrew Morton
<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> to [1/4] and [2/4].

[PATCH v2 4/4] x86/efi: Use proper units in efi_find_mirror()

[Andy Shevchenko]

Like in efi_print_mmap() use the proper units when printing sizes.

Currently it's hardcoded to 'MiB', though it might be changed to adaptive
efi_size_format() in the future.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/platform/efi/efi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 3badc8a..95f70da 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -144,8 +144,8 @@ void __init efi_find_mirror(void)
 		}
 	}
 	if (mirror_size)
-		pr_info("Memory: %lldM/%lldM mirrored memory\n",
-			mirror_size>>20, total_size>>20);
+		pr_info("Memory: %lld MiB/%lld MiB mirrored memory\n",
+			mirror_size >> 20, total_size >> 20);
 }
 
 /*
-- 
2.7.0.rc3