lkml.org 
[lkml]   [2017]   [Jan]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
    Date
    > -----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.

    >
    > 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).

    > +
    > + 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?

    > + 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.

    > }
    >
    > /*
    > --
    > 2.9.3

    \
     
     \ /
      Last update: 2017-01-03 00:26    [W:3.379 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site