Rework how we cache mtime to figure out if units changed

Instead of assuming that more-recently modified directories have higher mtime,
just look for any mtime changes, up or down. Since we don't want to remember
individual mtimes, hash them to obtain a single value.

This should help us behave properly in the case when the time jumps backwards
during boot: various files might have mtimes that in the future, but we won't
care. This fixes the following scenario:

We have /etc/systemd/system with T1. T1 is initially far in the past.
We have /run/systemd/generator with time T2.
The time is adjusted backwards, so T2 will be always in the future for a while.
Now the user writes new files to /etc/systemd/system, and T1 is updated to T1'.
Nevertheless, T1 < T1' << T2.
We would consider our cache to be up-to-date, falsely.

2 files changed, 34 insertions, 29 deletions

diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c
index ed4affd668..0f030ae431 100644
--- a/src/shared/unit-file.c
+++ b/src/shared/unit-file.c
@@ -1,5 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 
+#include "sd-id128.h"
+
 #include "dirent-util.h"
 #include "fd-util.h"
 #include "fs-util.h"
@@ -199,9 +201,14 @@ static bool lookup_paths_mtime_exclude(const LookupPaths *lp, const char *path)
                streq_ptr(path, lp->runtime_control);
 }
 
-bool lookup_paths_mtime_good(const LookupPaths *lp, usec_t mtime) {
-        char **dir;
+#define HASH_KEY SD_ID128_MAKE(4e,86,1b,e3,39,b3,40,46,98,5d,b8,11,34,8f,c3,c1)
+
+bool lookup_paths_timestamp_hash_same(const LookupPaths *lp, uint64_t timestamp_hash, uint64_t *ret_new) {
+        struct siphash state;
 
+        siphash24_init(&state, HASH_KEY.bytes);
+
+        char **dir;
         STRV_FOREACH(dir, (char**) lp->search_path) {
                 struct stat st;
 
@@ -217,18 +224,20 @@ bool lookup_paths_mtime_good(const LookupPaths *lp, usec_t mtime) {
                         continue;
                 }
 
-                if (timespec_load(&st.st_mtim) > mtime) {
-                        log_debug_errno(errno, "Unit dir %s has changed, need to update cache.", *dir);
-                        return false;
-                }
+                siphash24_compress_usec_t(timespec_load(&st.st_mtim), &state);
         }
 
-        return true;
+        uint64_t updated = siphash24_finalize(&state);
+        if (ret_new)
+                *ret_new = updated;
+        if (updated != timestamp_hash)
+                log_debug("Modification times have changed, need to update cache.");
+        return updated == timestamp_hash;
 }
 
 int unit_file_build_name_map(
                 const LookupPaths *lp,
-                usec_t *cache_mtime,
+                uint64_t *cache_timestamp_hash,
                 Hashmap **unit_ids_map,
                 Hashmap **unit_names_map,
                 Set **path_cache) {
@@ -245,14 +254,18 @@ int unit_file_build_name_map(
 
         _cleanup_hashmap_free_ Hashmap *ids = NULL, *names = NULL;
         _cleanup_set_free_free_ Set *paths = NULL;
+        uint64_t timestamp_hash;
         char **dir;
         int r;
-        usec_t mtime = 0;
 
-        /* Before doing anything, check if the mtime that was passed is still valid. If
-         * yes, do nothing. If *cache_time == 0, always build the cache. */
-        if (cache_mtime && *cache_mtime > 0 && lookup_paths_mtime_good(lp, *cache_mtime))
-                return 0;
+        /* Before doing anything, check if the timestamp hash that was passed is still valid.
+         * If yes, do nothing. */
+        if (cache_timestamp_hash &&
+            lookup_paths_timestamp_hash_same(lp, *cache_timestamp_hash, &timestamp_hash))
+                        return 0;
+
+        /* The timestamp hash is now set based on the mtimes from before when we start reading files.
+         * If anything is modified concurrently, we'll consider the cache outdated. */
 
         if (path_cache) {
                 paths = set_new(&path_hash_ops_free);
@@ -263,7 +276,6 @@ int unit_file_build_name_map(
         STRV_FOREACH(dir, (char**) lp->search_path) {
                 struct dirent *de;
                 _cleanup_closedir_ DIR *d = NULL;
-                struct stat st;
 
                 d = opendir(*dir);
                 if (!d) {
@@ -272,13 +284,6 @@ int unit_file_build_name_map(
                         continue;
                 }
 
-                /* Determine the latest lookup path modification time */
-                if (fstat(dirfd(d), &st) < 0)
-                        return log_error_errno(errno, "Failed to fstat %s: %m", *dir);
-
-                if (!lookup_paths_mtime_exclude(lp, *dir))
-                        mtime = MAX(mtime, timespec_load(&st.st_mtim));
-
                 FOREACH_DIRENT_ALL(de, d, log_warning_errno(errno, "Failed to read \"%s\", ignoring: %m", *dir)) {
                         char *filename;
                         _cleanup_free_ char *_filename_free = NULL, *simplified = NULL;
@@ -417,8 +422,8 @@ int unit_file_build_name_map(
                                                  basename(dst), src);
         }
 
-        if (cache_mtime)
-                *cache_mtime = mtime;
+        if (cache_timestamp_hash)
+                *cache_timestamp_hash = timestamp_hash;
 
         hashmap_free_and_replace(*unit_ids_map, ids);
         hashmap_free_and_replace(*unit_names_map, names);
diff --git a/src/shared/unit-file.h b/src/shared/unit-file.h
index d6d041d714..d36bb07cc2 100644
--- a/src/shared/unit-file.h
+++ b/src/shared/unit-file.h
@@ -43,19 +43,19 @@ bool unit_type_may_template(UnitType type) _const_;
 int unit_symlink_name_compatible(const char *symlink, const char *target, bool instance_propagation);
 int unit_validate_alias_symlink_and_warn(const char *filename, const char *target);
 
-bool lookup_paths_mtime_good(const LookupPaths *lp, usec_t mtime);
+bool lookup_paths_timestamp_hash_same(const LookupPaths *lp, uint64_t timestamp_hash, uint64_t *ret_new);
 int unit_file_build_name_map(
                 const LookupPaths *lp,
-                usec_t *ret_time,
-                Hashmap **ret_unit_ids_map,
-                Hashmap **ret_unit_names_map,
-                Set **ret_path_cache);
+                uint64_t *cache_timestamp_hash,
+                Hashmap **unit_ids_map,
+                Hashmap **unit_names_map,
+                Set **path_cache);
 
 int unit_file_find_fragment(
                 Hashmap *unit_ids_map,
                 Hashmap *unit_name_map,
                 const char *unit_name,
                 const char **ret_fragment_path,
-                Set **names);
+                Set **ret_names);
 
 const char* runlevel_to_target(const char *rl);