Hi Balbir, Thanks for reviewing the patch!
On Fri, Jun 01, 2018 at 12:51:05AM +1000, Balbir Singh wrote: > On Thu, May 31, 2018 at 10:15 PM, Gautham R. Shenoy [..snip..] > > > > +static u64 get_snooze_timeout(struct cpuidle_device *dev, > > + struct cpuidle_driver *drv, > > + int index) > > +{ > > + int i; > > + > > + if (unlikely(!snooze_timeout_en)) > > + return default_snooze_timeout; > > + > > + for (i = index + 1; i < drv->state_count; i++) { > > + struct cpuidle_state *s = &drv->states[i]; > > + struct cpuidle_state_usage *su = &dev->states_usage[i]; > > + > > + if (s->disabled || su->disable) > > + continue; > > + > > + return s->target_residency * tb_ticks_per_usec; > > Can we ensure this is not prone to overflow? s->target_residency is an "unsigned int" so can take a maximum value of UINT_MAX. tb_ticks_per_usec is an "unsigned long" with a value in the range of 100-1000. The return value is a u64. The product of s->target_residency and tb_ticks_per_usec should be contained in u64. Is there a potential case of overflow that I am missing ? > > Otherwise looks good > > Reviewed-by: Balbir Singh <bsinghar...@gmail.com> -- Thanks and Regards gautham.