* [PATCH 3/10] NTP: add ntp_update_frequency
@ 2005-12-21 23:21 Roman Zippel
2006-01-11 23:05 ` john stultz
0 siblings, 1 reply; 4+ messages in thread
From: Roman Zippel @ 2005-12-21 23:21 UTC (permalink / raw)
To: johnstul, linux-kernel
This introduces ntp_update_frequency and deinlines ntp_clear() (as it's
not performance critical).
It also changes how tick_nsec is calculated from tick_usec, instead of
scaling it using TICK_USEC_TO_NSEC it's simply shifted by the difference.
Since ntp doesn't change the tick value, the result in practice is the
same, but it's easier to change this into a clock parameter, which can
be calculated during boot.
Signed-off-by: Roman Zippel <zippel@linux-m68k.org>
---
include/linux/timex.h | 14 ++------------
kernel/time.c | 7 ++++---
kernel/timer.c | 28 ++++++++++++++++++++++++++--
3 files changed, 32 insertions(+), 17 deletions(-)
Index: linux-2.6-mm/include/linux/timex.h
===================================================================
--- linux-2.6-mm.orig/include/linux/timex.h 2005-12-21 12:11:56.000000000 +0100
+++ linux-2.6-mm/include/linux/timex.h 2005-12-21 12:12:00.000000000 +0100
@@ -219,18 +219,8 @@ extern long time_reftime; /* time at las
extern long time_adjust; /* The amount of adjtime left */
extern long time_next_adjust; /* Value for time_adjust at next tick */
-/**
- * ntp_clear - Clears the NTP state variables
- *
- * Must be called while holding a write on the xtime_lock
- */
-static inline void ntp_clear(void)
-{
- time_adjust = 0; /* stop active adjtime() */
- time_status |= STA_UNSYNC;
- time_maxerror = NTP_PHASE_LIMIT;
- time_esterror = NTP_PHASE_LIMIT;
-}
+extern void ntp_clear(void);
+extern void ntp_update_frequency(void);
/**
* ntp_synced - Returns 1 if the NTP status is not UNSYNC
Index: linux-2.6-mm/kernel/time.c
===================================================================
--- linux-2.6-mm.orig/kernel/time.c 2005-12-21 12:11:48.000000000 +0100
+++ linux-2.6-mm/kernel/time.c 2005-12-21 12:12:00.000000000 +0100
@@ -334,10 +334,11 @@ int do_adjtimex(struct timex *txc)
time_freq = max(time_freq, -time_tolerance);
} /* STA_PLL */
} /* txc->modes & ADJ_OFFSET */
- if (txc->modes & ADJ_TICK) {
+ if (txc->modes & ADJ_TICK)
tick_usec = txc->tick;
- tick_nsec = TICK_USEC_TO_NSEC(tick_usec);
- }
+
+ if (txc->modes & ADJ_TICK)
+ ntp_update_frequency();
} /* txc->modes */
leave: if ((time_status & (STA_UNSYNC|STA_CLOCKERR)) != 0)
result = TIME_ERROR;
Index: linux-2.6-mm/kernel/timer.c
===================================================================
--- linux-2.6-mm.orig/kernel/timer.c 2005-12-21 12:11:56.000000000 +0100
+++ linux-2.6-mm/kernel/timer.c 2005-12-21 12:12:00.000000000 +0100
@@ -551,8 +551,8 @@ found:
* Timekeeping variables
*/
unsigned long tick_usec = TICK_USEC; /* USER_HZ period (usec) */
-unsigned long tick_nsec = TICK_NSEC; /* ACTHZ period (nsec) */
-static unsigned long tick_nsec_curr = TICK_NSEC;
+unsigned long tick_nsec; /* ACTHZ period (nsec) */
+static unsigned long tick_nsec_curr;
/*
* The current time
@@ -591,6 +591,29 @@ long time_reftime; /* time at last adj
long time_adjust;
long time_next_adjust;
+/**
+ * ntp_clear - Clears the NTP state variables
+ *
+ * Must be called while holding a write on the xtime_lock
+ */
+void ntp_clear(void)
+{
+ time_adjust = 0; /* stop active adjtime() */
+ time_status |= STA_UNSYNC;
+ time_maxerror = NTP_PHASE_LIMIT;
+ time_esterror = NTP_PHASE_LIMIT;
+
+ ntp_update_frequency();
+
+ tick_nsec_curr = tick_nsec;
+}
+
+void ntp_update_frequency(void)
+{
+ tick_nsec = tick_usec * 1000;
+ tick_nsec -= NSEC_PER_SEC / HZ - TICK_NSEC;
+}
+
/*
* this routine handles the overflow of the microsecond field
*
@@ -1348,6 +1371,7 @@ static struct notifier_block __devinitda
void __init init_timers(void)
{
+ ntp_clear();
timer_cpu_notify(&timers_nb, (unsigned long)CPU_UP_PREPARE,
(void *)(long)smp_processor_id());
register_cpu_notifier(&timers_nb);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 3/10] NTP: add ntp_update_frequency
2005-12-21 23:21 [PATCH 3/10] NTP: add ntp_update_frequency Roman Zippel
@ 2006-01-11 23:05 ` john stultz
2006-01-11 23:08 ` john stultz
2006-01-12 12:28 ` Roman Zippel
0 siblings, 2 replies; 4+ messages in thread
From: john stultz @ 2006-01-11 23:05 UTC (permalink / raw)
To: Roman Zippel; +Cc: linux-kernel
On Thu, 2005-12-22 at 00:21 +0100, Roman Zippel wrote:
> This introduces ntp_update_frequency and deinlines ntp_clear() (as it's
> not performance critical).
> It also changes how tick_nsec is calculated from tick_usec, instead of
> scaling it using TICK_USEC_TO_NSEC it's simply shifted by the difference.
> Since ntp doesn't change the tick value, the result in practice is the
> same, but it's easier to change this into a clock parameter, which can
> be calculated during boot.
>
Looks ok. Few comments below.
>
> Signed-off-by: Roman Zippel <zippel@linux-m68k.org>
>
> ---
>
> include/linux/timex.h | 14 ++------------
> kernel/time.c | 7 ++++---
> kernel/timer.c | 28 ++++++++++++++++++++++++++--
> 3 files changed, 32 insertions(+), 17 deletions(-)
>
> Index: linux-2.6-mm/include/linux/timex.h
> ===================================================================
> --- linux-2.6-mm.orig/include/linux/timex.h 2005-12-21 12:11:56.000000000 +0100
> +++ linux-2.6-mm/include/linux/timex.h 2005-12-21 12:12:00.000000000 +0100
> @@ -219,18 +219,8 @@ extern long time_reftime; /* time at las
> extern long time_adjust; /* The amount of adjtime left */
> extern long time_next_adjust; /* Value for time_adjust at next tick */
>
> -/**
> - * ntp_clear - Clears the NTP state variables
> - *
> - * Must be called while holding a write on the xtime_lock
> - */
> -static inline void ntp_clear(void)
> -{
> - time_adjust = 0; /* stop active adjtime() */
> - time_status |= STA_UNSYNC;
> - time_maxerror = NTP_PHASE_LIMIT;
> - time_esterror = NTP_PHASE_LIMIT;
> -}
> +extern void ntp_clear(void);
> +extern void ntp_update_frequency(void);
Yep, that looks good.
> /**
> * ntp_synced - Returns 1 if the NTP status is not UNSYNC
> Index: linux-2.6-mm/kernel/time.c
> ===================================================================
> --- linux-2.6-mm.orig/kernel/time.c 2005-12-21 12:11:48.000000000 +0100
> +++ linux-2.6-mm/kernel/time.c 2005-12-21 12:12:00.000000000 +0100
> @@ -334,10 +334,11 @@ int do_adjtimex(struct timex *txc)
> time_freq = max(time_freq, -time_tolerance);
> } /* STA_PLL */
> } /* txc->modes & ADJ_OFFSET */
> - if (txc->modes & ADJ_TICK) {
> + if (txc->modes & ADJ_TICK)
> tick_usec = txc->tick;
> - tick_nsec = TICK_USEC_TO_NSEC(tick_usec);
> - }
> +
> + if (txc->modes & ADJ_TICK)
> + ntp_update_frequency();
Why the extra conditional instead of just adding ntp_update_frequency()
inside the braces?
> } /* txc->modes */
> leave: if ((time_status & (STA_UNSYNC|STA_CLOCKERR)) != 0)
> result = TIME_ERROR;
> Index: linux-2.6-mm/kernel/timer.c
> ===================================================================
> --- linux-2.6-mm.orig/kernel/timer.c 2005-12-21 12:11:56.000000000 +0100
> +++ linux-2.6-mm/kernel/timer.c 2005-12-21 12:12:00.000000000 +0100
> @@ -551,8 +551,8 @@ found:
> * Timekeeping variables
> */
> unsigned long tick_usec = TICK_USEC; /* USER_HZ period (usec) */
> -unsigned long tick_nsec = TICK_NSEC; /* ACTHZ period (nsec) */
> -static unsigned long tick_nsec_curr = TICK_NSEC;
> +unsigned long tick_nsec; /* ACTHZ period (nsec) */
> +static unsigned long tick_nsec_curr;
>
> /*
> * The current time
> @@ -591,6 +591,29 @@ long time_reftime; /* time at last adj
> long time_adjust;
> long time_next_adjust;
>
> +/**
> + * ntp_clear - Clears the NTP state variables
> + *
> + * Must be called while holding a write on the xtime_lock
> + */
> +void ntp_clear(void)
> +{
> + time_adjust = 0; /* stop active adjtime() */
> + time_status |= STA_UNSYNC;
> + time_maxerror = NTP_PHASE_LIMIT;
> + time_esterror = NTP_PHASE_LIMIT;
> +
> + ntp_update_frequency();
> +
> + tick_nsec_curr = tick_nsec;
> +}
> +
> +void ntp_update_frequency(void)
> +{
> + tick_nsec = tick_usec * 1000;
> + tick_nsec -= NSEC_PER_SEC / HZ - TICK_NSEC;
> +}
Could you add another "john is slow and forgetful" comment here?
> /*
> * this routine handles the overflow of the microsecond field
> *
> @@ -1348,6 +1371,7 @@ static struct notifier_block __devinitda
>
> void __init init_timers(void)
> {
> + ntp_clear();
> timer_cpu_notify(&timers_nb, (unsigned long)CPU_UP_PREPARE,
> (void *)(long)smp_processor_id());
> register_cpu_notifier(&timers_nb);
thanks
-john
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 3/10] NTP: add ntp_update_frequency
2006-01-11 23:05 ` john stultz
@ 2006-01-11 23:08 ` john stultz
2006-01-12 12:28 ` Roman Zippel
1 sibling, 0 replies; 4+ messages in thread
From: john stultz @ 2006-01-11 23:08 UTC (permalink / raw)
To: Roman Zippel; +Cc: linux-kernel
On Wed, 2006-01-11 at 15:05 -0800, john stultz wrote:
> On Thu, 2005-12-22 at 00:21 +0100, Roman Zippel wrote:
> > This introduces ntp_update_frequency and deinlines ntp_clear() (as it's
> > not performance critical).
> > It also changes how tick_nsec is calculated from tick_usec, instead of
> > scaling it using TICK_USEC_TO_NSEC it's simply shifted by the difference.
> > Since ntp doesn't change the tick value, the result in practice is the
> > same, but it's easier to change this into a clock parameter, which can
> > be calculated during boot.
> >
One last thing, shouldn't this patch kill TICK_USEC_TO_NSEC ?
thanks
-john
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 3/10] NTP: add ntp_update_frequency
2006-01-11 23:05 ` john stultz
2006-01-11 23:08 ` john stultz
@ 2006-01-12 12:28 ` Roman Zippel
1 sibling, 0 replies; 4+ messages in thread
From: Roman Zippel @ 2006-01-12 12:28 UTC (permalink / raw)
To: john stultz; +Cc: linux-kernel
Hi,
On Wed, 11 Jan 2006, john stultz wrote:
> > This introduces ntp_update_frequency and deinlines ntp_clear() (as it's
> > not performance critical).
> > It also changes how tick_nsec is calculated from tick_usec, instead of
> > scaling it using TICK_USEC_TO_NSEC it's simply shifted by the difference.
> > Since ntp doesn't change the tick value, the result in practice is the
> > same, but it's easier to change this into a clock parameter, which can
> > be calculated during boot.
> >
>
> One last thing, shouldn't this patch kill TICK_USEC_TO_NSEC ?
If it's the only user it could be removed, but jiffies.h can still be
cleaned up in a separate pass.
> > @@ -334,10 +334,11 @@ int do_adjtimex(struct timex *txc)
> > time_freq = max(time_freq, -time_tolerance);
> > } /* STA_PLL */
> > } /* txc->modes & ADJ_OFFSET */
> > - if (txc->modes & ADJ_TICK) {
> > + if (txc->modes & ADJ_TICK)
> > tick_usec = txc->tick;
> > - tick_nsec = TICK_USEC_TO_NSEC(tick_usec);
> > - }
> > +
> > + if (txc->modes & ADJ_TICK)
> > + ntp_update_frequency();
>
> Why the extra conditional instead of just adding ntp_update_frequency()
> inside the braces?
This changes in the next patch. :)
> > +void ntp_update_frequency(void)
> > +{
> > + tick_nsec = tick_usec * 1000;
> > + tick_nsec -= NSEC_PER_SEC / HZ - TICK_NSEC;
> > +}
>
> Could you add another "john is slow and forgetful" comment here?
Ok.
bye, Roman
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2006-01-12 12:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-12-21 23:21 [PATCH 3/10] NTP: add ntp_update_frequency Roman Zippel
2006-01-11 23:05 ` john stultz
2006-01-11 23:08 ` john stultz
2006-01-12 12:28 ` Roman Zippel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).