Re: [PATCH] joydev: fix HZ->millisecond transformation

[Vojtech Pavlik <vojtech@suse.cz>]

On Fri, Dec 20, 2002 at 10:16:27AM -0800, george anzinger wrote:
> Vojtech Pavlik wrote:
> > 
> > On Fri, Dec 20, 2002 at 09:24:58AM -0800, george anzinger wrote:
> > > Vojtech Pavlik wrote:
> > > >
> > > > On Fri, Dec 20, 2002 at 02:41:50AM -0800, george anzinger wrote:
> > > > > Bjorn Helgaas wrote:
> > > > > >
> > > > > > * fix a problem with HZ->millisecond transformation on
> > > > > >   non-x86 archs (from 2.5 change by vojtech@suse.cz)
> > > > > >
> > > > > > Applies to 2.4.20.
> > > > > >
> > > > > > diff -Nru a/drivers/input/joydev.c b/drivers/input/joydev.c
> > > > > > --- a/drivers/input/joydev.c    Mon Dec 16 12:16:32 2002
> > > > > > +++ b/drivers/input/joydev.c    Mon Dec 16 12:16:32 2002
> > > > > > @@ -50,6 +50,8 @@
> > > > > >  #define JOYDEV_MINORS          32
> > > > > >  #define JOYDEV_BUFFER_SIZE     64
> > > > > >
> > > > > > +#define MSECS(t)       (1000 * ((t) / HZ) + 1000 * ((t) % HZ) / HZ)
> > > > > Uh...
> > > > > ^^^^^^^^^^^^^^^^
> > > > > by definition this is zero, is it not?
> > > >
> > > > No, both parts of the equaition can be nonzero.
> > >
> > > I don't think so.  s%HZ has to be less than HZ.  Then
> > > dividing that by HZ should result in zero.  Where is my
> > > thinking flawed?
> > 
> > You first multiply it by 1000.
> 
> But then it should read: (1000 * (t)) % HZ) / HZ

The expression is evaluated from left to right. No need for parentheses
here.

> But this is still zero.  There is no way that ((X % HZ) /
> HZ) is other than zero, me thinks.

Example:

HZ = 100
t = 70

1000 * (t % HZ) / HZ = 1000 * 70 / 100 = 700 != 0

Anyway (x % HZ) / HZ must be zero, but that's something different.

> > > > Though it might be easier to say (1000 * t) / HZ, now that I think about
> > > > it.
> > >
> > > That overflows...  As does the other if HZ is less than 1000....
> > 
> > You're right, t can be all 32 bits.
> 
> What, exactly, is this code trying to do?  It looks like the
> number is being passed to user space...

It's trying to convert jiffies to microseconds from computer start (or
any other arbitrary point in time), thus making the value independent of
HZ.

> -- 
> George Anzinger   george@mvista.com
> High-res-timers: 
> http://sourceforge.net/projects/high-res-timers/
> Preemption patch:
> http://www.kernel.org/pub/linux/kernel/people/rml

-- 
Vojtech Pavlik
SuSE Labs