Re: [PATCH 19/34]: include/jiffies: add usecs_to_jiffies() functi on

Re: [PATCH 19/34]: include/jiffies: add usecs_to_jiffies() functi on

[Makhlis, Lev]

Nishanth Aravamudan <nacc@us.ibm.com> wrote:

> +static inline unsigned long usecs_to_jiffies(const unsigned int u)
> +{
> +	if (u > jiffies_to_usecs(MAX_JIFFY_OFFSET))
> +		return MAX_JIFFY_OFFSET;
> +#if HZ <= 1000 && !(1000 % HZ)
> +	return (u + (1000000 / HZ) - 1000) / (1000000 / HZ);
> +#elif HZ > 1000 && !(HZ % 1000)
> +	return u * (HZ / 1000000);
> +#else
> +	return (u * HZ + 999999) / 1000000;
> +#endif
> +}

Shouldn't this use 1000000 instead of 1000 everywhere?
It returns 0 if HZ=10000.

[UPDATE PATCH 19/34]: include/jiffies: add usecs_to_jiffies() function

[Nishanth Aravamudan]

On Thu, Feb 03, 2005 at 10:07:15AM -0600, Makhlis, Lev wrote:
> Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> 
> > +static inline unsigned long usecs_to_jiffies(const unsigned int u)
> > +{
> > +	if (u > jiffies_to_usecs(MAX_JIFFY_OFFSET))
> > +		return MAX_JIFFY_OFFSET;
> > +#if HZ <= 1000 && !(1000 % HZ)
> > +	return (u + (1000000 / HZ) - 1000) / (1000000 / HZ);
> > +#elif HZ > 1000 && !(HZ % 1000)
> > +	return u * (HZ / 1000000);
> > +#else
> > +	return (u * HZ + 999999) / 1000000;
> > +#endif
> > +}
> 
> Shouldn't this use 1000000 instead of 1000 everywhere?
> It returns 0 if HZ=10000.

Thanks for your feedback!

I believe you are correct... Thanks for catching this! This ends up also being a
problem for jiffies_to_usecs() actually, as it improperly converts certain
values as it is coded. The attached patch, which overrides the previous one
seems to be more correct. Andrew, if you would prefer an incremental patch,
please let me know.

Description: Add a usecs_to_jiffies() function. With the potential for dynamic HZ
values much higher than 1000, we may need to consider times as small as usecs in
terms of jiffies. We have msecs_to_jiffies(), jiffies_to_msecs() and
jiffies_to_usecs(), but no usecs_to_jiffies(). Additionally, the
usecs_to_jiffies() conversion is wrong. The function currently is just a linear
translation of the jiffies_to_msecs() macro, but this is incorrect, as it
changes the rounding point. Please check my math.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>

--- 2.6.11-rc2-kj-v/include/linux/jiffies.h	2005-01-24 09:34:19.000000000 -0800
+++ 2.6.11-rc2-kj/include/linux/jiffies.h	2005-02-03 10:44:22.000000000 -0800
@@ -265,10 +265,10 @@ static inline unsigned int jiffies_to_ms
 
 static inline unsigned int jiffies_to_usecs(const unsigned long j)
 {
-#if HZ <= 1000 && !(1000 % HZ)
+#if HZ <= 1000000 && !(1000000 % HZ)
 	return (1000000 / HZ) * j;
-#elif HZ > 1000 && !(HZ % 1000)
-	return (j*1000 + (HZ - 1000))/(HZ / 1000);
+#elif HZ > 1000000 && !(HZ % 1000000)
+	return (j + (HZ / 1000000) - 1)/(HZ / 1000000);
 #else
 	return (j * 1000000) / HZ;
 #endif
@@ -287,6 +287,19 @@ static inline unsigned long msecs_to_jif
 #endif
 }
 
+static inline unsigned long usecs_to_jiffies(const unsigned int u)
+{
+	if (u > jiffies_to_usecs(MAX_JIFFY_OFFSET))
+		return MAX_JIFFY_OFFSET;
+#if HZ <= 1000000 && !(1000000 % HZ)
+	return (u + (1000000 / HZ) - 1) / (1000000 / HZ);
+#elif HZ > 1000000 && !(HZ % 1000000)
+	return u * (HZ / 1000000);
+#else
+	return (u * HZ + 999999) / 1000000;
+#endif
+}
+
 /*
  * The TICK_NSEC - 1 rounds up the value to the next resolution.  Note
  * that a remainder subtract here would not do the right thing as the

[UPDATE PATCH]: include/jiffies: fix usecs_to_jiffies()/jiffies_to_usecs() math

[Nishanth Aravamudan]

On Thu, Feb 03, 2005 at 10:48:05AM -0800, Nishanth Aravamudan wrote:
> On Thu, Feb 03, 2005 at 10:07:15AM -0600, Makhlis, Lev wrote:
> > Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> > 
> > > +static inline unsigned long usecs_to_jiffies(const unsigned int u)
> > > +{
> > > +	if (u > jiffies_to_usecs(MAX_JIFFY_OFFSET))
> > > +		return MAX_JIFFY_OFFSET;
> > > +#if HZ <= 1000 && !(1000 % HZ)
> > > +	return (u + (1000000 / HZ) - 1000) / (1000000 / HZ);
> > > +#elif HZ > 1000 && !(HZ % 1000)
> > > +	return u * (HZ / 1000000);
> > > +#else
> > > +	return (u * HZ + 999999) / 1000000;
> > > +#endif
> > > +}
> > 
> > Shouldn't this use 1000000 instead of 1000 everywhere?
> > It returns 0 if HZ=10000.
> 
> Thanks for your feedback!
> 
> I believe you are correct... Thanks for catching this! This ends up also being a
> problem for jiffies_to_usecs() actually, as it improperly converts certain
> values as it is coded. The attached patch, which overrides the previous one
> seems to be more correct. Andrew, if you would prefer an incremental patch,
> please let me know.
> 
> Description: Add a usecs_to_jiffies() function. With the potential for dynamic HZ

As was kindly pointed out to me, usecs_to_jiffies() has already exists! :) The
currently included patch is for 2.6.11-rc3 and fixes the math in that version.

Thanks,
Nish

Description: Fixes the math of both jiffies_to_usecs() and usecs_to_jiffies()
which improperly assume the same rounding point -- 1,000 -- as jiffies_to_msecs()
and msecs_to_jiffies(), when in fact it should be 1,000,000. Furthermore, the
actual math of both functions is actually wrong and will lead to more than just
rounding errors.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>

--- 2.6.11-rc3-kj-v/include/linux/jiffies.h	2005-02-03 17:05:13.000000000 -0800
+++ 2.6.11-rc3-kj/include/linux/jiffies.h	2005-02-03 17:01:48.000000000 -0800
@@ -265,10 +265,10 @@ static inline unsigned int jiffies_to_ms
 
 static inline unsigned int jiffies_to_usecs(const unsigned long j)
 {
-#if HZ <= 1000 && !(1000 % HZ)
+#if HZ <= 1000000 && !(1000000 % HZ)
 	return (1000000 / HZ) * j;
-#elif HZ > 1000 && !(HZ % 1000)
-	return (j*1000 + (HZ - 1000))/(HZ / 1000);
+#elif HZ > 1000000 && !(HZ % 1000000)
+	return (j + (HZ / 1000000) - 1)/(HZ / 1000000);
 #else
 	return (j * 1000000) / HZ;
 #endif
@@ -291,9 +291,9 @@ static inline unsigned long usecs_to_jif
 {
 	if (u > jiffies_to_usecs(MAX_JIFFY_OFFSET))
 		return MAX_JIFFY_OFFSET;
-#if HZ <= 1000 && !(1000 % HZ)
-	return (u + (1000000 / HZ) - 1000) / (1000000 / HZ);
-#elif HZ > 1000 && !(HZ % 1000)
+#if HZ <= 1000000 && !(1000000 % HZ)
+	return (u + (1000000 / HZ) - 1) / (1000000 / HZ);
+#elif HZ > 1000000 && !(HZ % 1000000)
 	return u * (HZ / 1000000);
 #else
 	return (u * HZ + 999999) / 1000000;