> >             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)

Reply via email to