Messages in this thread | | | From | David Laight <> | Subject | RE: [PATCH][next] bcache: Use 64-bit arithmetic instead of 32-bit | Date | Fri, 12 Feb 2021 15:31:16 +0000 |
| |
> > if (c->gc_stats.in_use <= BCH_WRITEBACK_FRAGMENT_THRESHOLD_MID) { > > - fp_term = dc->writeback_rate_fp_term_low * > > + fp_term = (int64_t)dc->writeback_rate_fp_term_low * > > (c->gc_stats.in_use - BCH_WRITEBACK_FRAGMENT_THRESHOLD_LOW); > > } else if (c->gc_stats.in_use <= BCH_WRITEBACK_FRAGMENT_THRESHOLD_HIGH) { > > - fp_term = dc->writeback_rate_fp_term_mid * > > + fp_term = (int64_t)dc->writeback_rate_fp_term_mid * > > (c->gc_stats.in_use - BCH_WRITEBACK_FRAGMENT_THRESHOLD_MID); > > } else { > > - fp_term = dc->writeback_rate_fp_term_high * > > + fp_term = (int64_t)dc->writeback_rate_fp_term_high * > > (c->gc_stats.in_use - BCH_WRITEBACK_FRAGMENT_THRESHOLD_HIGH); > > } > > fps = div_s64(dirty, dirty_buckets) * fp_term; > > > > Hmm, should such thing be handled by compiler ? Otherwise this kind of > potential overflow issue will be endless time to time. > > I am not a compiler expert, should we have to do such explicit type cast > all the time ?
We do to get a 64bit product from two 32bit values. An alternative for the above would be: fp_term = c->gc_stats.in_use - BCH_WRITEBACK_FRAGMENT_THRESHOLD_HIGH; fp_term *= dc->writeback_rate_fp_term_high;
I hope BCH_WRITEBACK_FRAGMENT_THRESHOLD_LOW is zero :-)
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
| |