[PATCH v3] cpuidle: Fix last_residency division

[Nicolas Pitre]

On Wed, 29 Jun 2016, Nicolas Pitre wrote:

> On Wed, 29 Jun 2016, Daniel Lezcano wrote:
> 
> > On 06/29/2016 09:06 AM, Shreyas B. Prabhu wrote:
> > > diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
> > > index f87f399..c8ea5ad 100644
> > > --- a/drivers/cpuidle/cpuidle.h
> > > +++ b/drivers/cpuidle/cpuidle.h
> > > @@ -68,4 +68,27 @@ static inline void
> > > cpuidle_coupled_unregister_device(struct cpuidle_device *dev)
> > >   }
> > >   #endif
> > >
> > > +/*
> > > + * Used for calculating last_residency in usec. Optimized for case
> > > + * where last_residency in nsecs is < INT_MAX/2 by using faster
> > > + * approximation. Approximated value has less than 1% error.
> > > + */
> > > +static inline int convert_nsec_to_usec(u64 nsec)
> > > +{
> > > +	if (likely(nsec < INT_MAX / 2)) {
> > 
> > UINT_MAX ?
> 
> Actually this can be better than that.
> 
> > > +		int usec = (int)nsec;
> 
> First, you'll want an unsigned type. Given the provided argument is u64, 
> we can assume there won't be any negative values here.
> 
> Then it would be wise to use a type with an explicit width, like U32.
> 
> > > +		usec += usec >> 5;
> > > +		usec = usec >> 10;
> > > +		return usec;
> 
> And now you want to maximize the available range. So not to overflow the 
> first addition, we must respect:
> 
> 	usec + (usec >> 5) <= 0xffffffff 
> 	usec + usec/32 <= 0xffffffff
> 	usec <= (0xffffffff * 32) / 33
> 
> Therefore:
> 
> 	nsec <= 0xf83e0f82

And to be sure, you should use 0xf83e0f82UL to avoid any potential sign 
extension.


Nicolas