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@xxxxxxxxxxxxx>
> Cc: John Stultz <john.stultz@xxxxxxxxxx>
> Cc: Richard Cochran <richardcochran@xxxxxxxxx>
> Cc: Prarit Bhargava <prarit@xxxxxxxxxx>
> Signed-off-by: Miroslav Lichvar <mlichvar@xxxxxxxxxx>
> ---
> 
> 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
> 
>