* [PATCH] lib/string_helpers: fix string_get_size() unit promotion after rounding
@ 2026-04-07 14:33 Shuvam Pandey
2026-04-07 14:53 ` Andy Shevchenko
2026-04-19 15:31 ` [PATCH v2] lib/string_helpers: renormalize string_get_size() after rounding carry Shuvam Pandey
0 siblings, 2 replies; 11+ messages in thread
From: Shuvam Pandey @ 2026-04-07 14:33 UTC (permalink / raw)
To: Andrew Morton, Kees Cook, Andy Shevchenko; +Cc: linux-kernel, linux-hardening
string_get_size() rounds the fractional part before formatting the
result. If that carry pushes the integer part to the unit divisor, the
value is still printed in the old unit.
This yields outputs like "1000 kB" for 999500 bytes and "1024 KiB" for
1048064 bytes instead of promoting them to the next unit.
Renormalize the value after the carry so it is promoted before
formatting. Add KUnit coverage for the decimal and binary boundary
cases.
Fixes: 3c9f3681d0b4 ("[SCSI] lib: add generic helper to print sizes rounded to the correct SI range")
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
lib/string_helpers.c | 7 +++++++
lib/tests/string_helpers_kunit.c | 4 ++++
2 files changed, 11 insertions(+)
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 169eaf583494..aa1ba47e28bd 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -121,6 +121,13 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
size += 1;
}
+ if (size >= divisor[units_base]) {
+ size = 1;
+ remainder = 0;
+ i++;
+ j = 2;
+ }
+
if (j) {
snprintf(tmp, sizeof(tmp), ".%03u", remainder);
tmp[j+1] = '\0';
diff --git a/lib/tests/string_helpers_kunit.c b/lib/tests/string_helpers_kunit.c
index c853046183d2..16118fb16c9a 100644
--- a/lib/tests/string_helpers_kunit.c
+++ b/lib/tests/string_helpers_kunit.c
@@ -558,6 +558,10 @@ static void test_get_size(struct kunit *test)
/* weird block sizes */
test_string_get_size_one(3000, 1900, "5.70 MB", "5.44 MiB");
+ /* rounding carry into the next unit */
+ test_string_get_size_one(999500, 1, "1.00 MB", "976 KiB");
+ test_string_get_size_one(1048064, 1, "1.05 MB", "1.00 MiB");
+
/* huge values */
test_string_get_size_one(U64_MAX, 4096, "75.6 ZB", "64.0 ZiB");
test_string_get_size_one(4096, U64_MAX, "75.6 ZB", "64.0 ZiB");
--
2.50.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] lib/string_helpers: fix string_get_size() unit promotion after rounding
2026-04-07 14:33 [PATCH] lib/string_helpers: fix string_get_size() unit promotion after rounding Shuvam Pandey
@ 2026-04-07 14:53 ` Andy Shevchenko
[not found] ` <CANAAWH+ihOpBVB5_Q4gxLs0_zO=pHb-kF53UXE-pqg1U9E9OKw@mail.gmail.com>
2026-04-19 15:31 ` [PATCH v2] lib/string_helpers: renormalize string_get_size() after rounding carry Shuvam Pandey
1 sibling, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2026-04-07 14:53 UTC (permalink / raw)
To: Shuvam Pandey
Cc: Andrew Morton, Kees Cook, Andy Shevchenko, linux-kernel,
linux-hardening
On Tue, Apr 07, 2026 at 08:18:06PM +0545, Shuvam Pandey wrote:
> string_get_size() rounds the fractional part before formatting the
> result. If that carry pushes the integer part to the unit divisor, the
> value is still printed in the old unit.
>
> This yields outputs like "1000 kB" for 999500 bytes and "1024 KiB" for
> 1048064 bytes instead of promoting them to the next unit.
What does it print for 1048575, 1048576, 1048577 respectively?
(before and after your patch)
All the same for numbers around 2MiB.
> Renormalize the value after the carry so it is promoted before
> formatting. Add KUnit coverage for the decimal and binary boundary
> cases.
...
> --- a/lib/tests/string_helpers_kunit.c
> +++ b/lib/tests/string_helpers_kunit.c
Thumb up for the test cases!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] lib/string_helpers: fix string_get_size() unit promotion after rounding
[not found] ` <CANAAWHKZXbFAr7=DRCrS46GDsWi88Q=v883RQrgF4+EScksP=w@mail.gmail.com>
@ 2026-04-14 7:19 ` Andy Shevchenko
2026-04-14 8:06 ` Shuvam Pandey
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2026-04-14 7:19 UTC (permalink / raw)
To: Shuvam Pandey; +Cc: Andrew Morton, Kees Cook, linux-kernel, linux-hardening
First of all, do not top-post!
Second, I just realised now that the discussion was done in private emails, do
not do this. Please, answer again with all what was discussed to the public
mailing lists as this message does.
On Fri, Apr 10, 2026 at 09:11:59PM +0545, Shuvam Pandey wrote:
> I think the open question is whether string_get_size() is intended to
> round across a unit boundary at all.
> There seem to be two possible interpretations:
>
> 1. Keep the current 3-significant figure rounding, and if that rounding
> carries to the unit divisor, renormalize into the next unit. That is
> what my patch does.
>
> 2. Treat the unit boundary as hard, so values below 1.00 MiB / 1.00 MB
> stay below that threshold and are not rounded across it.
>
> If (2) is the intended behavior, then this is broader than the
> carry-renormalization case and my patch is not the right fix.
>
> Which behavior should string_get_size() implement here, or am I
> missing a third intended behavior?
>
> On Thu, Apr 9, 2026 at 1:55 AM Shuvam Pandey <shuvampandey1@gmail.com>
> wrote:
>
> > I checked the additional base 10 values as well.
> >
> > For blk_size = 1, the current code gives:
> >
> > 999999:
> >
> > STRING_UNITS_10: before 1000 kB, after 1.00 MB
> >
> > 1000000:
> >
> > STRING_UNITS_10: before 1.00 MB, after 1.00 MB
> >
> > 1000001:
> >
> > STRING_UNITS_10: before 1.00 MB, after 1.00 MB
> >
> > 1048575:
> >
> > STRING_UNITS_2: before 1024 KiB, after 1.00 MiB
> >
> > 1048576:
> >
> > STRING_UNITS_2: before 1.00 MiB, after 1.00 MiB
> >
> > 1048577:
> >
> > STRING_UNITS_2: before 1.00 MiB, after 1.00 MiB
> >
> > So this patch only changes the carry-to-divisor cases, where the current
> > code rounds the value up but still prints it in the old unit.
> >
> > That said, I think your point about the broader semantics is valid.
> >
> > string_get_size() has long rounded to 3 significant figures, and my
> > change only makes that existing behavior consistent after a carry. If
> > intended behavior is instead that values like 1048575 should stay
> > below 1.00 MiB, then the real issue is not just the missing promotion
> > here, but the rounding policy itself.
> >
> > So I think the patch is correct for the current implementation, but it
> > may not be the right fix if the intended behavior near unit boundaries
> > is supposed to be different.
> >
> > I'll take another look at the expected behavior around these boundary
> > cases and follow up once I've re-evaluated that.
> >
> > On Thu, Apr 9, 2026 at 1:38 AM Andy Shevchenko <
> > andriy.shevchenko@intel.com> wrote:
> >
> >> On Tue, Apr 07, 2026 at 08:48:59PM +0545, Shuvam Pandey wrote:
> >> > For blk_size = 1:
> >> >
> >> > 1048575:
> >> > STRING_UNITS_10: before 1.05 MB, after 1.05 MB
> >> > STRING_UNITS_2: before 1024 KiB, after 1.00 MiB
> >> >
> >> > 1048576:
> >> > STRING_UNITS_10: before 1.05 MB, after 1.05 MB
> >> > STRING_UNITS_2: before 1.00 MiB, after 1.00 MiB
> >> >
> >> > 1048577:
> >> > STRING_UNITS_10: before 1.05 MB, after 1.05 MB
> >> > STRING_UNITS_2: before 1.00 MiB, after 1.00 MiB
> >> >
> >> > Likewise around 2 MiB:
> >> >
> >> > 2097151:
> >> > STRING_UNITS_10: before 2.10 MB, after 2.10 MB
> >> > STRING_UNITS_2: before 2.00 MiB, after 2.00 MiB
> >> >
> >> > 2097152:
> >> > STRING_UNITS_10: before 2.10 MB, after 2.10 MB
> >> > STRING_UNITS_2: before 2.00 MiB, after 2.00 MiB
> >> >
> >> > 2097153:
> >> > STRING_UNITS_10: before 2.10 MB, after 2.10 MB
> >> > STRING_UNITS_2: before 2.00 MiB, after 2.00 MiB
> >> >
> >> > So the change only affects the carry-to-divisor case itself; values that
> >> > are already in the next unit stay unchanged.
> >> >
> >> > I also rechecked the neighboring value 1048574, and it changes from 1024
> >> > KiB to 1.00 MiB as well. I can add coverage for 1048574..1048577 in v2
> >> if
> >> > you think that would be useful.
> >>
> >> Also with base 10 for 999999 1000000 1000001...
> >>
> >> I'm trying to understand the current behaviour better and the logic of
> >> your
> >> change. But looking at the above I think your patch doesn't improve the
> >> case.
> >> And might even be another bug in the current code. I mean, do we have to
> >> round
> >> the numbers? 1048575 and lower are not equal 1.00 MiB strictly speaking.
> >> It's rather 0.999 MiB.
> >>
> >> > On Tue, Apr 7, 2026 at 8:38 PM Andy Shevchenko <
> >> andriy.shevchenko@intel.com>
> >> > wrote:
> >> >
> >> > > On Tue, Apr 07, 2026 at 08:18:06PM +0545, Shuvam Pandey wrote:
> >> > > > string_get_size() rounds the fractional part before formatting the
> >> > > > result. If that carry pushes the integer part to the unit divisor,
> >> the
> >> > > > value is still printed in the old unit.
> >> > > >
> >> > > > This yields outputs like "1000 kB" for 999500 bytes and "1024 KiB"
> >> for
> >> > > > 1048064 bytes instead of promoting them to the next unit.
> >> > >
> >> > > What does it print for 1048575, 1048576, 1048577 respectively?
> >> > > (before and after your patch)
> >> > >
> >> > > All the same for numbers around 2MiB.
> >> > >
> >> > > > Renormalize the value after the carry so it is promoted before
> >> > > > formatting. Add KUnit coverage for the decimal and binary boundary
> >> > > > cases.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] lib/string_helpers: fix string_get_size() unit promotion after rounding
2026-04-14 7:19 ` Andy Shevchenko
@ 2026-04-14 8:06 ` Shuvam Pandey
0 siblings, 0 replies; 11+ messages in thread
From: Shuvam Pandey @ 2026-04-14 8:06 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Andrew Morton, Kees Cook, linux-kernel, linux-hardening
Hi Andy,
Sorry, I should have kept the discussion on-list.
This is about the string_get_size() patch for outputs like "1000 kB" and
"1024 KiB", i.e. cases where the current code has already rounded to the
unit divisor but still prints the old unit.
> I'm trying to understand the current behaviour better and the logic of your
> change. But looking at the above I think your patch doesn't improve the case.
> And might even be another bug in the current code. I mean, do we have to round
> the numbers? 1048575 and lower are not equal 1.00 MiB strictly speaking.
> It's rather 0.999 MiB.
I took another look at the code and the old versions. I think the issue
here is narrower than whether values should cross the boundary at all.
For the base-10 values you asked about earlier, and for the binary
boundary case, the current code already rounds across the boundary.
With blk_size = 1, it prints:
999999 -> 1000 kB
1000000 -> 1.00 MB
1000001 -> 1.00 MB
and
1048575 -> 1024 KiB
1048576 -> 1.00 MiB
1048577 -> 1.00 MiB
So the patch does not introduce new cross-boundary rounding. It only changes
how those already-rounded results are displayed: it renormalizes 1000 kB to
1.00 MB and 1024 KiB to 1.00 MiB.
Around the first MB/MiB boundary with blk_size = 1, the display changes for:
STRING_UNITS_10: 999500 .. 999999
STRING_UNITS_2: 1048064 .. 1048575
with the adjacent values staying unchanged, e.g.:
999499 -> 999 kB
1000000 -> 1.00 MB
1048063 -> 1023 KiB
1048576 -> 1.00 MiB
That behavior comes from the arithmetic-rounding logic added by
564b026fbd0d ("string_helpers: fix precision loss for some inputs").
Before that change, 999999 printed 999 kB and 1048575 printed 1023 KiB,
so my original Fixes tag was wrong.
The v2 will carry:
Fixes: 564b026fbd0d ("string_helpers: fix precision loss for some inputs")
If the intended policy is instead that values below 1.00 MB / MiB must
never round across the unit boundary, then I agree this is broader than a
renormalization bug and the rounding policy itself would need changing.
But with the current documented 3 significant figures behavior, leaving
the result as 1000 kB / 1024 KiB is inconsistent with that formatting,
because the existing rounding has already carried the value to the next
unit boundary.
Unless there is a reason to take a different approach, I'll send a v2 with
the corrected Fixes tag, an updated changelog framing this as post-rounding
renormalization, and threshold tests around the affected ranges.
Thanks,
Shuvam
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] lib/string_helpers: renormalize string_get_size() after rounding carry
2026-04-07 14:33 [PATCH] lib/string_helpers: fix string_get_size() unit promotion after rounding Shuvam Pandey
2026-04-07 14:53 ` Andy Shevchenko
@ 2026-04-19 15:31 ` Shuvam Pandey
2026-04-20 6:56 ` Andy Shevchenko
1 sibling, 1 reply; 11+ messages in thread
From: Shuvam Pandey @ 2026-04-19 15:31 UTC (permalink / raw)
To: Kees Cook, Andrew Morton
Cc: Andy Shevchenko, linux-hardening, linux-kernel, Shuvam Pandey
string_get_size() rounds the fractional part before formatting the
result. If that carry pushes the integer part to the unit divisor, the
helper still prints the old unit and produces strings like "1000 kB"
and "1024 KiB".
That misformatted carry case comes from the arithmetic rounding added by
commit 564b026fbd0d ("string_helpers: fix precision loss for some
inputs"). The helper already rounds those values up numerically; it
just fails to renormalize them into the next unit before formatting.
Renormalize the carried result into the next named unit so those cases
print as "1.00 MB" and "1.00 MiB" instead. Only do that when another
named unit exists, so the existing "UNK" fallback is preserved at the
top end. Extend the KUnit coverage around the first decimal and binary
boundaries, including adjacent values and non-byte block sizes that hit
the same carry path.
Fixes: 564b026fbd0d ("string_helpers: fix precision loss for some inputs")
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
Changes in v2:
- correct the Fixes tag to 564b026fbd0d
- renormalize only when another named unit exists
- preserve the top-end UNK fallback
- add decimal and binary boundary/neighbor KUnit coverage
- add non-byte block-size cases that hit the same carry path
lib/string_helpers.c | 12 ++++++++++++
lib/tests/string_helpers_kunit.c | 25 +++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 169eaf5834949..bcabb24fe0dbf 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -121,6 +121,18 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
size += 1;
}
+ /*
+ * Renormalize into the next named unit, but preserve the top-end
+ * UNK fallback. After promotion the value is exactly 1 of the next
+ * unit, so keep two fractional digits for the usual 1.00 formatting.
+ */
+ if (size >= divisor[units_base] && i + 1 < ARRAY_SIZE(units_2)) {
+ size = 1;
+ remainder = 0;
+ i++;
+ j = 2;
+ }
+
if (j) {
snprintf(tmp, sizeof(tmp), ".%03u", remainder);
tmp[j+1] = '\0';
diff --git a/lib/tests/string_helpers_kunit.c b/lib/tests/string_helpers_kunit.c
index c853046183d24..67822cb48fd15 100644
--- a/lib/tests/string_helpers_kunit.c
+++ b/lib/tests/string_helpers_kunit.c
@@ -558,6 +558,31 @@ static void test_get_size(struct kunit *test)
/* weird block sizes */
test_string_get_size_one(3000, 1900, "5.70 MB", "5.44 MiB");
+ /* rounding carry into the next unit at the first decimal boundary */
+ test_string_get_size_one(999499, 1, "999 kB", "976 KiB");
+ test_string_get_size_one(999500, 1, "1.00 MB", "976 KiB");
+ test_string_get_size_one(999999, 1, "1.00 MB", "977 KiB");
+ test_string_get_size_one(1000000, 1, "1.00 MB", "977 KiB");
+ test_string_get_size_one(1000001, 1, "1.00 MB", "977 KiB");
+
+ /* rounding carry into the next unit at the first binary boundary */
+ test_string_get_size_one(1048063, 1, "1.05 MB", "1023 KiB");
+ test_string_get_size_one(1048064, 1, "1.05 MB", "1.00 MiB");
+ test_string_get_size_one(1048575, 1, "1.05 MB", "1.00 MiB");
+ test_string_get_size_one(1048576, 1, "1.05 MB", "1.00 MiB");
+ test_string_get_size_one(1048577, 1, "1.05 MB", "1.00 MiB");
+
+ /* values already in the next binary unit stay unchanged */
+ test_string_get_size_one(2097151, 1, "2.10 MB", "2.00 MiB");
+ test_string_get_size_one(2097152, 1, "2.10 MB", "2.00 MiB");
+ test_string_get_size_one(2097153, 1, "2.10 MB", "2.00 MiB");
+
+ /* non-byte block sizes hit the same carry path */
+ test_string_get_size_one(4997, 200, "999 kB", "976 KiB");
+ test_string_get_size_one(4998, 200, "1.00 MB", "976 KiB");
+ test_string_get_size_one(9981, 105, "1.05 MB", "1023 KiB");
+ test_string_get_size_one(9982, 105, "1.05 MB", "1.00 MiB");
+
/* huge values */
test_string_get_size_one(U64_MAX, 4096, "75.6 ZB", "64.0 ZiB");
test_string_get_size_one(4096, U64_MAX, "75.6 ZB", "64.0 ZiB");
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] lib/string_helpers: renormalize string_get_size() after rounding carry
2026-04-19 15:31 ` [PATCH v2] lib/string_helpers: renormalize string_get_size() after rounding carry Shuvam Pandey
@ 2026-04-20 6:56 ` Andy Shevchenko
2026-04-21 3:04 ` James Bottomley
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2026-04-20 6:56 UTC (permalink / raw)
To: Shuvam Pandey, James Bottomley
Cc: Kees Cook, Andrew Morton, Andy Shevchenko, linux-hardening,
linux-kernel
Please, keep the author of the original change in the loop.
James, what do you think?
On Sun, Apr 19, 2026 at 6:31 PM Shuvam Pandey <shuvampandey1@gmail.com> wrote:
>
> string_get_size() rounds the fractional part before formatting the
> result. If that carry pushes the integer part to the unit divisor, the
> helper still prints the old unit and produces strings like "1000 kB"
> and "1024 KiB".
>
> That misformatted carry case comes from the arithmetic rounding added by
> commit 564b026fbd0d ("string_helpers: fix precision loss for some
> inputs"). The helper already rounds those values up numerically; it
> just fails to renormalize them into the next unit before formatting.
>
> Renormalize the carried result into the next named unit so those cases
> print as "1.00 MB" and "1.00 MiB" instead. Only do that when another
> named unit exists, so the existing "UNK" fallback is preserved at the
> top end. Extend the KUnit coverage around the first decimal and binary
> boundaries, including adjacent values and non-byte block sizes that hit
> the same carry path.
>
> Fixes: 564b026fbd0d ("string_helpers: fix precision loss for some inputs")
> Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
> ---
> Changes in v2:
> - correct the Fixes tag to 564b026fbd0d
> - renormalize only when another named unit exists
> - preserve the top-end UNK fallback
> - add decimal and binary boundary/neighbor KUnit coverage
> - add non-byte block-size cases that hit the same carry path
>
> lib/string_helpers.c | 12 ++++++++++++
> lib/tests/string_helpers_kunit.c | 25 +++++++++++++++++++++++++
> 2 files changed, 37 insertions(+)
>
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 169eaf5834949..bcabb24fe0dbf 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -121,6 +121,18 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
> size += 1;
> }
>
> + /*
> + * Renormalize into the next named unit, but preserve the top-end
> + * UNK fallback. After promotion the value is exactly 1 of the next
> + * unit, so keep two fractional digits for the usual 1.00 formatting.
> + */
> + if (size >= divisor[units_base] && i + 1 < ARRAY_SIZE(units_2)) {
> + size = 1;
> + remainder = 0;
> + i++;
> + j = 2;
> + }
> +
> if (j) {
> snprintf(tmp, sizeof(tmp), ".%03u", remainder);
> tmp[j+1] = '\0';
> diff --git a/lib/tests/string_helpers_kunit.c b/lib/tests/string_helpers_kunit.c
> index c853046183d24..67822cb48fd15 100644
> --- a/lib/tests/string_helpers_kunit.c
> +++ b/lib/tests/string_helpers_kunit.c
> @@ -558,6 +558,31 @@ static void test_get_size(struct kunit *test)
> /* weird block sizes */
> test_string_get_size_one(3000, 1900, "5.70 MB", "5.44 MiB");
>
> + /* rounding carry into the next unit at the first decimal boundary */
> + test_string_get_size_one(999499, 1, "999 kB", "976 KiB");
> + test_string_get_size_one(999500, 1, "1.00 MB", "976 KiB");
> + test_string_get_size_one(999999, 1, "1.00 MB", "977 KiB");
> + test_string_get_size_one(1000000, 1, "1.00 MB", "977 KiB");
> + test_string_get_size_one(1000001, 1, "1.00 MB", "977 KiB");
> +
> + /* rounding carry into the next unit at the first binary boundary */
> + test_string_get_size_one(1048063, 1, "1.05 MB", "1023 KiB");
> + test_string_get_size_one(1048064, 1, "1.05 MB", "1.00 MiB");
> + test_string_get_size_one(1048575, 1, "1.05 MB", "1.00 MiB");
> + test_string_get_size_one(1048576, 1, "1.05 MB", "1.00 MiB");
> + test_string_get_size_one(1048577, 1, "1.05 MB", "1.00 MiB");
> +
> + /* values already in the next binary unit stay unchanged */
> + test_string_get_size_one(2097151, 1, "2.10 MB", "2.00 MiB");
> + test_string_get_size_one(2097152, 1, "2.10 MB", "2.00 MiB");
> + test_string_get_size_one(2097153, 1, "2.10 MB", "2.00 MiB");
> +
> + /* non-byte block sizes hit the same carry path */
> + test_string_get_size_one(4997, 200, "999 kB", "976 KiB");
> + test_string_get_size_one(4998, 200, "1.00 MB", "976 KiB");
> + test_string_get_size_one(9981, 105, "1.05 MB", "1023 KiB");
> + test_string_get_size_one(9982, 105, "1.05 MB", "1.00 MiB");
> +
> /* huge values */
> test_string_get_size_one(U64_MAX, 4096, "75.6 ZB", "64.0 ZiB");
> test_string_get_size_one(4096, U64_MAX, "75.6 ZB", "64.0 ZiB");
> --
> 2.50.1 (Apple Git-155)
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] lib/string_helpers: renormalize string_get_size() after rounding carry
2026-04-20 6:56 ` Andy Shevchenko
@ 2026-04-21 3:04 ` James Bottomley
2026-04-21 3:38 ` Shuvam Pandey
0 siblings, 1 reply; 11+ messages in thread
From: James Bottomley @ 2026-04-21 3:04 UTC (permalink / raw)
To: Andy Shevchenko, Shuvam Pandey
Cc: Kees Cook, Andrew Morton, Andy Shevchenko, linux-hardening,
linux-kernel
On Mon, 2026-04-20 at 09:56 +0300, Andy Shevchenko wrote:
> Please, keep the author of the original change in the loop.
> James, what do you think?
Well firstly I would note that the original behaviour isn't a bug. It
works that way because of the binary case. So if you feed in 1023 in
for STRING_UNITS_2 it will return 1023 B. If we're going to allow that
slop for binary, it didn't seem reasonable to be precious about the
over by 1 of decimal.
However, if the desire is for this only ever to print out three
significant figures (is that the desire? because I don't really get it
from the changelog) then this:
> > + if (size >= divisor[units_base] && i + 1 <
> > ARRAY_SIZE(units_2)) {
> > + size = 1;
> > + remainder = 0;
> > + i++;
> > + j = 2;
Isn't right. Size is base_10 here, so it should always be compared
against 1000 and, for the STRING_UNITS_2 case, the lower two digits
need to be carried over to the remainder (with roundup). I'd also drop
the comparison against ARRAY_SIZE because you'd still violate the 3sf
rule even if the unit is UNK.
Regards,
James
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] lib/string_helpers: renormalize string_get_size() after rounding carry
2026-04-21 3:04 ` James Bottomley
@ 2026-04-21 3:38 ` Shuvam Pandey
2026-04-21 4:53 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Shuvam Pandey @ 2026-04-21 3:38 UTC (permalink / raw)
To: James Bottomley
Cc: Andy Shevchenko, Kees Cook, Andrew Morton, Andy Shevchenko,
linux-hardening, linux-kernel
Hi James,
> Well firstly I would note that the original behaviour isn't a bug.
> ...
> However, if the desire is for this only ever to print out three
> significant figures ...
> Isn't right.
Thanks, that clarifies it.
I was looking at the 1000 kB / 1024 KiB cases as a narrow formatting
issue after the rounding carry, but that is too narrow a reading. As
you point out, the current helper is not based on a strict always 3
significant figures rule, and my patch only adjusts one carry case
rather than addressing the broader behaviour.
So I agree this should not go forward as a bug fix in its current form.
I'll drop this patch here and revisit it only if I can come back with a
better understanding of the intended semantics.
Thanks,
Shuvam
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] lib/string_helpers: renormalize string_get_size() after rounding carry
2026-04-21 3:38 ` Shuvam Pandey
@ 2026-04-21 4:53 ` Andy Shevchenko
2026-04-21 10:10 ` Shuvam Pandey
2026-04-21 12:16 ` James Bottomley
0 siblings, 2 replies; 11+ messages in thread
From: Andy Shevchenko @ 2026-04-21 4:53 UTC (permalink / raw)
To: Shuvam Pandey
Cc: James Bottomley, Kees Cook, Andrew Morton, Andy Shevchenko,
linux-hardening, linux-kernel
On Tue, Apr 21, 2026 at 6:38 AM Shuvam Pandey <shuvampandey1@gmail.com> wrote:
> > Well firstly I would note that the original behaviour isn't a bug.
James, thank you for the prompt reply!
> So I agree this should not go forward as a bug fix in its current form.
> I'll drop this patch here and revisit it only if I can come back with a
> better understanding of the intended semantics.
Shuvam, perhaps we need to add a few more test cases first to
understand current behaviour better?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] lib/string_helpers: renormalize string_get_size() after rounding carry
2026-04-21 4:53 ` Andy Shevchenko
@ 2026-04-21 10:10 ` Shuvam Pandey
2026-04-21 12:16 ` James Bottomley
1 sibling, 0 replies; 11+ messages in thread
From: Shuvam Pandey @ 2026-04-21 10:10 UTC (permalink / raw)
To: Andy Shevchenko
Cc: James Bottomley, Kees Cook, Andrew Morton, Andy Shevchenko,
linux-hardening, linux-kernel
On Tue, Apr 21, 2026 at 10:39 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> Shuvam, perhaps we need to add a few more test cases first to
> understand current behaviour better?
Yes, that makes sense.
I'll put together a separate test-only patch covering the boundary
cases first, so the current behaviour is explicit before proposing any
behavioural change.
Thanks,
Shuvam
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] lib/string_helpers: renormalize string_get_size() after rounding carry
2026-04-21 4:53 ` Andy Shevchenko
2026-04-21 10:10 ` Shuvam Pandey
@ 2026-04-21 12:16 ` James Bottomley
1 sibling, 0 replies; 11+ messages in thread
From: James Bottomley @ 2026-04-21 12:16 UTC (permalink / raw)
To: Andy Shevchenko, Shuvam Pandey
Cc: Kees Cook, Andrew Morton, Andy Shevchenko, linux-hardening,
linux-kernel
On Tue, 2026-04-21 at 07:53 +0300, Andy Shevchenko wrote:
> On Tue, Apr 21, 2026 at 6:38 AM Shuvam Pandey
> <shuvampandey1@gmail.com> wrote:
>
> > > Well firstly I would note that the original behaviour isn't a
> > > bug.
>
> James, thank you for the prompt reply!
>
> > So I agree this should not go forward as a bug fix in its current
> > form. I'll drop this patch here and revisit it only if I can come
> > back with a better understanding of the intended semantics.
>
> Shuvam, perhaps we need to add a few more test cases first to
> understand current behaviour better?
Sure, but the base 2 one is the problem. It will go up to 1023 in
whatever units but for binary that's under 1 of the next unit up. So
1023 B == 1.00 KiB
1018 B == 0.99 KiB
1000 B == 0.98 KiB
And so on. However, technically the latter two (when converted to the
upper unit) are only 2sf not 3 because the leading zero doesn't count.
If you run the above to 3sf it comes out
1023 B == 0.999 KiB
1018 B == 0.994 KiB
1000 B == 0.977 KiB
If the desire is really to be 3sf everywhere, you need the latter and
the routine can be adjusted to do that: the while around the do_div
would have to be checking strictly >= 1000 and you now have to set j=0
if size comes out 0. Rounding also has to be done inside that loop now
(because too much precision is lost in the j = 3 case).
But, like I said, this isn't a bug fix it's a behaviour change.
Regards,
James
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-04-21 12:16 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-07 14:33 [PATCH] lib/string_helpers: fix string_get_size() unit promotion after rounding Shuvam Pandey
2026-04-07 14:53 ` Andy Shevchenko
[not found] ` <CANAAWH+ihOpBVB5_Q4gxLs0_zO=pHb-kF53UXE-pqg1U9E9OKw@mail.gmail.com>
[not found] ` <adayT_7xAtOLiVlq@ashevche-desk.local>
[not found] ` <CANAAWHKDBoi1x=MpAyU5bQ88=ZOOrkaJNziyhcK9Ei7c8oNSHg@mail.gmail.com>
[not found] ` <CANAAWHKZXbFAr7=DRCrS46GDsWi88Q=v883RQrgF4+EScksP=w@mail.gmail.com>
2026-04-14 7:19 ` Andy Shevchenko
2026-04-14 8:06 ` Shuvam Pandey
2026-04-19 15:31 ` [PATCH v2] lib/string_helpers: renormalize string_get_size() after rounding carry Shuvam Pandey
2026-04-20 6:56 ` Andy Shevchenko
2026-04-21 3:04 ` James Bottomley
2026-04-21 3:38 ` Shuvam Pandey
2026-04-21 4:53 ` Andy Shevchenko
2026-04-21 10:10 ` Shuvam Pandey
2026-04-21 12:16 ` James Bottomley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.