On Saturday, September 7th, 2024 at 12:00 AM, Hongbo Li <[email protected]> wrote:
> > On 2024/9/7 12:06, Reed Riley wrote: > > > Bcachefs often uses this function to divide by nanosecond times - which > > can easily cause problems when cast to u32. For example, `cat > > /sys/fs/bcachefs/*/internal/rebalance_status` would return invalid data > > in the `duration waited` field because dividing by the number of > > nanoseconds in a minute requires the divisor parameter to be u64. > > > > Signed-off-by: Reed Riley [email protected] > > --- > > fs/bcachefs/sysfs.c | 2 +- > > fs/bcachefs/util.c | 12 ++++++------ > > 2 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/fs/bcachefs/sysfs.c b/fs/bcachefs/sysfs.c > > index f393023a3ae2..561998b2b5dc 100644 > > --- a/fs/bcachefs/sysfs.c > > +++ b/fs/bcachefs/sysfs.c > > @@ -288,7 +288,7 @@ static int bch2_compression_stats_to_text(struct > > printbuf *out, struct bch_fs *c > > prt_tab_rjust(out); > > > > prt_human_readable_u64(out, nr_extents > > - ? div_u64(sectors_uncompressed << 9, nr_extents) > > + ? div64_u64(sectors_uncompressed << 9, nr_extents) > > : 0); > > > ah, it seems that your code lacks indentation. But the changes you've > made appear to be fine. May be you could add some comments in the commit > message to explain what has been modified in v2 (just separate it with > ---) to make it more clearer. But I see that Kent has already merged > your code. 😄 Oh, all I did with the second version was remove one callsite to this function that, as you had pointed out, didn't _technically_ need it. (I'd included it previously because the 'XXX' comment right above it had suggested that there might be future refactoring, and having the right function in place might make future refactoring safer? But it's certainly not necessary.) Either version of the patch is equally fine from my perspective, and I'll defer entirely to the maintainers. > Thanks, > Hongbo > > > prt_tab_rjust(out); > > prt_newline(out); > > diff --git a/fs/bcachefs/util.c b/fs/bcachefs/util.c > > index 1b8554460af4..144d8df48b1e 100644 > > --- a/fs/bcachefs/util.c > > +++ b/fs/bcachefs/util.c > > @@ -64,7 +64,7 @@ static int bch2_pow(u64 n, u64 p, u64 *res) > > *res = 1; > > > > while (p--) { > > - if (*res > div_u64(U64_MAX, n)) > > + if (*res > div64_u64(U64_MAX, n)) > > return -ERANGE; > > *res *= n; > > } > > @@ -140,14 +140,14 @@ static int __bch2_strtou64_h(const char *cp, u64 *res) > > > > parse_or_ret(cp, parse_unit_suffix(cp, &b)); > > > > - if (v > div_u64(U64_MAX, b)) > > + if (v > div64_u64(U64_MAX, b)) > > return -ERANGE; > > v *= b; > > > > - if (f_n > div_u64(U64_MAX, b)) > > + if (f_n > div64_u64(U64_MAX, b)) > > return -ERANGE; > > > > - f_n = div_u64(f_n * b, f_d); > > + f_n = div64_u64(f_n * b, f_d); > > if (v + f_n < v) > > return -ERANGE; > > v += f_n; > > @@ -360,7 +360,7 @@ void bch2_pr_time_units(struct printbuf *out, u64 ns) > > { > > const struct time_unit *u = bch2_pick_time_units(ns); > > > > - prt_printf(out, "%llu %s", div_u64(ns, u->nsecs), u->name); > > + prt_printf(out, "%llu %s", div64_u64(ns, u->nsecs), u->name); > > } > > > > static void bch2_pr_time_units_aligned(struct printbuf *out, u64 ns) > > @@ -477,7 +477,7 @@ void bch2_time_stats_to_text(struct printbuf *out, > > struct bch2_time_stats *stats > > bool is_last = eytzinger0_next(i, NR_QUANTILES) == -1; > > > > u64 q = max(quantiles->entries[i].m, last_q); > > - prt_printf(out, "%llu ", div_u64(q, u->nsecs)); > > + prt_printf(out, "%llu ", div64_u64(q, u->nsecs)); > > if (is_last) > > prt_newline(out); > > last_q = q; > > -- > > 2.46.0
