[PATCH] NTP shift_right cleanup

Create a macro shift_right() that avoids the numerous ugly conditionals in the
NTP code that look like:

        if(a < 0)
                b = -(-a >> shift);
        else
                b = a >> shift;

Replacing it with:

        b = shift_right(a, shift);

This should have zero effect on the logic, however it should probably have
a bit of testing just to be sure.

Also replace open-coded min/max with the macros.

Signed-off-by : John Stultz <johnstul@us.ibm.com>

Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/include/linux/timex.h b/include/linux/timex.h
index 7e050a2cc35be69f2685309ff9c445adbba8ae68..04a4a8cb4ed37ab150b6575b3ae9f9268967e5e7 100644 (file)
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -282,6 +282,13 @@ static inline int ntp_synced(void)
        return !(time_status & STA_UNSYNC);
 }
 
+/* Required to safely shift negative values */
+#define shift_right(x, s) ({   \
+       __typeof__(x) __x = (x);        \
+       __typeof__(s) __s = (s);        \
+       __x < 0 ? -(-__x >> __s) : __x >> __s;  \
+})
+
 
 #ifdef CONFIG_TIME_INTERPOLATION
 

diff --git a/kernel/time.c b/kernel/time.c
index a3c2100470e106becb5155d091e2bf9c6d1dee7c..245d595a13cb9f566f67c2ac6ab6dfb8c0af1143 100644 (file)
--- a/kernel/time.c
+++ b/kernel/time.c
@@ -338,30 +338,20 @@ int do_adjtimex(struct timex *txc)
                        if (mtemp >= MINSEC) {
                            ltemp = (time_offset / mtemp) << (SHIFT_USEC -
                                                              SHIFT_UPDATE);
-                           if (ltemp < 0)
-                               time_freq -= -ltemp >> SHIFT_KH;
-                           else
-                               time_freq += ltemp >> SHIFT_KH;
+                           time_freq += shift_right(ltemp, SHIFT_KH);
                        } else /* calibration interval too short (p. 12) */
                                result = TIME_ERROR;
                    } else {    /* PLL mode */
                        if (mtemp < MAXSEC) {
                            ltemp *= mtemp;
-                           if (ltemp < 0)
-                               time_freq -= -ltemp >> (time_constant +
-                                                       time_constant +
-                                                       SHIFT_KF - SHIFT_USEC);
-                           else
-                               time_freq += ltemp >> (time_constant +
+                           time_freq += shift_right(ltemp,(time_constant +
                                                       time_constant +
-                                                      SHIFT_KF - SHIFT_USEC);
+                                                      SHIFT_KF - SHIFT_USEC));
                        } else /* calibration interval too long (p. 12) */
                                result = TIME_ERROR;
                    }
-                   if (time_freq > time_tolerance)
-                       time_freq = time_tolerance;
-                   else if (time_freq < -time_tolerance)
-                       time_freq = -time_tolerance;
+                   time_freq = min(time_freq, time_tolerance);
+                   time_freq = max(time_freq, -time_tolerance);
                } /* STA_PLL || STA_PPSTIME */
            } /* txc->modes & ADJ_OFFSET */
            if (txc->modes & ADJ_TICK) {
@@ -384,10 +374,7 @@ leave:     if ((time_status & (STA_UNSYNC|STA_CLOCKERR)) != 0
        if ((txc->modes & ADJ_OFFSET_SINGLESHOT) == ADJ_OFFSET_SINGLESHOT)
            txc->offset    = save_adjust;
        else {
-           if (time_offset < 0)
-               txc->offset = -(-time_offset >> SHIFT_UPDATE);
-           else
-               txc->offset = time_offset >> SHIFT_UPDATE;
+           txc->offset = shift_right(time_offset, SHIFT_UPDATE);
        }
        txc->freq          = time_freq + pps_freq;
        txc->maxerror      = time_maxerror;

diff --git a/kernel/timer.c b/kernel/timer.c
index 6ed1a826e5ced0f3812bdc5126cd46ada4ef92eb..6b94adb45b03b0ccd1a239470347087393c8f886 100644 (file)
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -703,23 +703,13 @@ static void second_overflow(void)
      * the adjustment over not more than the number of
      * seconds between updates.
      */
-    if (time_offset < 0) {
-       ltemp = -time_offset;
-       if (!(time_status & STA_FLL))
-           ltemp >>= SHIFT_KG + time_constant;
-       if (ltemp > (MAXPHASE / MINSEC) << SHIFT_UPDATE)
-           ltemp = (MAXPHASE / MINSEC) << SHIFT_UPDATE;
-       time_offset += ltemp;
-       time_adj = -ltemp << (SHIFT_SCALE - SHIFT_HZ - SHIFT_UPDATE);
-    } else {
        ltemp = time_offset;
        if (!(time_status & STA_FLL))
-           ltemp >>= SHIFT_KG + time_constant;
-       if (ltemp > (MAXPHASE / MINSEC) << SHIFT_UPDATE)
-           ltemp = (MAXPHASE / MINSEC) << SHIFT_UPDATE;
+               ltemp = shift_right(ltemp, SHIFT_KG + time_constant);
+       ltemp = min(ltemp, (MAXPHASE / MINSEC) << SHIFT_UPDATE);
+       ltemp = max(ltemp, -(MAXPHASE / MINSEC) << SHIFT_UPDATE);
        time_offset -= ltemp;
        time_adj = ltemp << (SHIFT_SCALE - SHIFT_HZ - SHIFT_UPDATE);
-    }
 
     /*
      * Compute the frequency estimate and additional phase
@@ -736,39 +726,25 @@ static void second_overflow(void)
                         STA_PPSWANDER | STA_PPSERROR);
     }
     ltemp = time_freq + pps_freq;
-    if (ltemp < 0)
-       time_adj -= -ltemp >>
-           (SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE);
-    else
-       time_adj += ltemp >>
-           (SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE);
+    time_adj += shift_right(ltemp,(SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE));
 
 #if HZ == 100
     /* Compensate for (HZ==100) != (1 << SHIFT_HZ).
      * Add 25% and 3.125% to get 128.125; => only 0.125% error (p. 14)
      */
-    if (time_adj < 0)
-       time_adj -= (-time_adj >> 2) + (-time_adj >> 5);
-    else
-       time_adj += (time_adj >> 2) + (time_adj >> 5);
+    time_adj += shift_right(time_adj, 2) + shift_right(time_adj, 5);
 #endif
 #if HZ == 250
     /* Compensate for (HZ==250) != (1 << SHIFT_HZ).
      * Add 1.5625% and 0.78125% to get 255.85938; => only 0.05% error (p. 14)
      */
-    if (time_adj < 0)
-       time_adj -= (-time_adj >> 6) + (-time_adj >> 7);
-    else
-       time_adj += (time_adj >> 6) + (time_adj >> 7);
+    time_adj += shift_right(time_adj, 6) + shift_right(time_adj, 7);
 #endif
 #if HZ == 1000
     /* Compensate for (HZ==1000) != (1 << SHIFT_HZ).
      * Add 1.5625% and 0.78125% to get 1023.4375; => only 0.05% error (p. 14)
      */
-    if (time_adj < 0)
-       time_adj -= (-time_adj >> 6) + (-time_adj >> 7);
-    else
-       time_adj += (time_adj >> 6) + (time_adj >> 7);
+    time_adj += shift_right(time_adj, 6) + shift_right(time_adj, 7);
 #endif
 }
 
@@ -787,10 +763,8 @@ static void update_wall_time_one_tick(void)
             * Limit the amount of the step to be in the range
             * -tickadj .. +tickadj
             */
-            if (time_adjust > tickadj)
-               time_adjust_step = tickadj;
-            else if (time_adjust < -tickadj)
-               time_adjust_step = -tickadj;
+            time_adjust_step = min(time_adjust_step, (long)tickadj);
+            time_adjust_step = max(time_adjust_step, (long)-tickadj);
 
            /* Reduce by this step the amount of time left  */
            time_adjust -= time_adjust_step;
@@ -801,13 +775,8 @@ static void update_wall_time_one_tick(void)
         * advance the tick more.
         */
        time_phase += time_adj;
-       if (time_phase <= -FINENSEC) {
-               long ltemp = -time_phase >> (SHIFT_SCALE - 10);
-               time_phase += ltemp << (SHIFT_SCALE - 10);
-               delta_nsec -= ltemp;
-       }
-       else if (time_phase >= FINENSEC) {
-               long ltemp = time_phase >> (SHIFT_SCALE - 10);
+       if ((time_phase >= FINENSEC) || (time_phase <= -FINENSEC)) {
+               long ltemp = shift_right(time_phase, (SHIFT_SCALE - 10));
                time_phase -= ltemp << (SHIFT_SCALE - 10);
                delta_nsec += ltemp;
        }