current_tick_length used to do a little more, but now it just returns
tick_length, which we can also access directly at the few places, where
it's needed.

Signed-off-by: Roman Zippel <zippel@linux-m68k.org>

---
 include/linux/timex.h     |    4 ++--
 kernel/time/ntp.c         |   16 ++--------------
 kernel/time/timekeeping.c |    5 ++---
 3 files changed, 6 insertions(+), 19 deletions(-)

Index: linux-2.6-mm/include/linux/timex.h
===================================================================
--- linux-2.6-mm.orig/include/linux/timex.h
+++ linux-2.6-mm/include/linux/timex.h
@@ -234,8 +234,8 @@ static inline int ntp_synced(void)
 #endif
 #define NTP_INTERVAL_LENGTH (NSEC_PER_SEC/NTP_INTERVAL_FREQ)
 
-/* Returns how long ticks are at present, in ns / 2^(SHIFT_SCALE-10). */
-extern u64 current_tick_length(void);
+/* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
+extern u64 tick_length;
 
 extern void second_overflow(void);
 extern void update_ntp_one_tick(void);
Index: linux-2.6-mm/kernel/time/ntp.c
===================================================================
--- linux-2.6-mm.orig/kernel/time/ntp.c
+++ linux-2.6-mm/kernel/time/ntp.c
@@ -23,7 +23,8 @@
  */
 unsigned long tick_usec = TICK_USEC; 		/* USER_HZ period (usec) */
 unsigned long tick_nsec;			/* ACTHZ period (nsec) */
-static u64 tick_length, tick_length_base;
+u64 tick_length;
+static u64 tick_length_base;
 
 #define MAX_TICKADJ		500		/* microsecs */
 #define MAX_TICKADJ_SCALED	(((u64)(MAX_TICKADJ * NSEC_PER_USEC) << \
@@ -203,19 +204,6 @@ void second_overflow(void)
 	}
 }
 
-/*
- * Return how long ticks are at the moment, that is, how much time
- * update_wall_time_one_tick will add to xtime next time we call it
- * (assuming no calls to do_adjtimex in the meantime).
- * The return value is in fixed-point nanoseconds shifted by the
- * specified number of bits to the right of the binary point.
- * This function has no side-effects.
- */
-u64 current_tick_le...
[ message continues ]

Hrm. I'm not sure I like using a global variable instead of using an
accessor function. At least with the accessor function folks couldn't
just tweak the value outside ntp as easily.

Is there any additional rational for this change?

thanks
-john


--

Hi,



Useless bloat?

bye, Roman
--

Its not a huge deal, but we've seen globally scoped timekeeping
variables misused either accidentally or intentionally. Awhile back ppc
was corrupting time_offset by using it for a timezone offset value. 


I agree its a trade off. But do you have performance numbers to make the
maintainability trade off worth it? (I admit, it is used in the
timer_interrupt, so it may very well be worth it, but we might want to
check first).

thanks
-john


--

Hi,


ppc used to do a whole lot more than that, so I don't think that counts as 
a valid example.
tick_length is overwritten at every timer and modifying it would have 

It depends on the architecture, but it's effects regularly executed code 
and every byte counts.

bye, Roman
--

Then make the original function an inline. With -O2 it should compile
to exactly the same thing.
--

Hi,


This would also defeat John's intention of keeping the value static.

bye, Roman
--

Well, don't mistake me for being fanatical about it. Having the values
be static is cleaner, but if its a real performance issue, then clearly
performance wins.

I do like Ray's suggestion, and think using the inline'd function is
preferred to the raw variable, as it better establishes through use if
nothing else, the read-only nature of the value outside of ntp.

thanks
-john

--

Hi,


There are two aspects, such functions tend to generate slightly larger and 

In other languages one would use private or protected for this, but we don't 
have this. I'm not too fond of an inline function, as it would mark it as 
some kind of public API, which it isn't. It's just an internal value used by 
the timekeeping code, which happens to be needed by a few source files. If 
you want to make a little more private, it would be better to move it to 
header under kernel/time/.

bye, Roman
--