linux-kernel - RE: [PATCH][next] bcache: Use 64-bit arithmetic instead of 32-bit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0a2eb2e143ad480cbce3f84c3c920b5f@AcuMS.aculab.com>
Date:   Fri, 12 Feb 2021 15:31:16 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Coly Li' <colyli@...e.de>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Kent Overstreet <kent.overstreet@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Christina Jacob <cjacob@...vell.com>,
        Hariprasad Kelam <hkelam@...vell.com>,
        Sunil Goutham <sgoutham@...vell.com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>
CC:     "linux-bcache@...r.kernel.org" <linux-bcache@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH][next] bcache: Use 64-bit arithmetic instead of 32-bit

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