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

Groups > linux.kernel > #1311977 > unrolled thread

Re: [PATCH v1 2/2] x86/efi: print size and base 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

  Re: [PATCH v1 2/2] x86/efi: print size and base in binary units in efi_print_memmap Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-01-19 08:40 +0100
    Re: [PATCH v1 2/2] x86/efi: print size and base in binary units in  efi_print_memmap Matt Fleming <matt@codeblueprint.co.uk> - 2016-01-21 15:10 +0100
      Re: [PATCH v1 2/2] x86/efi: print size and base in binary units in efi_print_memmap Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-01-21 15:30 +0100

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

On Tue, Jan 12, 2016 at 3:13 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> From: Robert Elliott <elliott@hpe.com>
>
> 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@hpe.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

[toc] | [next] | [standalone]

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

On Tue, 19 Jan, at 09:32:26AM, Andy Shevchenko wrote:
> On Tue, Jan 12, 2016 at 3:13 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > From: Robert Elliott <elliott@hpe.com>
> >
> > 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@hpe.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> 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?

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

#1314224

On Thu, Jan 21, 2016 at 4:05 PM, Matt Fleming <matt@codeblueprint.co.uk> 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@linux.intel.com> 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@hpe.com>
>> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>
>> 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web