From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757611AbcAYTpk (ORCPT ); Mon, 25 Jan 2016 14:45:40 -0500 Received: from bedivere.hansenpartnership.com ([66.63.167.143]:55568 "EHLO bedivere.hansenpartnership.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757182AbcAYTpi (ORCPT ); Mon, 25 Jan 2016 14:45:38 -0500 Message-ID: <1453751131.2363.54.camel@HansenPartnership.com> Subject: Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap From: James Bottomley To: Andy Shevchenko Cc: "Elliott, Robert (Persistent Memory)" , Andy Shevchenko , Matt Fleming , Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , "linux-efi@vger.kernel.org" , Rasmus Villemoes , Andrew Morton , "linux-kernel @ vger . kernel . org" Date: Mon, 25 Jan 2016 11:45:31 -0800 In-Reply-To: References: <1453560913-134672-1-git-send-email-andriy.shevchenko@linux.intel.com> <1453560913-134672-4-git-send-email-andriy.shevchenko@linux.intel.com> <1453567445.2470.24.camel@HansenPartnership.com> <94D0CD8314A33A4D9D801C0FE68B40295BF3B840@G9W0745.americas.hpqcorp.net> <1453748172.2363.36.camel@HansenPartnership.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.5 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2016-01-25 at 21:28 +0200, Andy Shevchenko wrote: > On Mon, Jan 25, 2016 at 8:56 PM, James Bottomley > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Subject: Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap Date: Mon, 25 Jan 2016 11:45:31 -0800 Message-ID: <1453751131.2363.54.camel@HansenPartnership.com> References: <1453560913-134672-1-git-send-email-andriy.shevchenko@linux.intel.com> <1453560913-134672-4-git-send-email-andriy.shevchenko@linux.intel.com> <1453567445.2470.24.camel@HansenPartnership.com> <94D0CD8314A33A4D9D801C0FE68B40295BF3B840@G9W0745.americas.hpqcorp.net> <1453748172.2363.36.camel@HansenPartnership.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-efi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Andy Shevchenko Cc: "Elliott, Robert (Persistent Memory)" , Andy Shevchenko , Matt Fleming , Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , "linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Rasmus Villemoes , Andrew Morton , "linux-kernel @ vger . kernel . org" List-Id: linux-efi@vger.kernel.org On Mon, 2016-01-25 at 21:28 +0200, Andy Shevchenko wrote: > On Mon, Jan 25, 2016 at 8:56 PM, James Bottomley > 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