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

Groups > linux.kernel > #1550137 > unrolled thread

RE: [PATCH 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

  RE: [PATCH 3/4] hv_util: use do_adjtimex() to update system time "Alex Ng (LIS)" <alexng@microsoft.com> - 2017-01-03 20:50 +0100
    Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time Stephen Hemminger <stephen@networkplumber.org> - 2017-01-09 18:30 +0100
      Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-01-09 18:50 +0100
        Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time Stephen Hemminger <stephen@networkplumber.org> - 2017-01-09 19:00 +0100
          Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-01-09 19:20 +0100
            Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time Stephen Hemminger <stephen@networkplumber.org> - 2017-01-09 19:40 +0100

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

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Tuesday, January 3, 2017 4:32 AM
> To: Alex Ng (LIS) <alexng@microsoft.com>
> Cc: devel@linuxdriverproject.org; linux-kernel@vger.kernel.org; KY
> Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>;
> John Stultz <john.stultz@linaro.org>; Thomas Gleixner <tglx@linutronix.de>
> Subject: Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
> 
> "Alex Ng (LIS)" <alexng@microsoft.com> writes:
> 
> >> -----Original Message-----
> >> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> >> Sent: Monday, January 2, 2017 11:41 AM
> >> To: devel@linuxdriverproject.org
> >> Cc: linux-kernel@vger.kernel.org; KY Srinivasan <kys@microsoft.com>;
> >> Haiyang Zhang <haiyangz@microsoft.com>; John Stultz
> >> <john.stultz@linaro.org>; Thomas Gleixner <tglx@linutronix.de>; Alex
> >> Ng
> >> (LIS) <alexng@microsoft.com>
> >> Subject: [PATCH 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.
> >
> > These are all good points. I am working on a patch to address point 2.
> > It will allow new TimeSync behavior to be disabled even if the
> > TimeSync IC is enabled from the host. This can be set to prevent
> > TimeSync IC from interfering with NTP client.
> >
> 
> Good, this can happen in parallel to my series, right?

Yes, that is correct.

> 
> >>
> >> Instead of calling do_settimeofday64() we can pretend being an NTP
> >> client and use do_adjtimex().
> >>
> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> ---
> >>  drivers/hv/hv_util.c | 25 ++++++++++++++++++++++---
> >>  1 file changed, 22 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c index
> >> 94719eb..4c0fbb0 100644
> >> --- a/drivers/hv/hv_util.c
> >> +++ b/drivers/hv/hv_util.c
> >> @@ -182,9 +182,10 @@ 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};
> >>
> >>  	wrk = container_of(work, struct adj_time_work, work);
> >>
> >> @@ -205,7 +206,25 @@ 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;
> >> +
> >> +	/* Try adjusting time by using phase adjustment if possible */
> >> +	if (abs(delta) > MAXPHASE) {
> >> +		do_settimeofday64(&host_ts);
> >> +		return;
> >> +	}
> >
> > We should also call do_settimeofday64() if the host sends flag
> > ICTIMESYNCFLAG_SYNC. This is a signal from host that the guest shall
> > sync with host time immediately (often when the guest has just
> > booted).
> 
> Ok, point taken, will do in v2. We don't get ICTIMESYNCFLAG_SYNC very
> often, right?

This is correct. SYNC flags are sent rarely and usually only after a guest has
been resumed from a pause.

> 
> >
> >> +
> >> +	txc.modes = ADJ_TICK | ADJ_FREQUENCY | ADJ_OFFSET |
> >> ADJ_NANO |
> >> +		ADJ_STATUS;
> >> +	txc.tick = TICK_USEC;
> >> +	txc.freq = 0;
> >
> > I'm not familiar with the ADJ_FREQUENCY flag. What does setting this to
> 'zero' achieve?
> > Are there any side-effects from doing this?
> 
> Zero means no frequency adjustment required (we reset it in case it was
> previously made by an NTP client).
> 
> >
> >> +	txc.status = STA_PLL;
> >> +	txc.offset = delta;
> >> +	do_adjtimex(&txc);
> >
> > Might be a good idea to handle the return code from do_adjtimex() and
> > log something in case of error.
> 
> I can add a debug message here but as this is a regular action we don't want
> to get a flood of messages in case this fails permanently. I'd avoid printing
> info messages here.
> 

Agree. A debug level message is reasonable.

> >
> >>  }
> >>
> >>  /*
> >> --
> >> 2.9.3
> 
> --
>   Vitaly

[toc] | [next] | [standalone]

#1554526

On Tue, 3 Jan 2017 19:48:29 +0000
"Alex Ng (LIS)" <alexng@microsoft.com> wrote:

> > -----Original Message-----
> > From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> > Sent: Tuesday, January 3, 2017 4:32 AM
> > To: Alex Ng (LIS) <alexng@microsoft.com>
> > Cc: devel@linuxdriverproject.org; linux-kernel@vger.kernel.org; KY
> > Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>;
> > John Stultz <john.stultz@linaro.org>; Thomas Gleixner <tglx@linutronix.de>
> > Subject: Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
> > 
> > "Alex Ng (LIS)" <alexng@microsoft.com> writes:
> >   
> > >> -----Original Message-----
> > >> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> > >> Sent: Monday, January 2, 2017 11:41 AM
> > >> To: devel@linuxdriverproject.org
> > >> Cc: linux-kernel@vger.kernel.org; KY Srinivasan <kys@microsoft.com>;
> > >> Haiyang Zhang <haiyangz@microsoft.com>; John Stultz
> > >> <john.stultz@linaro.org>; Thomas Gleixner <tglx@linutronix.de>; Alex
> > >> Ng
> > >> (LIS) <alexng@microsoft.com>
> > >> Subject: [PATCH 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.  
> > >
> > > These are all good points. I am working on a patch to address point 2.
> > > It will allow new TimeSync behavior to be disabled even if the
> > > TimeSync IC is enabled from the host. This can be set to prevent
> > > TimeSync IC from interfering with NTP client.
> > >  
> > 
> > Good, this can happen in parallel to my series, right?  
> 
> Yes, that is correct.
> 
> >   
> > >>
> > >> Instead of calling do_settimeofday64() we can pretend being an NTP
> > >> client and use do_adjtimex().
> > >>
> > >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

An alternative would be for hyper-v util to provide a clocksource device and
let NTP manage the adjustment. The advantage of this would be HV util not fighting
with NTP, and using standard API's. The downside would be the complexity of configuring
NTP, and difficulty of writing a clock source pseudo device.

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

#1554548

Stephen Hemminger <stephen@networkplumber.org> writes:

> An alternative would be for hyper-v util to provide a clocksource device and
> let NTP manage the adjustment. The advantage of this would be HV util not fighting
> with NTP, and using standard API's. The downside would be the complexity of configuring
> NTP, and difficulty of writing a clock source pseudo device.

Yes, I see this option. But as I wrote to John I'm afraid we'll have to
come up with a custom interface from hv_util to userspace which no NTP
server will want to support (because, first of all, it's not about
'network time' any more). We can write our own daemon which will read
from this interface and do adjtimex but in this case I don't see much
value in this data traveling from kernel to userspace and back...

-- 
  Vitaly

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

#1554553

On Mon, 09 Jan 2017 18:40:15 +0100
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> Stephen Hemminger <stephen@networkplumber.org> writes:
> 
> > An alternative would be for hyper-v util to provide a clocksource device and
> > let NTP manage the adjustment. The advantage of this would be HV util not fighting
> > with NTP, and using standard API's. The downside would be the complexity of configuring
> > NTP, and difficulty of writing a clock source pseudo device.  
> 
> Yes, I see this option. But as I wrote to John I'm afraid we'll have to
> come up with a custom interface from hv_util to userspace which no NTP
> server will want to support (because, first of all, it's not about
> 'network time' any more). We can write our own daemon which will read
> from this interface and do adjtimex but in this case I don't see much
> value in this data traveling from kernel to userspace and back...
> 

Master NTP servers are connected to authoritative clock sources. I have no idea how
that is configured, but it should be possible.

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

#1554575

Stephen Hemminger <stephen@networkplumber.org> writes:

> On Mon, 09 Jan 2017 18:40:15 +0100
> Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
>> Stephen Hemminger <stephen@networkplumber.org> writes:
>> 
>> > An alternative would be for hyper-v util to provide a clocksource device and
>> > let NTP manage the adjustment. The advantage of this would be HV util not fighting
>> > with NTP, and using standard API's. The downside would be the complexity of configuring
>> > NTP, and difficulty of writing a clock source pseudo device.  
>> 
>> Yes, I see this option. But as I wrote to John I'm afraid we'll have to
>> come up with a custom interface from hv_util to userspace which no NTP
>> server will want to support (because, first of all, it's not about
>> 'network time' any more). We can write our own daemon which will read
>> from this interface and do adjtimex but in this case I don't see much
>> value in this data traveling from kernel to userspace and back...
>> 
>
> Master NTP servers are connected to authoritative clock sources. I have no idea how
> that is configured, but it should be possible.

As far as I understand these servers are connected to GPS receivers or
something like that and we can probably pretend being one but I'm not
sure that the TimeSync v4 protocol fits there, we'll probably lose the
precision - currently we calculate the delta in a small kernel function
with interrupts disabled, in my tests I see it floating around several
hundred - few thousand nanoseconds from host's time.

-- 
  Vitaly

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

#1554595

On Mon, 09 Jan 2017 19:14:30 +0100
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> Stephen Hemminger <stephen@networkplumber.org> writes:
> 
> > On Mon, 09 Jan 2017 18:40:15 +0100
> > Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> >  
> >> Stephen Hemminger <stephen@networkplumber.org> writes:
> >>   
> >> > An alternative would be for hyper-v util to provide a clocksource device and
> >> > let NTP manage the adjustment. The advantage of this would be HV util not fighting
> >> > with NTP, and using standard API's. The downside would be the complexity of configuring
> >> > NTP, and difficulty of writing a clock source pseudo device.    
> >> 
> >> Yes, I see this option. But as I wrote to John I'm afraid we'll have to
> >> come up with a custom interface from hv_util to userspace which no NTP
> >> server will want to support (because, first of all, it's not about
> >> 'network time' any more). We can write our own daemon which will read
> >> from this interface and do adjtimex but in this case I don't see much
> >> value in this data traveling from kernel to userspace and back...
> >>   
> >
> > Master NTP servers are connected to authoritative clock sources. I have no idea how
> > that is configured, but it should be possible.  
> 
> As far as I understand these servers are connected to GPS receivers or
> something like that and we can probably pretend being one but I'm not
> sure that the TimeSync v4 protocol fits there, we'll probably lose the
> precision - currently we calculate the delta in a small kernel function
> with interrupts disabled, in my tests I see it floating around several
> hundred - few thousand nanoseconds from host's time.

My understanding is that NTP doesn't work very well at small time intervals.
Probably the ideal solution is something where kernel corrects time but
there is also way to communicate to NTP server that the kernel time is being maintained by
other entitity.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web