From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756004AbbKCWN4 (ORCPT ); Tue, 3 Nov 2015 17:13:56 -0500 Received: from mail-wi0-f170.google.com ([209.85.212.170]:33522 "EHLO mail-wi0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751266AbbKCWNy (ORCPT ); Tue, 3 Nov 2015 17:13:54 -0500 From: Rasmus Villemoes To: James Bottomley Cc: Vitaly Kuznetsov , linux-scsi , "ulf.hansson\@linaro.org" , "andriy.shevchenko\@linux.intel.com" , "keescook\@chromium.org" , "linux-kernel\@vger.kernel.org" , "akpm\@linux-foundation.org" Subject: Re: [PATCH v2] string_helpers: fix precision loss for some inputs Organization: D03 References: <1446582810.6440.36.camel@HansenPartnership.com> <1446585708.6440.47.camel@HansenPartnership.com> X-Hashcash: 1:20:151103:keescook@chromium.org::F8QKUn62AuUTQvvi:00000000000000000000000000000000000000000Qc5 X-Hashcash: 1:20:151103:linux-scsi@vger.kernel.org::xwtFiMbxLFswIL5C:0000000000000000000000000000000000011S9 X-Hashcash: 1:20:151103:james.bottomley@hansenpartnership.com::yGIzxTfNAE16uT5f:0000000000000000000000002MqU X-Hashcash: 1:20:151103:andriy.shevchenko@linux.intel.com::tUGLP+dB/d5Hc4BE:00000000000000000000000000003lSf X-Hashcash: 1:20:151103:linux-kernel@vger.kernel.org::2/IZ7GyK60EHkO9O:0000000000000000000000000000000004adA X-Hashcash: 1:20:151103:vkuznets@redhat.com::VBztLKzr1UUtRtvw:0000000000000000000000000000000000000000007k0a X-Hashcash: 1:20:151103:ulf.hansson@linaro.org::PxTr1hrF98D3lh3+:000000000000000000000000000000000000000A1HP X-Hashcash: 1:20:151103:akpm@linux-foundation.org::M58fKbNMsfQElMjq:0000000000000000000000000000000000009c7l Date: Tue, 03 Nov 2015 23:13:46 +0100 In-Reply-To: <1446585708.6440.47.camel@HansenPartnership.com> (James Bottomley's message of "Tue, 03 Nov 2015 13:21:48 -0800") Message-ID: <87d1vqd945.fsf@rasmusvillemoes.dk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 03 2015, James Bottomley wrote: > From: James Bottomley > > It was noticed that we lose precision in the final calculation for some > inputs. The most egregious example is size=3000 blk_size=1900 in units of 10 > should yield 5.70 MB but in fact yields 3.00 MB (oops). This is because the > current algorithm doesn't correctly account for all the remainders in the > logarithms. Fix this by doing a correct calculation in the remainders based > on napier's algorithm. Additionally, now we have the correct result, we have > to account for arithmetic rounding because we're printing 3 digits of > precision. This means that if the fourth digit is five or greater, we have to > round up, so add a section to ensure correct rounding. Finally account for > all possible inputs correctly, including zero for block size. > > Reported-by: Vitaly Kuznetsov > Cc: stable@vger.kernel.org # delay backport by two months for testing > Fixes: b9f28d863594c429e1df35a0474d2663ca28b307 > Signed-off-by: James Bottomley > > -- > > v2: updated with a recommendation from Rasmus Villemoes to truncate the > initial precision at just under 32 bits > > diff --git a/lib/string_helpers.c b/lib/string_helpers.c > index 5939f63..363faca 100644 > --- a/lib/string_helpers.c > +++ b/lib/string_helpers.c > @@ -43,38 +43,40 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units, > [STRING_UNITS_10] = 1000, > [STRING_UNITS_2] = 1024, > }; > - int i, j; > - u32 remainder = 0, sf_cap, exp; > + static const unsigned int rounding[] = { 500, 50, 5, 0}; j necessarily ends up being 0, 1 or 2. Any reason to include the last entry? > + > + while (blk_size >= UINT_MAX) > i++; > - } > > - exp = divisor[units] / (u32)blk_size; > - /* > - * size must be strictly greater than exp here to ensure that remainder > - * is greater than divisor[units] coming out of the if below. > - */ > - if (size > exp) { > - remainder = do_div(size, divisor[units]); > - remainder *= blk_size; > + while (size >= UINT_MAX) > i++; Please spell it U32_MAX. Also, it's not clear why you left out the do_divs ;-) Rasmus