[PATCH RFC] allow constant folding in msecs_to_jiffies where possible for gcc

[PATCH RFC] allow constant folding in msecs_to_jiffies where possible for gcc

[Nicholas Mc Guire]

A number of cleanup patches where switching var * HZ / 1000
constructs to msecs_to_jiffies(var) to ensure that all corener
cases are handled properly. The downside of this though is that
it now uses a function call and also was not performing
constant folding where it was originally possible.

msecs_to_jiffies() will calculate jiffies even if constants are
passed in that could be handled by constant folding at compile time
using __builtin_constant_p() gcc can optimize the constant case
again.

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

reported by Aaron Sierra <asierra@xes-inc.com>
solution suggested by Joe Perches <joe@perches.com>

Patch is against 4.0-rc6 (localversion-next is -next-20150401)

Boot tested on x86 64 only

 kernel/time/time.c |   39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/kernel/time/time.c b/kernel/time/time.c
index 2c85b77..8cb550a 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -496,15 +496,14 @@ EXPORT_SYMBOL(ns_to_timespec64);
  *   the input value by a factor or dividing it with a factor
  *
  * We must also be careful about 32-bit overflows.
+ *
+ * msecs_to_jiffies() will check for the passed in value being constant
+ * via __builtin_constant_p() allowing gcc to eliminate most of the code
+ * __msecs_to_jiffies() will be called if the value passed does not allow
+ * gcc to do constant folding.
  */
-unsigned long msecs_to_jiffies(const unsigned int m)
+static unsigned long __msecs_to_jiffies(const unsigned int m)
 {
-	/*
-	 * Negative value, means infinite timeout:
-	 */
-	if ((int)m < 0)
-		return MAX_JIFFY_OFFSET;
-
 #if HZ <= MSEC_PER_SEC && !(MSEC_PER_SEC % HZ)
 	/*
 	 * HZ is equal to or smaller than 1000, and 1000 is a nice
@@ -537,6 +536,32 @@ unsigned long msecs_to_jiffies(const unsigned int m)
 		>> MSEC_TO_HZ_SHR32;
 #endif
 }
+
+unsigned long msecs_to_jiffies(const unsigned int m)
+{
+	/*
+	 * Negative value, means infinite timeout:
+	 */
+	if ((int)m < 0)
+		return MAX_JIFFY_OFFSET;
+
+	if (__builtin_constant_p(m)) {
+#if HZ <= MSEC_PER_SEC && !(MSEC_PER_SEC % HZ)
+		return (m + (MSEC_PER_SEC / HZ) - 1) / (MSEC_PER_SEC / HZ);
+#elif HZ > MSEC_PER_SEC && !(HZ % MSEC_PER_SEC)
+		if (m > jiffies_to_msecs(MAX_JIFFY_OFFSET))
+			return MAX_JIFFY_OFFSET;
+		return m * (HZ / MSEC_PER_SEC);
+#else
+		if (HZ > MSEC_PER_SEC && m > jiffies_to_msecs(MAX_JIFFY_OFFSET))
+			return MAX_JIFFY_OFFSET;
+
+		return (MSEC_TO_HZ_MUL32 * m + MSEC_TO_HZ_ADJ32)
+			>> MSEC_TO_HZ_SHR32;
+#endif
+	} else
+		return __msecs_to_jiffies(m);
+}
 EXPORT_SYMBOL(msecs_to_jiffies);
 
 unsigned long usecs_to_jiffies(const unsigned int u)
-- 
1.7.10.4

Re: [PATCH RFC] allow constant folding in msecs_to_jiffies where possible for gcc

[Joe Perches]

On Thu, 2015-04-02 at 08:37 +0200, Nicholas Mc Guire wrote:
> A number of cleanup patches where switching var * HZ / 1000
> constructs to msecs_to_jiffies(var) to ensure that all corener
> cases are handled properly. The downside of this though is that
> it now uses a function call and also was not performing
> constant folding where it was originally possible.
> 
> msecs_to_jiffies() will calculate jiffies even if constants are
> passed in that could be handled by constant folding at compile time
> using __builtin_constant_p() gcc can optimize the constant case
> again.

OK, but how could this actually work?

This code isn't visible in a module at compile time, so how
could the compiler actually optimize the function call away?

Look at the disassembly for any object that uses
msecs_to_jiffies(<constant>).

For instance: drivers/bluetooth/hci_vhci.o

	schedule_delayed_work(&data->open_timeout, msecs_to_jiffies(1000));
 60d:	bf e8 03 00 00       	mov    $0x3e8,%edi
 612:	e8 00 00 00 00       	callq  617 <vhci_open+0xc7>
			613: R_X86_64_PC32	msecs_to_jiffies-0x4

You need to make this code either a macro or static inline
in jiffies.h, and for that the kernel/timeconst.h file needs
to be available.