diff --git a/src/basic/siphash24.h b/src/basic/siphash24.h index 7f799ede3d..fe43256882 100644 --- a/src/basic/siphash24.h +++ b/src/basic/siphash24.h @@ -6,6 +6,8 @@ #include #include +#include "time-util.h" + struct siphash { uint64_t v0; uint64_t v1; @@ -25,6 +27,10 @@ static inline void siphash24_compress_boolean(bool in, struct siphash *state) { siphash24_compress(&i, sizeof i, state); } +static inline void siphash24_compress_usec_t(usec_t in, struct siphash *state) { + siphash24_compress(&in, sizeof in, state); +} + static inline void siphash24_compress_string(const char *in, struct siphash *state) { if (!in) return; diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index a8eee28502..a93f12b27c 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -5268,7 +5268,7 @@ int unit_load_fragment(Unit *u) { /* Possibly rebuild the fragment map to catch new units */ r = unit_file_build_name_map(&u->manager->lookup_paths, - &u->manager->unit_cache_mtime, + &u->manager->unit_cache_timestamp_hash, &u->manager->unit_id_map, &u->manager->unit_name_map, &u->manager->unit_path_cache); diff --git a/src/core/manager.c b/src/core/manager.c index 49bf5419d8..bd02337faf 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -704,7 +704,7 @@ static void manager_free_unit_name_maps(Manager *m) { m->unit_id_map = hashmap_free(m->unit_id_map); m->unit_name_map = hashmap_free(m->unit_name_map); m->unit_path_cache = set_free(m->unit_path_cache); - m->unit_cache_mtime = 0; + m->unit_cache_timestamp_hash = 0; } static int manager_setup_run_queue(Manager *m) { @@ -1947,19 +1947,22 @@ unsigned manager_dispatch_load_queue(Manager *m) { return n; } -bool manager_unit_file_maybe_loadable_from_cache(Unit *u) { +bool manager_unit_cache_should_retry_load(Unit *u) { assert(u); + /* Automatic reloading from disk only applies to units which were not found sometime in the past, and + * the not-found stub is kept pinned in the unit graph by dependencies. For units that were + * previously loaded, we don't do automatic reloading, and daemon-reload is necessary to update. */ if (u->load_state != UNIT_NOT_FOUND) return false; - if (u->manager->unit_cache_mtime == 0) - return false; - - if (u->manager->unit_cache_mtime > u->fragment_loadtime) + /* The cache has been updated since the last time we tried to load the unit. There might be new + * fragment paths to read. */ + if (u->manager->unit_cache_timestamp_hash != u->fragment_not_found_timestamp_hash) return true; - return !lookup_paths_mtime_good(&u->manager->lookup_paths, u->manager->unit_cache_mtime); + /* The cache needs to be updated because there are modifications on disk. */ + return !lookup_paths_timestamp_hash_same(&u->manager->lookup_paths, u->manager->unit_cache_timestamp_hash, NULL); } int manager_load_unit_prepare( @@ -2008,10 +2011,10 @@ int manager_load_unit_prepare( * first if anything in the usual paths was modified since the last time * the cache was loaded. Also check if the last time an attempt to load the * unit was made was before the most recent cache refresh, so that we know - * we need to try again - even if the cache is current, it might have been + * we need to try again — even if the cache is current, it might have been * updated in a different context before we had a chance to retry loading * this particular unit. */ - if (manager_unit_file_maybe_loadable_from_cache(ret)) + if (manager_unit_cache_should_retry_load(ret)) ret->load_state = UNIT_STUB; else { *_ret = ret; diff --git a/src/core/manager.h b/src/core/manager.h index d507dc1f3b..9e98b31c4b 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -233,7 +233,7 @@ struct Manager { Hashmap *unit_id_map; Hashmap *unit_name_map; Set *unit_path_cache; - usec_t unit_cache_mtime; + uint64_t unit_cache_timestamp_hash; char **transient_environment; /* The environment, as determined from config files, kernel cmdline and environment generators */ char **client_environment; /* Environment variables created by clients through the bus API */ @@ -464,7 +464,7 @@ Unit *manager_get_unit(Manager *m, const char *name); int manager_get_job_from_dbus_path(Manager *m, const char *s, Job **_j); -bool manager_unit_file_maybe_loadable_from_cache(Unit *u); +bool manager_unit_cache_should_retry_load(Unit *u); int manager_load_unit_prepare(Manager *m, const char *name, const char *path, sd_bus_error *e, Unit **_ret); int manager_load_unit(Manager *m, const char *name, const char *path, sd_bus_error *e, Unit **_ret); int manager_load_startable_unit_or_warn(Manager *m, const char *name, const char *path, Unit **ret); diff --git a/src/core/transaction.c b/src/core/transaction.c index 958243bc94..0fa419787e 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -960,12 +960,13 @@ int transaction_add_job_and_dependencies( * first if anything in the usual paths was modified since the last time * the cache was loaded. Also check if the last time an attempt to load the * unit was made was before the most recent cache refresh, so that we know - * we need to try again - even if the cache is current, it might have been + * we need to try again — even if the cache is current, it might have been * updated in a different context before we had a chance to retry loading * this particular unit. + * * Given building up the transaction is a synchronous operation, attempt * to load the unit immediately. */ - if (r < 0 && manager_unit_file_maybe_loadable_from_cache(unit)) { + if (r < 0 && manager_unit_cache_should_retry_load(unit)) { sd_bus_error_free(e); unit->load_state = UNIT_STUB; r = unit_load(unit); diff --git a/src/core/unit.c b/src/core/unit.c index e81ef2fd9c..518a07f619 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1671,18 +1671,18 @@ int unit_load(Unit *u) { return 0; fail: - /* We convert ENOEXEC errors to the UNIT_BAD_SETTING load state here. Configuration parsing code should hence - * return ENOEXEC to ensure units are placed in this state after loading */ + /* We convert ENOEXEC errors to the UNIT_BAD_SETTING load state here. Configuration parsing code + * should hence return ENOEXEC to ensure units are placed in this state after loading. */ u->load_state = u->load_state == UNIT_STUB ? UNIT_NOT_FOUND : r == -ENOEXEC ? UNIT_BAD_SETTING : UNIT_ERROR; u->load_error = r; - /* Record the last time we tried to load the unit, so that if the cache gets updated between now - * and the next time an attempt is made to load this unit, we know we need to check again */ + /* Record the timestamp on the cache, so that if the cache gets updated between now and the next time + * an attempt is made to load this unit, we know we need to check again. */ if (u->load_state == UNIT_NOT_FOUND) - u->fragment_loadtime = now(CLOCK_REALTIME); + u->fragment_not_found_timestamp_hash = u->manager->unit_cache_timestamp_hash; unit_add_to_dbus_queue(u); unit_add_to_gc_queue(u); diff --git a/src/core/unit.h b/src/core/unit.h index e217cec219..5a75136e3d 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -136,7 +136,7 @@ typedef struct Unit { char *source_path; /* if converted, the source file */ char **dropin_paths; - usec_t fragment_loadtime; + usec_t fragment_not_found_timestamp_hash; usec_t fragment_mtime; usec_t source_mtime; usec_t dropin_mtime; 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, ×tamp_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);