hv_util: adjust system time smoothly
cover hv_util: adjust system time smoothly
(First, I have to admit I'm not a timekeeping expert ...)
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.
With this series I suggest to use do_adjtimex() to adjust time. My tests show that such method gives equally good time convergence but avoids all the drawbacks described above.
Vitaly Kuznetsov (4):
timekeeping: export do_adjtimex() to modules
hv_util: switch to using timespec64
hv_util: use do_adjtimex() to update system time
hv_util: improve time adjustment accuracy by disabling interrupts
drivers/hv/hv_util.c | 33 +++++++++++++++++++++++++++++---- kernel/time/timekeeping.c | 1 + 2 files changed, 30 insertions(+), 4 deletions(-)
Signature · 1 line
2.9.3
PATCH 1/4 timekeeping: export do_adjtimex() to modules
While do_adjtimex() is available to userspace via adjtimex syscall it is not available to modules which may want to implement in-kernel 'NTP clients'. Hyper-V hv_utils is going to be the first one.
--- kernel/time/timekeeping.c | 1 + 1 file changed, 1 insertion(+)
kernel/time/timekeeping.c +1 -0
@@ -2312,6 +2312,7 @@ int do_adjtimex(struct timex *txc)return ret; } +EXPORT_SYMBOL_GPL (do_adjtimex);#ifdef CONFIG_NTP_PPS /**
Signature · 1 line
2.9.3
PATCH 2/4 hv_util: switch to using timespec64
In reply to Vitaly Kuznetsovdo_settimeofday() is deprecated, use do_settimeofday64() instead.
--- drivers/hv/hv_util.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/hv/hv_util.c +3 -3
@@ -184,7 +184,7 @@ static void hv_set_host_time(struct work_struct *work)struct adj_time_work *wrk;s64 host_tns;u64 newtime; -struct timespec host_ts; +struct timespec64 host_ts; wrk = container_of(work, struct adj_time_work, work);
@@ -203,9 +203,9 @@ static void hv_set_host_time(struct work_struct *work) newtime += (current_tick - wrk->ref_time); } host_tns = (newtime -WLTIMEDELTA ) *100 ; - host_ts =ns_to_timespec (host_tns); + host_ts =ns_to_timespec64 (host_tns); -do_settimeofday (&host_ts); +do_settimeofday64 (&host_ts); }/*
Signature · 1 line
2.9.3
PATCH 3/4 hv_util: use do_adjtimex() to update system time
In reply to Vitaly KuznetsovWith 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().
--- drivers/hv/hv_util.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)
drivers/hv/hv_util.c +22 -3
@@ -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 ; + } + + 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; +do_adjtimex (&txc); }/*
Signature · 1 line
2.9.3
RE: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
Quoted 10 lines
-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Monday, January 2, 2017 11:41 AM
To: [email protected]
Cc: [email protected]; KY Srinivasan <[email protected]>; Haiyang Zhang <[email protected]>; John Stultz <[email protected]>; Thomas Gleixner <[email protected]>; Alex Ng (LIS) <[email protected]>
Subject: [PATCH 3/4]hv_util: usedo_adjtimex()to update system time
Quoted 12 lines
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.
Quoted 32 lines
Instead of calling
do_settimeofday64()we can pretend being an NTP client and usedo_adjtimex().Signed-off-by Vitaly Kuznetsov <[email protected]>--- drivers/hv/hv_util.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)drivers/hv/hv_util.c index renamed from drivers/hv/hv_util.c +0
-094719eb..4c0fbb0 100644
drivers/hv/hv_util.c +4
-3@@ -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);
drivers/hv/hv_util.c static void hv_set_host_time(struct work_struct · line 220
Quoted 10 lines
drivers/hv/hv_util.c +10
-0+ 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).
drivers/hv/hv_util.c static void hv_set_host_time(struct work_struct *work) · line 224
Quoted 6 lines
drivers/hv/hv_util.c +2
-0+ + txc.modes = ADJ_TICK | ADJ_FREQUENCY | ADJ_OFFSET |
ADJ_NANO|drivers/hv/hv_util.c +3
-0+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?
drivers/hv/hv_util.c static void hv_set_host_time(struct work_struct *work) · line 227
Quoted 3 lines
drivers/hv/hv_util.c +3
-0+ 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.
Quoted 5 lines
}
/*
--
2.9.3
Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
Quoted 23 lines · Alex Ng (LIS)
Quoted 10 lines
-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Monday, January 2, 2017 11:41 AM
To: [email protected]
Cc: [email protected]; KY Srinivasan <[email protected]>; Haiyang Zhang <[email protected]>; John Stultz <[email protected]>; Thomas Gleixner <[email protected]>; Alex Ng (LIS) <[email protected]>
Subject: [PATCH 3/4]hv_util: usedo_adjtimex()to update system timeQuoted 12 lines
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.
Quoted 5 lines
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?
Quoted 39 lines
Quoted 39 lines
Instead of calling
do_settimeofday64()we can pretend being an NTP client and usedo_adjtimex().Signed-off-by Vitaly Kuznetsov <[email protected]>--- drivers/hv/hv_util.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)drivers/hv/hv_util.c index renamed from drivers/hv/hv_util.c +0
-094719eb..4c0fbb0 100644
drivers/hv/hv_util.c +11
-3@@ -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 ) {
Quoted 8 lines
drivers/hv/hv_util.c static void hv_set_host_time(struct work_struct *work) · line 219
Quoted 3 lines
drivers/hv/hv_util.c +3
-0+do_settimeofday64 (&host_ts); +return ; + }We should also call
do_settimeofday64()if the host sends flagICTIMESYNCFLAG_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?
Quoted 10 lines
drivers/hv/hv_util.c static void hv_set_host_time(struct work_struct *work) · line 224
Quoted 7 lines
drivers/hv/hv_util.c +2
-0+ + txc.modes = ADJ_TICK | ADJ_FREQUENCY | ADJ_OFFSET |
ADJ_NANO|drivers/hv/hv_util.c +3
-0+ADJ_STATUS ; + txc.tick =TICK_USEC ; + txc.freq =0 ;I'm not familiar with the
ADJ_FREQUENCYflag. 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).
Quoted 7 lines
drivers/hv/hv_util.c static void hv_set_host_time(struct work_struct *work) · line 227
Quoted 4 lines
drivers/hv/hv_util.c +3
-0+ 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.
Quoted 6 lines
Quoted 6 lines
}
/*
--
2.9.3
Signature · 1 line
Vitaly
RE: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
Quoted 40 lines
-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Tuesday, January 3, 2017 4:32 AM
To: Alex Ng (LIS) <[email protected]>
Cc: [email protected]; [email protected]; KY Srinivasan <[email protected]>; Haiyang Zhang <[email protected]>; John Stultz <[email protected]>; Thomas Gleixner <[email protected]>
Subject: Re: [PATCH 3/4]hv_util: usedo_adjtimex()to update system timeQuoted 11 lines · Alex Ng (LIS)
Quoted 11 lines
-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Monday, January 2, 2017 11:41 AM
To: [email protected]
Cc: [email protected]; KY Srinivasan <[email protected]>; Haiyang Zhang <[email protected]>; John Stultz <[email protected]>; Thomas Gleixner <[email protected]>; Alex Ng
(LIS) <[email protected]>
Subject: [PATCH 3/4]hv_util: usedo_adjtimex()to update system timeQuoted 3 lines
Quoted 3 lines
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]().
Quoted 4 lines
Quoted 4 lines
While the time from the host is very accurate such adjustments may cause
issues:
- Time is jumping forward and backward, some applications maymisbehave.
Quoted 2 lines
Quoted 2 lines
- 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
Quoted 3 lines
Quoted 3 lines
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" every5
Quoted 2 lines
Quoted 1 line
seconds to the system log.
Quoted 7 lines
Quoted 5 lines
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.
Quoted 49 lines
Quoted 40 lines
Quoted 40 lines
Instead of calling
do_settimeofday64()we can pretend being an NTP client and usedo_adjtimex().Signed-off-by Vitaly Kuznetsov <[email protected]>--- drivers/hv/hv_util.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)drivers/hv/hv_util.c index renamed from drivers/hv/hv_util.c +0
-094719eb..4c0fbb0 100644
drivers/hv/hv_util.c +11
-3@@ -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 ) {Quoted 8 lines
drivers/hv/hv_util.c static void hv_set_host_time(struct work_struct *work) · line 219
Quoted 3 lines
drivers/hv/hv_util.c +3
-0+do_settimeofday64 (&host_ts); +return ; + }We should also call
do_settimeofday64()if the host sends flagICTIMESYNCFLAG_SYNC. This is a signal from host that the guest shall sync with host time immediately (often when the guest has just booted).
Quoted 2 lines
Ok, point taken, will do in v2. We don't get
ICTIMESYNCFLAG_SYNCvery often, right?
This is correct. SYNC flags are sent rarely and usually only after a guest has been resumed from a pause.
Quoted 24 lines
Quoted 10 lines
drivers/hv/hv_util.c static void hv_set_host_time(struct work_struct *work) · line 224
Quoted 8 lines
drivers/hv/hv_util.c +2
-0+ + txc.modes = ADJ_TICK | ADJ_FREQUENCY | ADJ_OFFSET |
ADJ_NANO|drivers/hv/hv_util.c +3
-0+ADJ_STATUS ; + txc.tick =TICK_USEC ; + txc.freq =0 ;I'm not familiar with the
ADJ_FREQUENCYflag. What does setting this to'zero' achieve?
Quoted 1 line
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).
Quoted 7 lines
drivers/hv/hv_util.c static void hv_set_host_time(struct work_struct *work) · line 227
Quoted 4 lines
drivers/hv/hv_util.c +3
-0+ 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.
Quoted 4 lines
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.
Quoted 9 lines
Quoted 6 lines
Quoted 6 lines
}
/*
--
2.9.3--
Vitaly
Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
Quoted 46 lines · On Tue, 3 Jan 2017 19:48:29 +0000 "Alex Ng (LIS)" <[email protected]> wrote:
Quoted 46 lines
-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Tuesday, January 3, 2017 4:32 AM
To: Alex Ng (LIS) <[email protected]>
Cc: [email protected]; [email protected]; KY Srinivasan <[email protected]>; Haiyang Zhang <[email protected]>; John Stultz <[email protected]>; Thomas Gleixner <[email protected]>
Subject: Re: [PATCH 3/4]hv_util: usedo_adjtimex()to update system timeQuoted 11 lines · Alex Ng (LIS)
Quoted 11 lines
-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Monday, January 2, 2017 11:41 AM
To: [email protected]
Cc: [email protected]; KY Srinivasan <[email protected]>; Haiyang Zhang <[email protected]>; John Stultz <[email protected]>; Thomas Gleixner <[email protected]>; Alex Ng
(LIS) <[email protected]>
Subject: [PATCH 3/4]hv_util: usedo_adjtimex()to update system timeQuoted 3 lines
Quoted 3 lines
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]().
Quoted 4 lines
Quoted 4 lines
While the time from the host is very accurate such adjustments may cause
issues:
- Time is jumping forward and backward, some applications maymisbehave.
Quoted 2 lines
Quoted 2 lines
- 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
Quoted 3 lines
Quoted 3 lines
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" every5
Quoted 7 lines
Quoted 1 line
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.
Quoted 10 lines
Quoted 1 line
Good, this can happen in parallel to my series, right?
Yes, that is correct.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Instead of calling
do_settimeofday64()we can pretend being an NTP client and usedo_adjtimex().Signed-off-by Vitaly Kuznetsov <[email protected]>
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.
Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
Quoted 4 lines · Stephen Hemminger
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...
Signature · 1 line
Vitaly
Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
Quoted 7 lines · On Mon, 09 Jan 2017 18:40:15 +0100 Vitaly Kuznetsov <[email protected]> wrote:
Quoted 4 lines · Stephen Hemminger
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.
Quoted 7 lines
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_utilto 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.
Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
Quoted 18 lines · Stephen Hemminger
Quoted 7 lines · On Mon, 09 Jan 2017 18:40:15 +0100 Vitaly Kuznetsov <[email protected]> wrote:
Quoted 4 lines · Stephen Hemminger
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.
Quoted 7 lines
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_utilto 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...
Quoted 2 lines
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.
Signature · 1 line
Vitaly
Re: [PATCH 3/4] hv_util: use do_adjtimex() to update system time
Quoted 23 lines · On Mon, 09 Jan 2017 19:14:30 +0100 Vitaly Kuznetsov <[email protected]> wrote:
Quoted 18 lines · Stephen Hemminger
Quoted 7 lines · On Mon, 09 Jan 2017 18:40:15 +0100 Vitaly Kuznetsov <[email protected]> wrote:
Quoted 4 lines · Stephen Hemminger
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.
Quoted 7 lines
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_utilto 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...Quoted 2 lines
Master NTP servers are connected to authoritative clock sources. I have no idea how that is configured, but it should be possible.
Quoted 6 lines
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.
PATCH 4/4 hv_util: improve time adjustment accuracy by disabling interrupts
In reply to Vitaly KuznetsovIf we happen to receive interrupts during hv_set_host_time() execution our adjustments may get inaccurate. Make the whole function atomic. Unfortunately, we can's call do_settimeofday64() with interrupts disabled as some cross-CPU work is being done but this call happens very rarely.
--- drivers/hv/hv_util.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/hv/hv_util.c +6 -0
@@ -186,6 +186,9 @@ static void hv_set_host_time(struct work_struct *work)u64 newtime;struct timespec64 host_ts, our_ts;struct timex txc = {0 }; +unsigned long flags; + +local_irq_save (flags); wrk = container_of(work, struct adj_time_work, work);
@@ -214,6 +217,7 @@ static void hv_set_host_time(struct work_struct *work)/* Try adjusting time by using phase adjustment if possible */ if (abs (delta) >MAXPHASE ) { +local_irq_restore (flags);do_settimeofday64 (&host_ts);return ; }
@@ -225,6 +229,8 @@ static void hv_set_host_time(struct work_struct *work) txc.status =STA_PLL ; txc.offset = delta;do_adjtimex (&txc); + +local_irq_restore (flags); }/*
Signature · 1 line
2.9.3
Re: [PATCH 4/4] hv_util: improve time adjustment accuracy by disabling interrupts
Quoted 34 lines · On Mon, 2 Jan 2017 20:41:14 +0100 Vitaly Kuznetsov <[email protected]> wrote:
If we happen to receive interrupts during
hv_set_host_time()execution our adjustments may get inaccurate. Make the whole function atomic. Unfortunately, we can's calldo_settimeofday64()with interrupts disabled as some cross-CPU work is being done but this call happens very rarely.Signed-off-by Vitaly Kuznetsov <[email protected]>--- drivers/hv/hv_util.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/hv/hv_util.c +4
-0@@ -186,6 +186,9 @@ static void hv_set_host_time(struct work_struct *work)u64 newtime;struct timespec64 host_ts, our_ts;struct timex txc = {0 }; +unsigned long flags; + +local_irq_save (flags); wrk = container_of(work, struct adj_time_work, work);@@ -214,6 +217,7 @@ static void hv_set_host_time(struct work_struct *work)/* Try adjusting time by using phase adjustment if possible */ if (abs (delta) >MAXPHASE ) { +local_irq_restore (flags);do_settimeofday64 (&host_ts);return ; }
drivers/hv/hv_util.c static void hv_set_host_time(struct work_struct *work) · line 233
Quoted 5 lines
drivers/hv/hv_util.c +2
-0@@ -225,6 +229,8 @@ static void hv_set_host_time(struct work_struct *work) txc.status =STA_PLL ; txc.offset = delta;do_adjtimex (&txc); + +local_irq_restore (flags);
Yes, it should be atomic, but local irq save/restore is not sufficient protection because it does not protect against premptible kernel. Why not a mutex? or a spinlock?
Re: [PATCH 4/4] hv_util: improve time adjustment accuracy by disabling interrupts
Quoted 30 lines · Stephen Hemminger
Quoted 27 lines · On Mon, 2 Jan 2017 20:41:14 +0100 Vitaly Kuznetsov <[email protected]> wrote:
If we happen to receive interrupts during
hv_set_host_time()execution our adjustments may get inaccurate. Make the whole function atomic. Unfortunately, we can's calldo_settimeofday64()with interrupts disabled as some cross-CPU work is being done but this call happens very rarely.Signed-off-by Vitaly Kuznetsov <[email protected]>--- drivers/hv/hv_util.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/hv/hv_util.c +3
-0@@ -186,6 +186,9 @@ static void hv_set_host_time(struct work_struct *work)u64 newtime;struct timespec64 host_ts, our_ts;struct timex txc = {0 }; +unsigned long flags; + +local_irq_save (flags); wrk = container_of(work, struct adj_time_work, work);@@ -214,6 +217,7 @@ static void hv_set_host_time(struct work_struct *work)
Quoted 15 lines
drivers/hv/hv_util.c static void hv_set_host_time(struct work_struct *work) · line 233
Quoted 12 lines
drivers/hv/hv_util.c +3
-0/* Try adjusting time by using phase adjustment if possible */ if (abs (delta) >MAXPHASE ) { +local_irq_restore (flags);do_settimeofday64 (&host_ts);return ; }@@ -225,6 +229,8 @@ static void hv_set_host_time(struct work_struct *work) txc.status =STA_PLL ; txc.offset = delta;do_adjtimex (&txc); + +local_irq_restore (flags);Yes, it should be atomic, but local irq save/restore is not sufficient protection because it does not protect against premptible kernel. Why not a mutex? or a spinlock?
I may be missing something, but:
to make preemption happen we need to either get an interrupt or call scheduling manually (directly or via preempt_enable(), local_irq_restore(),...). Interrupts are disabled here and even if something will trigger manual schedulling it won't happen as:
#define preemptible() (preempt_count() == 0 && !irqs_disabled())
I don't see a good documentation but Documentation/preempt-locking.txt says:
"PREVENTING PREEMPTION USING INTERRUPT DISABLING
It is possible to prevent a preemption event using local_irq_disable and local_irq_save. Note, when doing so, you must be very careful to not cause an event that would set need_resched and result in a preemption check. When in doubt, rely on locking or explicit preemption disabling."
Spinlock with irqs disabled (spin_lock_irqsave()) would work too but just because we're disabling interrupts. We don't need a lock here because hv_set_host_time() is called from a workqueue and double execution is impossible.
Mutex would not help at all as it is sleepable (so we may get a timer interrupt).
The point I'm trying to make is: disabling interrupts is enough to prevent other code from being executed on the same CPU in the middle of hv_set_host_time(). The only exception I see is NMIs but we don't usually get them and there is no easy way of protection.
Signature · 1 line
Vitaly