Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1551013 > unrolled thread

[PATCH v2 3/4] hv_util: use do_adjtimex() to update system time

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 3/4] hv_util: use do_adjtimex() to update system time Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-01-04 18:40 +0100
    Re: [PATCH v2 3/4] hv_util: use do_adjtimex() to update system time Stephen Hemminger <stephen@networkplumber.org> - 2017-01-04 20:20 +0100
      Re: [PATCH v2 3/4] hv_util: use do_adjtimex() to update system time Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-01-05 13:50 +0100
    Re: [PATCH v2 3/4] hv_util: use do_adjtimex() to update system time John Stultz <john.stultz@linaro.org> - 2017-01-07 02:00 +0100

#1551013 — [PATCH v2 3/4] hv_util: use do_adjtimex() to update system time

With TimeSync version 4 protocol support we started updating system time
continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
there is a time sample from the host which triggers do_settimeofday[64]().
While the time from the host is very accurate such adjustments may cause
issues:
- Time is jumping forward and backward, some applications may misbehave.
- In case an NTP client is run in parallel things may go south, e.g. when
  an NTP client tries to adjust tick/frequency with ADJ_TICK/ADJ_FREQUENCY
  the Hyper-V module will not see this changes and time will oscillate and
  never converge.
- Systemd starts annoying you by printing "Time has been changed" every 5
  seconds to the system log.

Instead of calling do_settimeofday64() we can pretend being an NTP client
and use do_adjtimex(). Do do_settimeofday64() in case the difference is too
big or ICTIMESYNCFLAG_SYNC flag was set in the request.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v1:
- do do_settimeofday64() when ICTIMESYNCFLAG_SYNC flag is present in the
  request (Alex Ng)
- add pr_debug() for the case when do_adjtimex() fails (Alex Ng)
---
 drivers/hv/hv_util.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 94719eb..7e97231 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -182,9 +182,11 @@ struct adj_time_work {
 static void hv_set_host_time(struct work_struct *work)
 {
 	struct adj_time_work	*wrk;
-	s64 host_tns;
+	s64 host_tns, our_tns, delta;
 	u64 newtime;
-	struct timespec64 host_ts;
+	struct timespec64 host_ts, our_ts;
+	struct timex txc = {0};
+	int ret;
 
 	wrk = container_of(work, struct adj_time_work, work);
 
@@ -205,7 +207,31 @@ static void hv_set_host_time(struct work_struct *work)
 	host_tns = (newtime - WLTIMEDELTA) * 100;
 	host_ts = ns_to_timespec64(host_tns);
 
-	do_settimeofday64(&host_ts);
+	getnstimeofday64(&our_ts);
+	our_tns = timespec64_to_ns(&our_ts);
+
+	/* Difference between our time and host time */
+	delta = host_tns - our_tns;
+
+	/*
+	 * Do raw do_settimeofday64() in case delta is too big or we were
+	 * ordered to sync our time by the host.
+	 */
+	if (abs(delta) > MAXPHASE || wrk->flags & ICTIMESYNCFLAG_SYNC) {
+		do_settimeofday64(&host_ts);
+		return;
+	}
+
+	txc.modes = ADJ_TICK | ADJ_FREQUENCY | ADJ_OFFSET | ADJ_NANO |
+		ADJ_STATUS;
+	txc.tick = TICK_USEC;
+	txc.freq = 0;
+	txc.status = STA_PLL;
+	txc.offset = delta;
+
+	ret = do_adjtimex(&txc);
+	if (ret)
+		pr_debug("Failed to adjust system time: %d\n", ret);
 }
 
 /*
-- 
2.9.3

[toc] | [next] | [standalone]

#1551123

On Wed,  4 Jan 2017 18:24:38 +0100
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> With TimeSync version 4 protocol support we started updating system time
> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
> there is a time sample from the host which triggers do_settimeofday[64]().
> While the time from the host is very accurate such adjustments may cause
> issues:
> - Time is jumping forward and backward, some applications may misbehave.
> - In case an NTP client is run in parallel things may go south, e.g. when
>   an NTP client tries to adjust tick/frequency with ADJ_TICK/ADJ_FREQUENCY
>   the Hyper-V module will not see this changes and time will oscillate and
>   never converge.
> - Systemd starts annoying you by printing "Time has been changed" every 5
>   seconds to the system log.
> 
> Instead of calling do_settimeofday64() we can pretend being an NTP client
> and use do_adjtimex(). Do do_settimeofday64() in case the difference is too
> big or ICTIMESYNCFLAG_SYNC flag was set in the request.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> Changes since v1:
> - do do_settimeofday64() when ICTIMESYNCFLAG_SYNC flag is present in the
>   request (Alex Ng)
> - add pr_debug() for the case when do_adjtimex() fails (Alex Ng)
> ---
>  drivers/hv/hv_util.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 94719eb..7e97231 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -182,9 +182,11 @@ struct adj_time_work {
>  static void hv_set_host_time(struct work_struct *work)
>  {
>  	struct adj_time_work	*wrk;
> -	s64 host_tns;
> +	s64 host_tns, our_tns, delta;
>  	u64 newtime;
> -	struct timespec64 host_ts;
> +	struct timespec64 host_ts, our_ts;
> +	struct timex txc = {0};
> +	int ret;
>  
>  	wrk = container_of(work, struct adj_time_work, work);
>  
> @@ -205,7 +207,31 @@ static void hv_set_host_time(struct work_struct *work)
>  	host_tns = (newtime - WLTIMEDELTA) * 100;
>  	host_ts = ns_to_timespec64(host_tns);
>  
> -	do_settimeofday64(&host_ts);
> +	getnstimeofday64(&our_ts);
> +	our_tns = timespec64_to_ns(&our_ts);
> +
> +	/* Difference between our time and host time */
> +	delta = host_tns - our_tns;

This looks correct to me.
Did you consider using ktime? It provides a cleaner abstraction for handling
nanosecond time resolution.

[toc] | [prev] | [next] | [standalone]

#1551953

Stephen Hemminger <stephen@networkplumber.org> writes:

> On Wed,  4 Jan 2017 18:24:38 +0100
> Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
>> With TimeSync version 4 protocol support we started updating system time
>> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
>> there is a time sample from the host which triggers do_settimeofday[64]().
>> While the time from the host is very accurate such adjustments may cause
>> issues:
>> - Time is jumping forward and backward, some applications may misbehave.
>> - In case an NTP client is run in parallel things may go south, e.g. when
>>   an NTP client tries to adjust tick/frequency with ADJ_TICK/ADJ_FREQUENCY
>>   the Hyper-V module will not see this changes and time will oscillate and
>>   never converge.
>> - Systemd starts annoying you by printing "Time has been changed" every 5
>>   seconds to the system log.
>> 
>> Instead of calling do_settimeofday64() we can pretend being an NTP client
>> and use do_adjtimex(). Do do_settimeofday64() in case the difference is too
>> big or ICTIMESYNCFLAG_SYNC flag was set in the request.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>> Changes since v1:
>> - do do_settimeofday64() when ICTIMESYNCFLAG_SYNC flag is present in the
>>   request (Alex Ng)
>> - add pr_debug() for the case when do_adjtimex() fails (Alex Ng)
>> ---
>>  drivers/hv/hv_util.c | 32 +++++++++++++++++++++++++++++---
>>  1 file changed, 29 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
>> index 94719eb..7e97231 100644
>> --- a/drivers/hv/hv_util.c
>> +++ b/drivers/hv/hv_util.c
>> @@ -182,9 +182,11 @@ struct adj_time_work {
>>  static void hv_set_host_time(struct work_struct *work)
>>  {
>>  	struct adj_time_work	*wrk;
>> -	s64 host_tns;
>> +	s64 host_tns, our_tns, delta;
>>  	u64 newtime;
>> -	struct timespec64 host_ts;
>> +	struct timespec64 host_ts, our_ts;
>> +	struct timex txc = {0};
>> +	int ret;
>>  
>>  	wrk = container_of(work, struct adj_time_work, work);
>>  
>> @@ -205,7 +207,31 @@ static void hv_set_host_time(struct work_struct *work)
>>  	host_tns = (newtime - WLTIMEDELTA) * 100;
>>  	host_ts = ns_to_timespec64(host_tns);
>>  
>> -	do_settimeofday64(&host_ts);
>> +	getnstimeofday64(&our_ts);
>> +	our_tns = timespec64_to_ns(&our_ts);
>> +
>> +	/* Difference between our time and host time */
>> +	delta = host_tns - our_tns;
>
> This looks correct to me.
> Did you consider using ktime? It provides a cleaner abstraction for handling
> nanosecond time resolution.

I see. While s64 should work ktime seems preferable. I'll give it a try.

-- 
  Vitaly

[toc] | [prev] | [next] | [standalone]

#1553520

On Wed, Jan 4, 2017 at 9:24 AM, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> With TimeSync version 4 protocol support we started updating system time
> continuously through the whole lifetime of Hyper-V guests. Every 5 seconds
> there is a time sample from the host which triggers do_settimeofday[64]().
> While the time from the host is very accurate such adjustments may cause
> issues:
> - Time is jumping forward and backward, some applications may misbehave.
> - In case an NTP client is run in parallel things may go south, e.g. when
>   an NTP client tries to adjust tick/frequency with ADJ_TICK/ADJ_FREQUENCY
>   the Hyper-V module will not see this changes and time will oscillate and
>   never converge.
> - Systemd starts annoying you by printing "Time has been changed" every 5
>   seconds to the system log.
>
> Instead of calling do_settimeofday64() we can pretend being an NTP client
> and use do_adjtimex(). Do do_settimeofday64() in case the difference is too
> big or ICTIMESYNCFLAG_SYNC flag was set in the request.

So how does having the guest kernel (on behalf of the host) calling
adjtimex internally interact with NTP clients running on the guest?
The kernel sort of assumes a single user of adjtimex (having multiple
clients adjusting the clock doesn't work out so well).

thanks
-john

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web