[systemd-devel] [PATCH] timedated: when performing "SetTime" compensate for program lag
--- TODO | 2 -- src/timedate/timedated.c | 14 ++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/TODO b/TODO index 68b0af6..7b93404 100644 --- a/TODO +++ b/TODO @@ -190,8 +190,6 @@ Features: * we should try harder to collapse start jobs for swaps that end up being the same: http://lists.freedesktop.org/archives/systemd-devel/2014-November/025359.html -* timedated should compensate on SetTime for the time spent in polkit - * figure out when we can use the coarse timers * sd-resolve: drop res_query wrapping, people should call via the bus to resolved instead diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 753c3d1..7948bfa 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -529,6 +529,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu Context *c = userdata; int64_t utc; struct timespec ts; +usec_t start, ready; struct tm* tm; int r; @@ -569,6 +570,19 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu if (r == 0) return 1; +/* adjust ts for time spent in program */ +r = sd_bus_message_get_monotonic_usec(m, &start); +if (r < 0) { +/* we only get this data if we are using kdbus */ +if (r == -ENODATA) +goto nodata; + +return r; +} +ready = now(CLOCK_MONOTONIC); +timespec_store(&ts, timespec_load(&ts) + (ready - start)); +nodata: + /* Set system clock */ if (clock_settime(CLOCK_REALTIME, &ts) < 0) { log_error_errno(errno, "Failed to set local time: %m"); -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] timedated: when performing "SetTime" compensate for program lag
The start time could be moved back a little bit by using kdbus timestamps, but I'm guessing that that amount of time is insignificant. --- TODO | 2 -- src/timedate/timedated.c | 7 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/TODO b/TODO index 93dfa60..88055d3 100644 --- a/TODO +++ b/TODO @@ -190,8 +190,6 @@ Features: * we should try harder to collapse start jobs for swaps that end up being the same: http://lists.freedesktop.org/archives/systemd-devel/2014-November/025359.html -* timedated should compensate on SetTime for the time spent in polkit - * figure out when we can use the coarse timers * sd-resolve: drop res_query wrapping, people should call via the bus to resolved instead diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 753c3d1..603e155 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -529,6 +529,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu Context *c = userdata; int64_t utc; struct timespec ts; +usec_t start, ready; struct tm* tm; int r; @@ -536,6 +537,8 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu assert(m); assert(c); +start = now(CLOCK_MONOTONIC); + if (c->use_ntp) return sd_bus_error_setf(error, BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED, "Automatic time synchronization is enabled"); @@ -569,6 +572,10 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu if (r == 0) return 1; +/* adjust ts for time spent in polkit */ +ready = now(CLOCK_MONOTONIC); +timespec_store(&ts, timespec_load(&ts) + (ready - start)); + /* Set system clock */ if (clock_settime(CLOCK_REALTIME, &ts) < 0) { log_error_errno(errno, "Failed to set local time: %m"); -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] timedated: when performing "SetTime" compensate for program lag
I can't test this as kdbus doesn't build against Linux 3.19 --- TODO | 2 -- src/timedate/timedated.c | 14 ++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/TODO b/TODO index 93dfa60..88055d3 100644 --- a/TODO +++ b/TODO @@ -190,8 +190,6 @@ Features: * we should try harder to collapse start jobs for swaps that end up being the same: http://lists.freedesktop.org/archives/systemd-devel/2014-November/025359.html -* timedated should compensate on SetTime for the time spent in polkit - * figure out when we can use the coarse timers * sd-resolve: drop res_query wrapping, people should call via the bus to resolved instead diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 753c3d1..1287038 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -529,6 +529,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu Context *c = userdata; int64_t utc; struct timespec ts; +usec_t start, ready; struct tm* tm; int r; @@ -569,6 +570,17 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu if (r == 0) return 1; +/* adjust ts for time spent in program */ +r = sd_bus_message_get_monotonic_usec(m, &start); +if (r == -ENODATA) +continue; +else if (r < 0) +return r; +else { +ready = now(CLOCK_MONOTONIC); +timespec_store(&ts, timespec_load(&ts) + (ready - start)); +} + /* Set system clock */ if (clock_settime(CLOCK_REALTIME, &ts) < 0) { log_error_errno(errno, "Failed to set local time: %m"); @@ -702,6 +714,8 @@ int main(int argc, char *argv[]) { if (r < 0) goto finish; +(void)sd_bus_negotiate_timestamp(bus, true); + r = context_read_data(&context); if (r < 0) { log_error_errno(r, "Failed to read time zone data: %m"); -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] timedated: when performing "SetTime" compensate for program lag
On Sat, Feb 14, 2015 at 12:34 AM, Shawn Landden wrote: > --- > TODO | 2 -- > src/timedate/timedated.c | 14 ++ > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/TODO b/TODO > index 68b0af6..7b93404 100644 > --- a/TODO > +++ b/TODO > @@ -190,8 +190,6 @@ Features: > * we should try harder to collapse start jobs for swaps that end up being > the same: > > http://lists.freedesktop.org/archives/systemd-devel/2014-November/025359.html > > -* timedated should compensate on SetTime for the time spent in polkit > - > * figure out when we can use the coarse timers > > * sd-resolve: drop res_query wrapping, people should call via the bus to > resolved instead > diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c > index 753c3d1..7948bfa 100644 > --- a/src/timedate/timedated.c > +++ b/src/timedate/timedated.c > @@ -529,6 +529,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message > *m, void *userdata, sd_bu > Context *c = userdata; > int64_t utc; > struct timespec ts; > +usec_t start, ready; > struct tm* tm; > int r; > > @@ -569,6 +570,19 @@ static int method_set_time(sd_bus *bus, sd_bus_message > *m, void *userdata, sd_bu > if (r == 0) > return 1; > > +/* adjust ts for time spent in program */ > +r = sd_bus_message_get_monotonic_usec(m, &start); > +if (r < 0) { Minor nit: you can avoid the goto with if (r < 0 && r != -ENODATA) return r; else { ready = now(CLOCK_MONOTONIC); timespec_store(&ts, timespec_load(&ts) + (ready - start)); } ? > +/* we only get this data if we are using kdbus */ > +if (r == -ENODATA) > +goto nodata; > + > +return r; > +} > +ready = now(CLOCK_MONOTONIC); > +timespec_store(&ts, timespec_load(&ts) + (ready - start)); > +nodata: > + > /* Set system clock */ > if (clock_settime(CLOCK_REALTIME, &ts) < 0) { > log_error_errno(errno, "Failed to set local time: %m"); > -- > 2.2.1.209.g41e5f3a > > ___ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] timedated: when performing "SetTime" compensate for program lag
On Fri, 13.02.15 15:34, Shawn Landden (sh...@churchofgit.com) wrote: > --- > TODO | 2 -- > src/timedate/timedated.c | 14 ++ > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/TODO b/TODO > index 68b0af6..7b93404 100644 > --- a/TODO > +++ b/TODO > @@ -190,8 +190,6 @@ Features: > * we should try harder to collapse start jobs for swaps that end up being > the same: > > http://lists.freedesktop.org/archives/systemd-devel/2014-November/025359.html > > -* timedated should compensate on SetTime for the time spent in polkit > - > * figure out when we can use the coarse timers > > * sd-resolve: drop res_query wrapping, people should call via the bus to > resolved instead > diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c > index 753c3d1..7948bfa 100644 > --- a/src/timedate/timedated.c > +++ b/src/timedate/timedated.c > @@ -529,6 +529,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message > *m, void *userdata, sd_bu > Context *c = userdata; > int64_t utc; > struct timespec ts; > +usec_t start, ready; > struct tm* tm; > int r; > > @@ -569,6 +570,19 @@ static int method_set_time(sd_bus *bus, sd_bus_message > *m, void *userdata, sd_bu > if (r == 0) > return 1; > > +/* adjust ts for time spent in program */ > +r = sd_bus_message_get_monotonic_usec(m, &start); > +if (r < 0) { > +/* we only get this data if we are using kdbus */ > +if (r == -ENODATA) > +goto nodata; > + > +return r; > +} > +ready = now(CLOCK_MONOTONIC); > +timespec_store(&ts, timespec_load(&ts) + (ready - start)); > +nodata: > + It's ok to use "goto" for error cleanups, but in thise case this seems unnecessary... Also, to get timestamps you need to explicitly request them when setting up the bus connection with sd_bus_negotiate_timestamp(), otherwise messages will never carry timestamps. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] timedated: when performing "SetTime" compensate for program lag
Hi On Mon, Feb 16, 2015 at 8:47 PM, Shawn Landden wrote: > I can't test this as kdbus doesn't build against Linux 3.19 Ugh? kdbus should build fine against 3.19. We pushed the required fixes last week as 3.19 was released. Btw., I now got this patch twice with different content, but same subject. I guess this patch I'm replying to is the wrong one, right? Maybe add a "v2" in the subject next time, given that the fdo mail-delivery dates are pretty random right now ;) Thanks David > --- > TODO | 2 -- > src/timedate/timedated.c | 14 ++ > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/TODO b/TODO > index 93dfa60..88055d3 100644 > --- a/TODO > +++ b/TODO > @@ -190,8 +190,6 @@ Features: > * we should try harder to collapse start jobs for swaps that end up being > the same: > > http://lists.freedesktop.org/archives/systemd-devel/2014-November/025359.html > > -* timedated should compensate on SetTime for the time spent in polkit > - > * figure out when we can use the coarse timers > > * sd-resolve: drop res_query wrapping, people should call via the bus to > resolved instead > diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c > index 753c3d1..1287038 100644 > --- a/src/timedate/timedated.c > +++ b/src/timedate/timedated.c > @@ -529,6 +529,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message > *m, void *userdata, sd_bu > Context *c = userdata; > int64_t utc; > struct timespec ts; > +usec_t start, ready; > struct tm* tm; > int r; > > @@ -569,6 +570,17 @@ static int method_set_time(sd_bus *bus, sd_bus_message > *m, void *userdata, sd_bu > if (r == 0) > return 1; > > +/* adjust ts for time spent in program */ > +r = sd_bus_message_get_monotonic_usec(m, &start); > +if (r == -ENODATA) > +continue; > +else if (r < 0) > +return r; > +else { > +ready = now(CLOCK_MONOTONIC); > +timespec_store(&ts, timespec_load(&ts) + (ready - start)); > +} > + > /* Set system clock */ > if (clock_settime(CLOCK_REALTIME, &ts) < 0) { > log_error_errno(errno, "Failed to set local time: %m"); > @@ -702,6 +714,8 @@ int main(int argc, char *argv[]) { > if (r < 0) > goto finish; > > +(void)sd_bus_negotiate_timestamp(bus, true); > + > r = context_read_data(&context); > if (r < 0) { > log_error_errno(r, "Failed to read time zone data: %m"); > -- > 2.2.1.209.g41e5f3a > > ___ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] timedated: when performing "SetTime" compensate for program lag
Hi On Mon, Feb 16, 2015 at 8:47 PM, Shawn Landden wrote: > I can't test this as kdbus doesn't build against Linux 3.19 > --- > TODO | 2 -- > src/timedate/timedated.c | 14 ++ > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/TODO b/TODO > index 93dfa60..88055d3 100644 > --- a/TODO > +++ b/TODO > @@ -190,8 +190,6 @@ Features: > * we should try harder to collapse start jobs for swaps that end up being > the same: > > http://lists.freedesktop.org/archives/systemd-devel/2014-November/025359.html > > -* timedated should compensate on SetTime for the time spent in polkit > - > * figure out when we can use the coarse timers > > * sd-resolve: drop res_query wrapping, people should call via the bus to > resolved instead > diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c > index 753c3d1..1287038 100644 > --- a/src/timedate/timedated.c > +++ b/src/timedate/timedated.c > @@ -529,6 +529,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message > *m, void *userdata, sd_bu > Context *c = userdata; > int64_t utc; > struct timespec ts; > +usec_t start, ready; > struct tm* tm; > int r; > > @@ -569,6 +570,17 @@ static int method_set_time(sd_bus *bus, sd_bus_message > *m, void *userdata, sd_bu > if (r == 0) > return 1; > > +/* adjust ts for time spent in program */ > +r = sd_bus_message_get_monotonic_usec(m, &start); > +if (r == -ENODATA) > +continue; This does not compile (spurious 'continue'). I went ahead and fixed it, simplified the code a bit and applied it. I hope you don't mind. Applied, thanks! David > +else if (r < 0) > +return r; > +else { > +ready = now(CLOCK_MONOTONIC); > +timespec_store(&ts, timespec_load(&ts) + (ready - start)); > +} > + > /* Set system clock */ > if (clock_settime(CLOCK_REALTIME, &ts) < 0) { > log_error_errno(errno, "Failed to set local time: %m"); > @@ -702,6 +714,8 @@ int main(int argc, char *argv[]) { > if (r < 0) > goto finish; > > +(void)sd_bus_negotiate_timestamp(bus, true); > + > r = context_read_data(&context); > if (r < 0) { > log_error_errno(r, "Failed to read time zone data: %m"); > -- > 2.2.1.209.g41e5f3a > > ___ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel