From 2dd79846ddd5f31aaf178eb3457677139e38fac0 Mon Sep 17 00:00:00 2001 From: "Peter A. Bigot" Date: Mon, 9 Apr 2018 13:39:16 -0500 Subject: [PATCH] time-wait-sync: use watchfile to coordinate with timesyncd Systems that have an accurate real-time clock may have an initial unsynchronized time that is close enough to the synchronized time that the final adjustment doesn't trigger a waking "clock set" event. Have timesyncd touch a file in its runtime directory as a secondary signal for synchronization. Continue to support the timerfd-based trigger as a sufficient condition when the watchfile is not present. Closes issue #8683 --- man/systemd-time-wait-sync.service.xml | 30 +++--- man/systemd-timesyncd.service.xml | 14 ++- src/time-wait-sync/time-wait-sync.c | 162 ++++++++++++++++++++++++++------- src/timesync/timesyncd-manager.c | 1 + units/systemd-timesyncd.service.in | 1 + 5 files changed, 160 insertions(+), 48 deletions(-) diff --git a/man/systemd-time-wait-sync.service.xml b/man/systemd-time-wait-sync.service.xml index a707102..d0c861f 100644 --- a/man/systemd-time-wait-sync.service.xml +++ b/man/systemd-time-wait-sync.service.xml @@ -46,24 +46,30 @@ Description systemd-time-wait-sync is a system service that delays the start of units that depend on - time-sync.target until systemd-timesyncd.service or something else has - set the system time and marked it as synchronized. Reaching this state generally requires synchronization with an - external source, such as an NTP server. + time-sync.target until the system time has been synchronized with an accurate time source by + systemd-timesyncd.service. - When this unit is not enabled the time-sync.target synchronization point may be reached - as soon as the system time is advanced by systemd-timesyncd.service to the time stored at the - last shutdown. That time may not meet the expectations of dependent services that require an accurate - clock. + systemd-timesyncd.service notifies on successful synchronization. + systemd-time-wait-sync also tries to detect when the kernel marks the time as synchronized, + but this detection is not reliable and is intended only as a fallback for other servies that can be used to + synchronize time (e.g., ntpd, chronyd). - Notes + Files + + + + /run/systemd/timesync/synchronized + + + The presence of this file indicates to this service that the system clock has been synchronized. + + + + - This service works correctly with a time synchronization service like - systemd-timesyncd.service that uses the same protocol as NTP to set the time from a - synchronized source. When used with time synchronization services that follow a different protocol the event of - setting synchronized time may not be detected in which case this service will not complete. diff --git a/man/systemd-timesyncd.service.xml b/man/systemd-timesyncd.service.xml index 26f7837..49e00e2 100644 --- a/man/systemd-timesyncd.service.xml +++ b/man/systemd-timesyncd.service.xml @@ -80,10 +80,21 @@ /var/lib/systemd/timesync/clock - This file contains the timestamp of the last successful + The modification time of this file indicates the timestamp of the last successful synchronization. + + + /run/systemd/timesync/synchronized + + + A file that is touched on each successful synchronization, to assist + systemd-time-wait-sync and other applications to detecting synchronization + events. + + + @@ -94,6 +105,7 @@ timesyncd.conf5, systemd.network5, systemd-networkd.service8, + systemd-time-wait-sync.service8, timedatectl1, localtime5, hwclock8 diff --git a/src/time-wait-sync/time-wait-sync.c b/src/time-wait-sync/time-wait-sync.c index 9b55853..198c055 100644 --- a/src/time-wait-sync/time-wait-sync.c +++ b/src/time-wait-sync/time-wait-sync.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -32,32 +33,88 @@ #include "sd-event.h" #include "fd-util.h" +#include "fs-util.h" #include "missing.h" #include "signal-util.h" #include "time-util.h" typedef struct ClockState { - int fd; /* non-negative is descriptor from timerfd_create */ - int adjtime_state; /* return value from last adjtimex(2) call */ - sd_event_source *event_source; /* non-null is the active io event source */ + int timerfd_fd; /* non-negative is descriptor from timerfd_create */ + int adjtime_state; /* return value from last adjtimex(2) call */ + sd_event_source *timerfd_event_source; /* non-null is the active io event source */ + int inotify_fd; + sd_event_source *inotify_event_source; + int run_systemd_wd; + int run_systemd_timesync_wd; + bool has_watchfile; } ClockState; +static void clock_state_release_timerfd(ClockState *sp) { + sp->timerfd_event_source = sd_event_source_unref(sp->timerfd_event_source); + sp->timerfd_fd = safe_close(sp->timerfd_fd); +} + static void clock_state_release(ClockState *sp) { - sp->event_source = sd_event_source_unref(sp->event_source); - sp->fd = safe_close(sp->fd); + clock_state_release_timerfd(sp); + sp->inotify_event_source = sd_event_source_unref(sp->inotify_event_source); + sp->inotify_fd = safe_close(sp->inotify_fd); } static int clock_state_update(ClockState *sp, sd_event *event); -static int io_handler(sd_event_source * s, - int fd, - uint32_t revents, - void *userdata) { +static int update_notify_run_systemd_timesync(ClockState *sp) { + sp->run_systemd_timesync_wd = inotify_add_watch(sp->inotify_fd, "/run/systemd/timesync", IN_CREATE|IN_DELETE_SELF); + return sp->run_systemd_timesync_wd; +} + +static int timerfd_handler(sd_event_source *s, + int fd, + uint32_t revents, + void *userdata) { ClockState *sp = userdata; return clock_state_update(sp, sd_event_source_get_event(s)); } +static void process_inotify_event(sd_event *event, ClockState *sp, struct inotify_event *e) { + if (e->wd == sp->run_systemd_wd) { + /* Only thing we care about is seeing if we can start watching /run/systemd/timesync. */ + if (sp->run_systemd_timesync_wd < 0) + update_notify_run_systemd_timesync(sp); + } else if (e->wd == sp->run_systemd_timesync_wd) { + if (e->mask & IN_DELETE_SELF) { + /* Somebody removed /run/systemd/timesync. */ + (void) inotify_rm_watch(sp->inotify_fd, sp->run_systemd_timesync_wd); + sp->run_systemd_timesync_wd = -1; + } else + /* Somebody might have created /run/systemd/timesync/synchronized. */ + clock_state_update(sp, event); + } +} + +static int inotify_handler(sd_event_source *s, + int fd, + uint32_t revents, + void *userdata) { + sd_event *event = sd_event_source_get_event(s); + ClockState *sp = userdata; + union inotify_event_buffer buffer; + struct inotify_event *e; + ssize_t l; + + l = read(fd, &buffer, sizeof(buffer)); + if (l < 0) { + if (IN_SET(errno, EAGAIN, EINTR)) + return 0; + + return log_warning_errno(errno, "Lost access to inotify: %m"); + } + FOREACH_INOTIFY_EVENT(e, buffer, l) + process_inotify_event(event, sp, e); + + return 0; +} + static int clock_state_update(ClockState *sp, sd_event *event) { static const struct itimerspec its = { @@ -69,34 +126,39 @@ static int clock_state_update(ClockState *sp, usec_t t; const char * ts; - clock_state_release(sp); + clock_state_release_timerfd(sp); /* The kernel supports cancelling timers whenever its realtime clock is "set" (which can happen in a variety of - * ways, generally adjustments of at least 500 ms). The way this module works is we set up a timer that will + * ways, generally adjustments of at least 500 ms). The way this module works is we set up a timerfd that will * wake when the clock is set, and when that happens we read the clock synchronization state from the return * value of adjtimex(2), which supports the NTP time adjustment protocol. * * The kernel determines whether the clock is synchronized using driver-specific tests, based on time - * information passed by an application, generally through adjtimex(2). If the application asserts the clock - * is synchronized, but does not also do something that "sets the clock", the timer will not be cancelled and - * synchronization will not be detected. Should this behavior be observed with a time synchronization provider - * this code might be reworked to do a periodic check as well. + * information passed by an application, generally through adjtimex(2). If the application asserts the clock is + * synchronized, but does not also do something that "sets the clock", the timer will not be cancelled and + * synchronization will not be detected. * * Similarly, this service will never complete if the application sets the time without also providing - * information that adjtimex(2) can use to determine that the clock is synchronized. + * information that adjtimex(2) can use to determine that the clock is synchronized. This generally doesn't + * happen, but can if the system has a hardware clock that is accurate enough that the adjustment is too small + * to be a "set". + * + * Both these failure-to-detect situations are covered by having the presence/creation of + * /run/systemd/timesync/synchronized, which is considered sufficient to indicate a synchronized clock even if + * the kernel has not been updated. * - * Well-behaved implementations including systemd-timesyncd should not produce either situation. For timesyncd - * the initial setting of the time uses settimeofday(2), which sets the clock but does not mark it - * synchronized. When an NTP source is selected it sets the clock again with clock_adjtime(2) which does mark - * it synchronized. */ + * For timesyncd the initial setting of the time uses settimeofday(2), which sets the clock but does not mark + * it synchronized. When an NTP source is selected it sets the clock again with clock_adjtime(2) which marks it + * synchronized and also touches /run/systemd/timesync/synchronized which covers the case when the clock wasn't + * "set". */ r = timerfd_create(CLOCK_REALTIME, TFD_NONBLOCK | TFD_CLOEXEC); if (r < 0) { log_error_errno(errno, "Failed to create timerfd: %m"); goto finish; } - sp->fd = r; + sp->timerfd_fd = r; - r = timerfd_settime(sp->fd, TFD_TIMER_ABSTIME | TFD_TIMER_CANCEL_ON_SET, &its, NULL); + r = timerfd_settime(sp->timerfd_fd, TFD_TIMER_ABSTIME | TFD_TIMER_CANCEL_ON_SET, &its, NULL); if (r < 0) { log_error_errno(errno, "Failed to set timerfd conditions: %m"); goto finish; @@ -117,24 +179,26 @@ static int clock_state_update(ClockState *sp, strcpy(buf, "unrepresentable"); log_info("adjtime state %d status %x time %s", sp->adjtime_state, tx.status, ts); - if (sp->adjtime_state == TIME_ERROR) { - /* Not synchronized. Do a one-shot wait on the descriptor and inform the caller we need to keep + sp->has_watchfile = access("/run/systemd/timesync/synchronized", F_OK) >= 0; + if (sp->has_watchfile) + /* Presence of watch file overrides adjtime_state */ + r = 0; + else if (sp->adjtime_state == TIME_ERROR) { + /* Not synchronized. Do a one-shot wait on the descriptor and inform the caller we need to keep * running. */ - r = sd_event_add_io(event, &sp->event_source, sp->fd, - EPOLLIN, io_handler, sp); + r = sd_event_add_io(event, &sp->timerfd_event_source, sp->timerfd_fd, + EPOLLIN, timerfd_handler, sp); if (r < 0) { log_error_errno(r, "Failed to create time change monitor source: %m"); goto finish; } r = 1; - } else { + } else /* Synchronized; we can exit. */ - (void) sd_event_exit(event, 0); r = 0; - } finish: - if (r < 0) + if (r <= 0) (void) sd_event_exit(event, r); return r; } @@ -143,7 +207,10 @@ int main(int argc, char * argv[]) { int r; _cleanup_(sd_event_unrefp) sd_event *event; ClockState state = { - .fd = -1, + .timerfd_fd = -1, + .inotify_fd = -1, + .run_systemd_wd = -1, + .run_systemd_timesync_wd = -1, }; assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGTERM, SIGINT, -1) >= 0); @@ -172,17 +239,42 @@ int main(int argc, char * argv[]) { goto finish; } + r = inotify_init1(IN_NONBLOCK|IN_CLOEXEC); + if (r < 0) { + log_error_errno(errno, "Failed to create inotify descriptor: %m"); + goto finish; + } + state.inotify_fd = r; + + r = sd_event_add_io(event, &state.inotify_event_source, state.inotify_fd, + EPOLLIN, inotify_handler, &state); + if (r < 0) { + log_error_errno(r, "Failed to create notify event source: %m"); + goto finish; + } + + r = inotify_add_watch(state.inotify_fd, "/run/systemd/", IN_CREATE); + if (r < 0) { + log_error_errno(errno, "Failed to watch /run/systemd/: %m"); + goto finish; + } + state.run_systemd_wd = r; + + (void) update_notify_run_systemd_timesync(&state); + r = clock_state_update(&state, event); if (r > 0) { r = sd_event_loop(event); if (r < 0) log_error_errno(r, "Failed in event loop: %m"); - else if (state.adjtime_state == TIME_ERROR) { - log_error("Event loop terminated without synchronizing"); - r = -ECANCELED; - } } + if (state.has_watchfile) + log_debug("Exit enabled by: /run/systemd/timesync/synchonized"); + + if (state.adjtime_state == TIME_ERROR) + log_info("Exit without adjtimex synchronized."); + finish: clock_state_release(&state); return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c index cfdc43b..5b26baf 100644 --- a/src/timesync/timesyncd-manager.c +++ b/src/timesync/timesyncd-manager.c @@ -355,6 +355,7 @@ static int manager_adjust_clock(Manager *m, double offset, int leap_sec) { /* If touch fails, there isn't much we can do. Maybe it'll work next time. */ (void) touch("/var/lib/systemd/timesync/clock"); + (void) touch("/run/systemd/timesync/synchronized"); m->drift_ppm = tmx.freq / 65536; diff --git a/units/systemd-timesyncd.service.in b/units/systemd-timesyncd.service.in index d3bc4e9..5b8b243 100644 --- a/units/systemd-timesyncd.service.in +++ b/units/systemd-timesyncd.service.in @@ -37,6 +37,7 @@ MemoryDenyWriteExecute=yes RestrictRealtime=yes RestrictNamespaces=yes RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 +RuntimeDirectory=systemd/timesync SystemCallFilter=~@cpu-emulation @debug @keyring @module @mount @obsolete @raw-io @reboot @swap SystemCallArchitectures=native LockPersonality=yes -- 2.7.4