]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
pid1: use the cache mtime not clock to "mark" load attempts
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Fri, 28 Aug 2020 09:19:38 +0000 (11:19 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Mon, 31 Aug 2020 18:53:38 +0000 (20:53 +0200)
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.

src/core/manager.c
src/core/unit.c
src/core/unit.h

index 20ec7ba874ea7bd937c7523e5092efca27b15268..03599997845bccd23300cfb09e46c6651fa9840f 100644 (file)
@@ -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. */
index 0124451c78a6fef7cf026428b0355e4a7f9a447e..e70831869e2d4a14b2f8cffd749d07aea5724d27 100644 (file)
@@ -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);
index e217cec2191de50b1f283d74b6b1219a13727c91..a54d649cd32f0236fc25e18c38dbdf376c86c05e 100644 (file)
@@ -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;