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

[PATCH v1 1/2] 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-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 include/linux/string_helpers.h |  3 +++
 lib/string_helpers.c           | 21 ++++++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index dabe643..1d16240 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -10,6 +10,9 @@ enum string_size_units {
 	STRING_UNITS_2,		/* use binary powers of 2^10 */
 };
 
+extern const char *const string_units_10[];
+extern const char *const string_units_2[];
+
 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..3a4c812 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[] = {
+	"B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB",
+};
+EXPORT_SYMBOL(string_units_10);
+const char * const string_units_2[] = {
+	"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 >= ARRAY_SIZE(string_units_2))
 		unit = "UNK";
 	else
 		unit = units_str[units][i];
-- 
2.6.4

[PATCH v1 2/2] x86/efi: print size and base in binary units in efi_print_memmap

[Andy Shevchenko]

From: Robert Elliott <elliott-ZPxbGqLxI0U@public.gmane.org>

Print the base address for each range in decimal alongside the size.
Use a "(size @ base)" format similar to the fake_memmap kernel parameter.

Print the range and 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 @ 34 GiB)

Signed-off-by: Robert Elliott <elliott-ZPxbGqLxI0U@public.gmane.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 arch/x86/platform/efi/efi.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e0846b5..e762969 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>
@@ -222,6 +224,19 @@ int __init efi_memblock_x86_reserve_range(void)
 	return 0;
 }
 
+static char * __init efi_size_format(char *buf, size_t size, u64 bytes)
+{
+	unsigned long i;
+
+	if (bytes)
+		i = __ffs64(bytes) / 10;
+	else
+		i = 0;
+
+	snprintf(buf, size, "%llu%s", bytes >> (i * 10), string_units_2[i]);
+	return buf;
+}
+
 void __init efi_print_memmap(void)
 {
 #ifdef EFI_DEBUG
@@ -232,14 +247,16 @@ void __init efi_print_memmap(void)
 	for (p = memmap.map, i = 0;
 	     p < memmap.map_end;
 	     p += memmap.desc_size, i++) {
-		char buf[64];
+		char buf[64], buf2[32], 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 @ %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)));
+			efi_size_format(buf3, sizeof(buf3),
+				md->num_pages << EFI_PAGE_SHIFT),
+			efi_size_format(buf2, sizeof(buf2), md->phys_addr));
 	}
 #endif  /*  EFI_DEBUG  */
 }
-- 
2.6.4

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

[Andy Shevchenko]

On Tue, 2016-01-12 at 15:13 +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.

Please, comment on the idea, but discard both patches, I'm cooking
better approach.

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/string_helpers.h |  3 +++
>  lib/string_helpers.c           | 21 ++++++++++++---------
>  2 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/string_helpers.h
> b/include/linux/string_helpers.h
> index dabe643..1d16240 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -10,6 +10,9 @@ enum string_size_units {
>  	STRING_UNITS_2,		/* use binary powers of 2^10
> */
>  };
>  
> +extern const char *const string_units_10[];
> +extern const char *const string_units_2[];
> +
>  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..3a4c812 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[] = {
> +	"B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB",
> +};
> +EXPORT_SYMBOL(string_units_10);
> +const char * const string_units_2[] = {
> +	"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 >= ARRAY_SIZE(string_units_2))
>  		unit = "UNK";
>  	else
>  		unit = units_str[units][i];

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

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

[Matt Fleming]

On Thu, 14 Jan, at 11:45:05AM, Andy Shevchenko wrote:
> On Tue, 2016-01-12 at 15:13 +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.
> 
> Please, comment on the idea, but discard both patches, I'm cooking
> better approach.

I like this idea. I'm all for reusing existing code where possible.

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

[Andy Shevchenko]

On Thu, Jan 14, 2016 at 5:44 PM, Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
> On Thu, 14 Jan, at 11:45:05AM, Andy Shevchenko wrote:
>> On Tue, 2016-01-12 at 15:13 +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.
>>
>> Please, comment on the idea, but discard both patches, I'm cooking
>> better approach.
>
> I like this idea. I'm all for reusing existing code where possible.

Just sent v2.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/2] x86/efi: print size and base in binary units in efi_print_memmap

[Andy Shevchenko]

On Tue, Jan 12, 2016 at 3:13 PM, Andy Shevchenko
<andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> From: Robert Elliott <elliott-ZPxbGqLxI0U@public.gmane.org>
>
> Print the base address for each range in decimal alongside the size.
> Use a "(size @ base)" format similar to the fake_memmap kernel parameter.
>
> Print the range and 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 @ 34 GiB)
>
> Signed-off-by: Robert Elliott <elliott-ZPxbGqLxI0U@public.gmane.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Matt, seems another approach is not in favour, which means we rather
stick with this one. But see one comment below.

> ---
>  arch/x86/platform/efi/efi.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index e0846b5..e762969 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>
> @@ -222,6 +224,19 @@ int __init efi_memblock_x86_reserve_range(void)
>         return 0;
>  }
>
> +static char * __init efi_size_format(char *buf, size_t size, u64 bytes)
> +{
> +       unsigned long i;
> +
> +       if (bytes)
> +               i = __ffs64(bytes) / 10;
> +       else
> +               i = 0;
> +
> +       snprintf(buf, size, "%llu%s", bytes >> (i * 10), string_units_2[i]);

Here is missed space, should be: "%llu %s"

> +       return buf;
> +}
> +
>  void __init efi_print_memmap(void)
>  {
>  #ifdef EFI_DEBUG
> @@ -232,14 +247,16 @@ void __init efi_print_memmap(void)
>         for (p = memmap.map, i = 0;
>              p < memmap.map_end;
>              p += memmap.desc_size, i++) {
> -               char buf[64];
> +               char buf[64], buf2[32], 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 @ %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)));
> +                       efi_size_format(buf3, sizeof(buf3),
> +                               md->num_pages << EFI_PAGE_SHIFT),
> +                       efi_size_format(buf2, sizeof(buf2), md->phys_addr));
>         }
>  #endif  /*  EFI_DEBUG  */
>  }
> --
> 2.6.4
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/2] x86/efi: print size and base in binary units in efi_print_memmap

[Matt Fleming]

On Tue, 19 Jan, at 09:32:26AM, Andy Shevchenko wrote:
> On Tue, Jan 12, 2016 at 3:13 PM, Andy Shevchenko
> <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> > From: Robert Elliott <elliott-ZPxbGqLxI0U@public.gmane.org>
> >
> > Print the base address for each range in decimal alongside the size.
> > Use a "(size @ base)" format similar to the fake_memmap kernel parameter.
> >
> > Print the range and 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 @ 34 GiB)
> >
> > Signed-off-by: Robert Elliott <elliott-ZPxbGqLxI0U@public.gmane.org>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> Matt, seems another approach is not in favour, which means we rather
> stick with this one. But see one comment below.
 
Thanks Andy. I like the below patch (with the included space), but as
I said when reviewing Robert's version, I'm really not in favour of
the "@ address" portition of the string.

Andy, could you submit a v2 with the space fix and without the @
<addr> bit?

Re: [PATCH v1 2/2] x86/efi: print size and base in binary units in efi_print_memmap

[Andy Shevchenko]

On Thu, Jan 21, 2016 at 4:05 PM, Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
> On Tue, 19 Jan, at 09:32:26AM, Andy Shevchenko wrote:
>> On Tue, Jan 12, 2016 at 3:13 PM, Andy Shevchenko
>> <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:

…

>> > 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 @ 34 GiB)
>> >
>> > Signed-off-by: Robert Elliott <elliott-ZPxbGqLxI0U@public.gmane.org>
>> > Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>
>> Matt, seems another approach is not in favour, which means we rather
>> stick with this one. But see one comment below.
>
> Thanks Andy. I like the below patch (with the included space), but as
> I said when reviewing Robert's version, I'm really not in favour of
> the "@ address" portition of the string.
>
> Andy, could you submit a v2 with the space fix and without the @
> <addr> bit?

Will do soon.

-- 
With Best Regards,
Andy Shevchenko