From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Fri, 28 Aug 2020 09:19:38 +0000 (+0200)
Subject: pid1: use the cache mtime not clock to "mark" load attempts
X-Git-Tag: v247-rc1~324^2~2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c149d2b49128700a2ae361f43b9065b51c174838;p=thirdparty%2Fsystemd.git

pid1: use the cache mtime not clock to "mark" load attempts

We really only care if the cache has been reloaded between the time when we
last attempted to load this unit and now. So instead of recording the actual
time we try to load the unit, just store the timestamp of the cache. This has
the advantage that we'll notice if the cache mtime jumps forward or backward.

Also rename fragment_loadtime to fragment_not_found_time. It only gets set when
we failed to load the unit and the old name was suggesting it is always set.

In https://bugzilla.redhat.com/show_bug.cgi?id=1871327
(and most likely https://bugzilla.redhat.com/show_bug.cgi?id=1867930
and most likely https://bugzilla.redhat.com/show_bug.cgi?id=1872068) we try
to load a non-existent unit over and over from transaction_add_job_and_dependencies().
My understanding is that the clock was in the future during inital boot,
so cache_mtime is always in the future (since we don't touch the fs after initial boot),
so no matter how many times we try to load the unit and set
fragment_loadtime / fragment_not_found_time, it is always higher than cache_mtime,
so manager_unit_cache_should_retry_load() always returns true.
---

diff --git a/src/core/manager.c b/src/core/manager.c
index 20ec7ba874e..03599997845 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1961,7 +1961,7 @@ bool manager_unit_cache_should_retry_load(Unit *u) {
 
         /* 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_mtime > u->fragment_loadtime)
+        if (u->manager->unit_cache_mtime != u->fragment_not_found_time)
                 return true;
 
         /* The cache needs to be updated because there are modifications on disk. */
diff --git a/src/core/unit.c b/src/core/unit.c
index 0124451c78a..e70831869e2 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1679,10 +1679,10 @@ fail:
                                                      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_time = u->manager->unit_cache_mtime;
 
         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 e217cec2191..a54d649cd32 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_time;
         usec_t fragment_mtime;
         usec_t source_mtime;
         usec_t dropin_mtime;