Subject: Re: timeout issue in scsi drivers: mstohz()
To: David Laight <david@l8s.co.uk>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: tech-kern
Date: 04/04/2002 21:53:50
On Wed, Apr 03, 2002 at 10:17:31PM +0100, David Laight wrote:
> > +/*
> > + * macro to convert from milliseconds to hz without integer overflow
> > + * Default version using only 32bits arithmetics.
> > + * 64bit port can define 64bit version in their <machine/param.h>
> > + * 100000 is safe for hz < 20000
> > + */
> > +#ifndef mstohz
> > +#define mstohz(ms) \
> > + (__predict_false((ms) >= 100000)) ? \
> > + (((ms) / 1000 + 1) * hz) : \
> > + (((ms) * hz) / 1000)
> > +#endif
> > +#endif /* _KERNEL */
>
> For systems with restricted constants, replacing 100000
> with (say) 0x10000 is probably worthwhile.
Good point, I've remplaced it with 0x20000 (131072)
>
> Should the values be rounded up ever?
>
> If the values can be rounded down, dividing by 1024 will
> be a benefit on systems that don't have a fast divide.
I don't think this is safe, however. mstohz() may be used for something else
than SCSI.
And for some device the timeout may come from device specs.
>
> Also any software divide will be faster if unsigned
> arithmetic is used. IIRC 1000u is enough to force this,
> otherwise use ((ms) + 0u) * hz / 1000u
>
> EEEK - you've added so many superfluous parentheses it is
> impossibly to spot you've missed the most important set!
>
> look what (fixed_delay + mstohz( ms )) evaluates to!
> try:
> #define mstohz(ms) \
> (__predict_false((ms) >= 0x10000u) ? \
> ((ms) + 0u) / 1000u * hz : \
> ((ms) + 0u) * hz / 1000u)
Ops, good point.
--
Manuel Bouyer <bouyer@antioche.eu.org>
--