From 2dd79846ddd5f31aaf178eb3457677139e38fac0 Mon Sep 17 00:00:00 2001 From: "Peter A. Bigot" Date: Mon, 9 Apr 2018 13:39:16 -0500 Subject: 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 --- src/time-wait-sync/time-wait-sync.c | 162 ++++++++++++++++++++++++++++-------- 1 file changed, 127 insertions(+), 35 deletions(-) (limited to 'src/time-wait-sync') diff --git a/src/time-wait-sync/time-wait-sync.c b/src/time-wait-sync/time-wait-sync.c index 9b558532ce..198c055650 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; -- cgit v1.2.1