[PATCHv2] timekeeping: Update multiplier when NTP frequency is set directly

[PATCHv2] timekeeping: Update multiplier when NTP frequency is set directly

[Miroslav Lichvar]

When the NTP frequency is set directly from userspace using the
ADJ_FREQUENCY or ADJ_TICK timex mode, immediately update the
timekeeper's multiplier instead of waiting for the next tick.

This removes a hidden non-deterministic delay in setting of the
frequency and allows an extremely tight control of the system clock
with update rates close to or even exceeding the kernel HZ.

The update is limited to archs using modern timekeeping
(!ARCH_USES_GETTIMEOFFSET).

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---

Notes:
    RFC->v1:
    - added a new parameter to force the update of the timekeeper to the current
      NTP tick length only from adjtimex()
    - added timekeeping_advance() to keep the parameter local to timekeeping.c
    v1->v2:
    - replaced bool parameter with enum
    - changed timekeeping_advance() to not make any updates for TK_ADV_FREQ with
      !ARCH_USES_GETTIMEOFFSET (an alternative might be to update with zero offset)

 kernel/time/timekeeping.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 49cbceef5deb..a08ef3771ab5 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -34,6 +34,14 @@
 #define TK_MIRROR		(1 << 1)
 #define TK_CLOCK_WAS_SET	(1 << 2)
 
+enum timekeeping_adv_mode {
+	/* Update timekeeper when a tick has passed */
+	TK_ADV_TICK,
+
+	/* Update timekeeper on a direct frequency change */
+	TK_ADV_FREQ
+};
+
 /*
  * The most important data for readout fits into a single 64 byte
  * cache line.
@@ -2021,11 +2029,11 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
 	return offset;
 }
 
-/**
- * update_wall_time - Uses the current clocksource to increment the wall time
- *
+/*
+ * timekeeping_advance - Updates the timekeeper to the current time and
+ * current NTP tick length
  */
-void update_wall_time(void)
+static void timekeeping_advance(enum timekeeping_adv_mode mode)
 {
 	struct timekeeper *real_tk = &tk_core.timekeeper;
 	struct timekeeper *tk = &shadow_timekeeper;
@@ -2042,14 +2050,17 @@ void update_wall_time(void)
 
 #ifdef CONFIG_ARCH_USES_GETTIMEOFFSET
 	offset = real_tk->cycle_interval;
+
+	if (mode != TK_ADV_TICK)
+		goto out;
 #else
 	offset = clocksource_delta(tk_clock_read(&tk->tkr_mono),
 				   tk->tkr_mono.cycle_last, tk->tkr_mono.mask);
-#endif
 
 	/* Check if there's really nothing to do */
-	if (offset < real_tk->cycle_interval)
+	if (offset < real_tk->cycle_interval && mode == TK_ADV_TICK)
 		goto out;
+#endif
 
 	/* Do some additional sanity checking */
 	timekeeping_check_update(tk, offset);
@@ -2105,6 +2116,15 @@ void update_wall_time(void)
 		clock_was_set_delayed();
 }
 
+/**
+ * update_wall_time - Uses the current clocksource to increment the wall time
+ *
+ */
+void update_wall_time(void)
+{
+	timekeeping_advance(TK_ADV_TICK);
+}
+
 /**
  * getboottime64 - Return the real time of system boot.
  * @ts:		pointer to the timespec64 to be set
@@ -2332,6 +2352,10 @@ int do_adjtimex(struct timex *txc)
 	write_seqcount_end(&tk_core.seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
+	/* Update the multiplier immediately if frequency was set directly */
+	if (txc->modes & (ADJ_FREQUENCY | ADJ_TICK))
+		timekeeping_advance(TK_ADV_FREQ);
+
 	if (tai != orig_tai)
 		clock_was_set();
 
-- 
2.14.3

Re: [PATCHv2] timekeeping: Update multiplier when NTP frequency is set directly

[Thomas Gleixner]

On Mon, 4 Jun 2018, Miroslav Lichvar wrote:

> When the NTP frequency is set directly from userspace using the
> ADJ_FREQUENCY or ADJ_TICK timex mode, immediately update the
> timekeeper's multiplier instead of waiting for the next tick.
> 
> This removes a hidden non-deterministic delay in setting of the
> frequency and allows an extremely tight control of the system clock
> with update rates close to or even exceeding the kernel HZ.
> 
> The update is limited to archs using modern timekeeping
> (!ARCH_USES_GETTIMEOFFSET).

John ?????

> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
> ---
> 
> Notes:
>     RFC->v1:
>     - added a new parameter to force the update of the timekeeper to the current
>       NTP tick length only from adjtimex()
>     - added timekeeping_advance() to keep the parameter local to timekeeping.c
>     v1->v2:
>     - replaced bool parameter with enum
>     - changed timekeeping_advance() to not make any updates for TK_ADV_FREQ with
>       !ARCH_USES_GETTIMEOFFSET (an alternative might be to update with zero offset)
> 
>  kernel/time/timekeeping.c | 36 ++++++++++++++++++++++++++++++------
>  1 file changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 49cbceef5deb..a08ef3771ab5 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -34,6 +34,14 @@
>  #define TK_MIRROR		(1 << 1)
>  #define TK_CLOCK_WAS_SET	(1 << 2)
>  
> +enum timekeeping_adv_mode {
> +	/* Update timekeeper when a tick has passed */
> +	TK_ADV_TICK,
> +
> +	/* Update timekeeper on a direct frequency change */
> +	TK_ADV_FREQ
> +};
> +
>  /*
>   * The most important data for readout fits into a single 64 byte
>   * cache line.
> @@ -2021,11 +2029,11 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
>  	return offset;
>  }
>  
> -/**
> - * update_wall_time - Uses the current clocksource to increment the wall time
> - *
> +/*
> + * timekeeping_advance - Updates the timekeeper to the current time and
> + * current NTP tick length
>   */
> -void update_wall_time(void)
> +static void timekeeping_advance(enum timekeeping_adv_mode mode)
>  {
>  	struct timekeeper *real_tk = &tk_core.timekeeper;
>  	struct timekeeper *tk = &shadow_timekeeper;
> @@ -2042,14 +2050,17 @@ void update_wall_time(void)
>  
>  #ifdef CONFIG_ARCH_USES_GETTIMEOFFSET
>  	offset = real_tk->cycle_interval;
> +
> +	if (mode != TK_ADV_TICK)
> +		goto out;
>  #else
>  	offset = clocksource_delta(tk_clock_read(&tk->tkr_mono),
>  				   tk->tkr_mono.cycle_last, tk->tkr_mono.mask);
> -#endif
>  
>  	/* Check if there's really nothing to do */
> -	if (offset < real_tk->cycle_interval)
> +	if (offset < real_tk->cycle_interval && mode == TK_ADV_TICK)
>  		goto out;
> +#endif
>  
>  	/* Do some additional sanity checking */
>  	timekeeping_check_update(tk, offset);
> @@ -2105,6 +2116,15 @@ void update_wall_time(void)
>  		clock_was_set_delayed();
>  }
>  
> +/**
> + * update_wall_time - Uses the current clocksource to increment the wall time
> + *
> + */
> +void update_wall_time(void)
> +{
> +	timekeeping_advance(TK_ADV_TICK);
> +}
> +
>  /**
>   * getboottime64 - Return the real time of system boot.
>   * @ts:		pointer to the timespec64 to be set
> @@ -2332,6 +2352,10 @@ int do_adjtimex(struct timex *txc)
>  	write_seqcount_end(&tk_core.seq);
>  	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
>  
> +	/* Update the multiplier immediately if frequency was set directly */
> +	if (txc->modes & (ADJ_FREQUENCY | ADJ_TICK))
> +		timekeeping_advance(TK_ADV_FREQ);
> +
>  	if (tai != orig_tai)
>  		clock_was_set();
>  
> -- 
> 2.14.3
> 
> 

Re: [PATCHv2] timekeeping: Update multiplier when NTP frequency is set directly

[John Stultz]

On Fri, Jun 22, 2018 at 2:09 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 4 Jun 2018, Miroslav Lichvar wrote:
>
>> When the NTP frequency is set directly from userspace using the
>> ADJ_FREQUENCY or ADJ_TICK timex mode, immediately update the
>> timekeeper's multiplier instead of waiting for the next tick.
>>
>> This removes a hidden non-deterministic delay in setting of the
>> frequency and allows an extremely tight control of the system clock
>> with update rates close to or even exceeding the kernel HZ.
>>
>> The update is limited to archs using modern timekeeping
>> (!ARCH_USES_GETTIMEOFFSET).
>
> John ?????

Yea. I've got this in my toqueue list.  I'm going to take another pass
at testing and hopefully send it your way.

thanks
-john

Re: [PATCHv2] timekeeping: Update multiplier when NTP frequency is set directly

[Thomas Gleixner]

On Fri, 22 Jun 2018, John Stultz wrote:
> On Fri, Jun 22, 2018 at 2:09 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Mon, 4 Jun 2018, Miroslav Lichvar wrote:
> >
> >> When the NTP frequency is set directly from userspace using the
> >> ADJ_FREQUENCY or ADJ_TICK timex mode, immediately update the
> >> timekeeper's multiplier instead of waiting for the next tick.
> >>
> >> This removes a hidden non-deterministic delay in setting of the
> >> frequency and allows an extremely tight control of the system clock
> >> with update rates close to or even exceeding the kernel HZ.
> >>
> >> The update is limited to archs using modern timekeeping
> >> (!ARCH_USES_GETTIMEOFFSET).
> >
> > John ?????
> 
> Yea. I've got this in my toqueue list.  I'm going to take another pass
> at testing and hopefully send it your way.

No hurry. I just wanted to make sure that it didn't fall through the
cracks.

Thanks,

	tglx