Re: sparse warning, or why does jifies_to_msecs() return an int?

[David Mosberger <davidm@napali.hpl.hp.com>]

>>>>> On Sat, 15 Jan 2005 10:05:37 -0800 (PST), Linus Torvalds <torvalds@osdl.org> said:

  Linus> Hmm.. I don't think your patch is wrong per se, but I do
  Linus> think it's a bit too subtle. I'd almost rather make
  Linus> "jiffies_to_msecs()" just test for overflow instead, and that
  Linus> should also fix it.

You sure about that?

Actually, I think my patch was broken anyhow for HZ < 1000 because you
can potentially get integer-overflows in temporary results which could
make things come out wrong again.

I _think_ the attached patch works for all reasonable cases reasonably
uniformly, but if you thought the previous patch was subtle, I'm sure
you going to like this one even less.

Note that with the patch, platforms where HZ is not a power of two and
doesn't fit any of the other special cases (namely (HZ % 1000) != 0 &&
(1000 % HZ) != 0) would suffer a penalty.  AFAICS, this is true only
for Alpha/Rawhide (HZ=1200).  In such a case, rather than:

	(j * 1000)/HZ

the new code would compute:

	(j/HZ)*1000 + ((j%HZ)*1000)/HZ

It looks to me like we could get rid of all the ugly & complex
intermediate overflow-checks if we defined MAX_JIFFY_OFFSET
as:
	(~0UL / 1000)

However, on a 32-bit platform that runs at 1000 Hz, this would limit
us to 4294 seconds.  That may be cutting it a bit close.

	--david

===== include/linux/jiffies.h 1.11 vs edited =====
--- 1.11/include/linux/jiffies.h	2005-01-04 18:48:02 -08:00
+++ edited/include/linux/jiffies.h	2005-01-20 15:21:14 -08:00
@@ -254,13 +254,32 @@
  */
 static inline unsigned int jiffies_to_msecs(const unsigned long j)
 {
+	unsigned long res;
+
 #if HZ <= 1000 && !(1000 % HZ)
-	return (1000 / HZ) * j;
+	unsigned long max = ~0UL / (1000 / HZ);
+
+	if (j > max)
+		max = j;
+	res = (1000 / HZ) * j;
 #elif HZ > 1000 && !(HZ % 1000)
-	return (j + (HZ / 1000) - 1)/(HZ / 1000);
+	res = (j + (HZ / 1000) - 1) / (HZ / 1000);
 #else
-	return (j * 1000) / HZ;
+	/*
+	 * HZ better be a power of two; otherwise this gets real
+	 * expensive.  Better expensive than wrong, though.
+	 */
+# if HZ < 1000
+	unsigned long max = (~0UL / 1000) * HZ;
+
+	if (j > max)
+		j = max;
+# endif
+	res = (j / HZ) * 1000 + ((j % HZ) * 1000) / HZ;
 #endif
+	if (res > ~0U)
+		return ~0U;
+	return res;
 }
 
 static inline unsigned int jiffies_to_usecs(const unsigned long j)