Commit 1bb34a412750291e4e5e9f1d0fe7ae1b7e976098
Committed by
Linus Torvalds
1 parent
9c0cbd54ce
Exists in
master
and in
7 other branches
[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>
Showing 3 changed files with 24 additions and 61 deletions Side-by-side Diff
include/linux/timex.h
... | ... | @@ -282,6 +282,13 @@ |
282 | 282 | return !(time_status & STA_UNSYNC); |
283 | 283 | } |
284 | 284 | |
285 | +/* Required to safely shift negative values */ | |
286 | +#define shift_right(x, s) ({ \ | |
287 | + __typeof__(x) __x = (x); \ | |
288 | + __typeof__(s) __s = (s); \ | |
289 | + __x < 0 ? -(-__x >> __s) : __x >> __s; \ | |
290 | +}) | |
291 | + | |
285 | 292 | |
286 | 293 | #ifdef CONFIG_TIME_INTERPOLATION |
287 | 294 |
kernel/time.c
... | ... | @@ -338,30 +338,20 @@ |
338 | 338 | if (mtemp >= MINSEC) { |
339 | 339 | ltemp = (time_offset / mtemp) << (SHIFT_USEC - |
340 | 340 | SHIFT_UPDATE); |
341 | - if (ltemp < 0) | |
342 | - time_freq -= -ltemp >> SHIFT_KH; | |
343 | - else | |
344 | - time_freq += ltemp >> SHIFT_KH; | |
341 | + time_freq += shift_right(ltemp, SHIFT_KH); | |
345 | 342 | } else /* calibration interval too short (p. 12) */ |
346 | 343 | result = TIME_ERROR; |
347 | 344 | } else { /* PLL mode */ |
348 | 345 | if (mtemp < MAXSEC) { |
349 | 346 | ltemp *= mtemp; |
350 | - if (ltemp < 0) | |
351 | - time_freq -= -ltemp >> (time_constant + | |
352 | - time_constant + | |
353 | - SHIFT_KF - SHIFT_USEC); | |
354 | - else | |
355 | - time_freq += ltemp >> (time_constant + | |
347 | + time_freq += shift_right(ltemp,(time_constant + | |
356 | 348 | time_constant + |
357 | - SHIFT_KF - SHIFT_USEC); | |
349 | + SHIFT_KF - SHIFT_USEC)); | |
358 | 350 | } else /* calibration interval too long (p. 12) */ |
359 | 351 | result = TIME_ERROR; |
360 | 352 | } |
361 | - if (time_freq > time_tolerance) | |
362 | - time_freq = time_tolerance; | |
363 | - else if (time_freq < -time_tolerance) | |
364 | - time_freq = -time_tolerance; | |
353 | + time_freq = min(time_freq, time_tolerance); | |
354 | + time_freq = max(time_freq, -time_tolerance); | |
365 | 355 | } /* STA_PLL || STA_PPSTIME */ |
366 | 356 | } /* txc->modes & ADJ_OFFSET */ |
367 | 357 | if (txc->modes & ADJ_TICK) { |
... | ... | @@ -384,10 +374,7 @@ |
384 | 374 | if ((txc->modes & ADJ_OFFSET_SINGLESHOT) == ADJ_OFFSET_SINGLESHOT) |
385 | 375 | txc->offset = save_adjust; |
386 | 376 | else { |
387 | - if (time_offset < 0) | |
388 | - txc->offset = -(-time_offset >> SHIFT_UPDATE); | |
389 | - else | |
390 | - txc->offset = time_offset >> SHIFT_UPDATE; | |
377 | + txc->offset = shift_right(time_offset, SHIFT_UPDATE); | |
391 | 378 | } |
392 | 379 | txc->freq = time_freq + pps_freq; |
393 | 380 | txc->maxerror = time_maxerror; |
kernel/timer.c
... | ... | @@ -703,23 +703,13 @@ |
703 | 703 | * the adjustment over not more than the number of |
704 | 704 | * seconds between updates. |
705 | 705 | */ |
706 | - if (time_offset < 0) { | |
707 | - ltemp = -time_offset; | |
708 | - if (!(time_status & STA_FLL)) | |
709 | - ltemp >>= SHIFT_KG + time_constant; | |
710 | - if (ltemp > (MAXPHASE / MINSEC) << SHIFT_UPDATE) | |
711 | - ltemp = (MAXPHASE / MINSEC) << SHIFT_UPDATE; | |
712 | - time_offset += ltemp; | |
713 | - time_adj = -ltemp << (SHIFT_SCALE - SHIFT_HZ - SHIFT_UPDATE); | |
714 | - } else { | |
715 | 706 | ltemp = time_offset; |
716 | 707 | if (!(time_status & STA_FLL)) |
717 | - ltemp >>= SHIFT_KG + time_constant; | |
718 | - if (ltemp > (MAXPHASE / MINSEC) << SHIFT_UPDATE) | |
719 | - ltemp = (MAXPHASE / MINSEC) << SHIFT_UPDATE; | |
708 | + ltemp = shift_right(ltemp, SHIFT_KG + time_constant); | |
709 | + ltemp = min(ltemp, (MAXPHASE / MINSEC) << SHIFT_UPDATE); | |
710 | + ltemp = max(ltemp, -(MAXPHASE / MINSEC) << SHIFT_UPDATE); | |
720 | 711 | time_offset -= ltemp; |
721 | 712 | time_adj = ltemp << (SHIFT_SCALE - SHIFT_HZ - SHIFT_UPDATE); |
722 | - } | |
723 | 713 | |
724 | 714 | /* |
725 | 715 | * Compute the frequency estimate and additional phase |
726 | 716 | |
727 | 717 | |
728 | 718 | |
... | ... | @@ -736,39 +726,25 @@ |
736 | 726 | STA_PPSWANDER | STA_PPSERROR); |
737 | 727 | } |
738 | 728 | ltemp = time_freq + pps_freq; |
739 | - if (ltemp < 0) | |
740 | - time_adj -= -ltemp >> | |
741 | - (SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE); | |
742 | - else | |
743 | - time_adj += ltemp >> | |
744 | - (SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE); | |
729 | + time_adj += shift_right(ltemp,(SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE)); | |
745 | 730 | |
746 | 731 | #if HZ == 100 |
747 | 732 | /* Compensate for (HZ==100) != (1 << SHIFT_HZ). |
748 | 733 | * Add 25% and 3.125% to get 128.125; => only 0.125% error (p. 14) |
749 | 734 | */ |
750 | - if (time_adj < 0) | |
751 | - time_adj -= (-time_adj >> 2) + (-time_adj >> 5); | |
752 | - else | |
753 | - time_adj += (time_adj >> 2) + (time_adj >> 5); | |
735 | + time_adj += shift_right(time_adj, 2) + shift_right(time_adj, 5); | |
754 | 736 | #endif |
755 | 737 | #if HZ == 250 |
756 | 738 | /* Compensate for (HZ==250) != (1 << SHIFT_HZ). |
757 | 739 | * Add 1.5625% and 0.78125% to get 255.85938; => only 0.05% error (p. 14) |
758 | 740 | */ |
759 | - if (time_adj < 0) | |
760 | - time_adj -= (-time_adj >> 6) + (-time_adj >> 7); | |
761 | - else | |
762 | - time_adj += (time_adj >> 6) + (time_adj >> 7); | |
741 | + time_adj += shift_right(time_adj, 6) + shift_right(time_adj, 7); | |
763 | 742 | #endif |
764 | 743 | #if HZ == 1000 |
765 | 744 | /* Compensate for (HZ==1000) != (1 << SHIFT_HZ). |
766 | 745 | * Add 1.5625% and 0.78125% to get 1023.4375; => only 0.05% error (p. 14) |
767 | 746 | */ |
768 | - if (time_adj < 0) | |
769 | - time_adj -= (-time_adj >> 6) + (-time_adj >> 7); | |
770 | - else | |
771 | - time_adj += (time_adj >> 6) + (time_adj >> 7); | |
747 | + time_adj += shift_right(time_adj, 6) + shift_right(time_adj, 7); | |
772 | 748 | #endif |
773 | 749 | } |
774 | 750 | |
... | ... | @@ -787,10 +763,8 @@ |
787 | 763 | * Limit the amount of the step to be in the range |
788 | 764 | * -tickadj .. +tickadj |
789 | 765 | */ |
790 | - if (time_adjust > tickadj) | |
791 | - time_adjust_step = tickadj; | |
792 | - else if (time_adjust < -tickadj) | |
793 | - time_adjust_step = -tickadj; | |
766 | + time_adjust_step = min(time_adjust_step, (long)tickadj); | |
767 | + time_adjust_step = max(time_adjust_step, (long)-tickadj); | |
794 | 768 | |
795 | 769 | /* Reduce by this step the amount of time left */ |
796 | 770 | time_adjust -= time_adjust_step; |
... | ... | @@ -801,13 +775,8 @@ |
801 | 775 | * advance the tick more. |
802 | 776 | */ |
803 | 777 | time_phase += time_adj; |
804 | - if (time_phase <= -FINENSEC) { | |
805 | - long ltemp = -time_phase >> (SHIFT_SCALE - 10); | |
806 | - time_phase += ltemp << (SHIFT_SCALE - 10); | |
807 | - delta_nsec -= ltemp; | |
808 | - } | |
809 | - else if (time_phase >= FINENSEC) { | |
810 | - long ltemp = time_phase >> (SHIFT_SCALE - 10); | |
778 | + if ((time_phase >= FINENSEC) || (time_phase <= -FINENSEC)) { | |
779 | + long ltemp = shift_right(time_phase, (SHIFT_SCALE - 10)); | |
811 | 780 | time_phase -= ltemp << (SHIFT_SCALE - 10); |
812 | 781 | delta_nsec += ltemp; |
813 | 782 | } |