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

Groups > linux.kernel > #1315630 > unrolled thread

[PATCH v3 0/4] x86/efi: use binary units when printing

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/4] x86/efi: use binary units when printing Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-23 16:00 +0100
    [PATCH v3 2/4] lib/string_helpers: fix indentation in few places Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-23 16:00 +0100
    [PATCH v3 4/4] x86/efi: Use proper units in efi_find_mirror() Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-23 16:00 +0100
    [PATCH v3 1/4] lib/string_helpers: export string_units_{2,10} for others Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-23 16:00 +0100
      Re: [PATCH v3 1/4] lib/string_helpers: export string_units_{2,10}  for others James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-01-23 17:20 +0100
        Re: [PATCH v3 1/4] lib/string_helpers: export string_units_{2,10} for others Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-01-23 18:00 +0100
    Re: [PATCH v3 0/4] x86/efi: use binary units when printing James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-01-23 17:40 +0100
      Re: [PATCH v3 0/4] x86/efi: use binary units when printing Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-01-23 18:30 +0100

#1315630 — [PATCH v3 0/4] x86/efi: use binary units when printing

The patch series exports the arrays of binary and decimal units as it's
described by IEC.

First user of it is EFI code which would print sizes and other values using
binary prefix.

James, is this now okay to you?

Matt, I suppose we need to update the stuff in your tree.

Since v2:
- address James comment (don't nail array size)
- fix a title and commit message for patch 3 to be in align with the change

Andy Shevchenko (3):
  lib/string_helpers: export string_units_{2,10} for others
  lib/string_helpers: fix indentation in few places
  x86/efi: Use proper units in efi_find_mirror()

Robert Elliott (1):
  x86/efi: print size in binary units in efi_print_memmap

 arch/x86/platform/efi/efi.c    | 27 ++++++++++++++++++---------
 include/linux/string_helpers.h |  3 +++
 lib/string_helpers.c           | 26 ++++++++++++++------------
 3 files changed, 35 insertions(+), 21 deletions(-)

-- 
2.7.0.rc3

[toc] | [next] | [standalone]

#1315632 — [PATCH v3 2/4] lib/string_helpers: fix indentation in few places

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

There is no functional change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 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 86124c9..2ebd724 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 >= ARRAY_SIZE(string_units_2))
 		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

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

#1315633 — [PATCH v3 4/4] x86/efi: Use proper units in efi_find_mirror()

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

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

#1315641 — [PATCH v3 1/4] lib/string_helpers: export string_units_{2,10} for others

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 |  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..86124c9 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.7.0.rc3

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

#1315677 — Re: [PATCH v3 1/4] lib/string_helpers: export string_units_{2,10} for others

On Sat, 2016-01-23 at 16:55 +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.
> 
> 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..86124c9 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))

so now, no-one other than string_helpers.c can tell the size of the
array ... I don't think that's an improvement.  Also for a trivial
patch I'm starting to think there should be a three strikes rule: we
get a large number of bugs from allegedly trivial reworks which
wouldn't have happened if we'd retained the original working code in
the first place.

After two attempts, doesn't it perhaps strike you that a helper
function rather than a direct export would get over this difficulty? 
 It might also address the precision problem you introduced.

James

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

#1315698 — Re: [PATCH v3 1/4] lib/string_helpers: export string_units_{2,10} for others

On Sat, Jan 23, 2016 at 6:14 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:

> After two attempts, doesn't it perhaps strike you that a helper
> function rather than a direct export would get over this difficulty?

Yes, it would.

>  It might also address the precision problem you introduced.

You mean the issue with a unit range in the other cases (non-efi)?

-- 
With Best Regards,
Andy Shevchenko

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

#1315688

On Sat, 2016-01-23 at 16:55 +0200, Andy Shevchenko wrote:
> The patch series exports the arrays of binary and decimal units as
> it's
> described by IEC.
> 
> First user of it is EFI code which would print sizes and other values
> using
> binary prefix.
> 
> James, is this now okay to you?

Patch 2 is still pointless. I happen to like the <tab><space>label:
convention.  I realise others don't but an entire patch simply to
change my convention to your convention is a bit overkill.

I'll comment on the rest in the patches.

James

> Matt, I suppose we need to update the stuff in your tree.
> 
> Since v2:
> - address James comment (don't nail array size)
> - fix a title and commit message for patch 3 to be in align with the
> change
> 
> Andy Shevchenko (3):
>   lib/string_helpers: export string_units_{2,10} for others
>   lib/string_helpers: fix indentation in few places
>   x86/efi: Use proper units in efi_find_mirror()
> 
> Robert Elliott (1):
>   x86/efi: print size in binary units in efi_print_memmap
> 
>  arch/x86/platform/efi/efi.c    | 27 ++++++++++++++++++---------
>  include/linux/string_helpers.h |  3 +++
>  lib/string_helpers.c           | 26 ++++++++++++++------------
>  3 files changed, 35 insertions(+), 21 deletions(-)
> 

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

#1315706

On Sat, Jan 23, 2016 at 6:34 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Sat, 2016-01-23 at 16:55 +0200, Andy Shevchenko wrote:
>> The patch series exports the arrays of binary and decimal units as
>> it's
>> described by IEC.
>>
>> First user of it is EFI code which would print sizes and other values
>> using
>> binary prefix.
>>
>> James, is this now okay to you?
>
> Patch 2 is still pointless. I happen to like the <tab><space>label:
> convention.

Even there it's just <space>label:

>  I realise others don't but an entire patch simply to
> change my convention to your convention is a bit overkill.

Main point of that patch is to bring snprintf() to one line. Is it
also your style?

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web