mount: check right before invoking /bin/umount if it makes sense

Notifications from /proc/self/mountinfo are async, so if we stop a
service (and while doing so get rid of the credentials mount point of
it), then it will take a while until the notification reaches us and we
actually scan the table again. In particular as we nowadays ratelimit
notifications on the table, since it's so inefficient. And as I learnt
the ratelimiting is actually quite regularly hit during shutdown, where
a flurry of umount events are genreated. Hence, let's check if a mount
point is actually a mountpoint before trying to unmount it. And if it
isn't let's wait for the notification to come in.

(This race might be triggred not just by us on ourselves btw: there are
other daemons that unmount stuff when stopping where the race also
exists, but might simply be harder to trigger: if during service
shutdown these services remove some mount then they might collide with
us doing the same. After all, we have the rule to unmount everything
mounted automatically for you during shutdown.)

In the long run we should also start making us of this when it becomes
available: https://github.com/util-linux/util-linux/issues/2132 With
that we can make issues like this go away entirely from our side of
things at least.

Fixes: #25527

diff --git a/src/basic/unit-def.c b/src/basic/unit-def.c
index 86b66e2be0c49d5f9dbe336e8f18df0cb9856868..bfd748ada4aa0d7b194527b468cfdec5eb846995 100644 (file)
--- a/src/basic/unit-def.c
+++ b/src/basic/unit-def.c
@@ -152,6 +152,7 @@ static const char* const mount_state_table[_MOUNT_STATE_MAX] = {
         [MOUNT_REMOUNTING_SIGKILL] = "remounting-sigkill",
         [MOUNT_UNMOUNTING_SIGTERM] = "unmounting-sigterm",
         [MOUNT_UNMOUNTING_SIGKILL] = "unmounting-sigkill",
+        [MOUNT_UNMOUNTING_CATCHUP] = "unmounting-catchup",
         [MOUNT_FAILED]             = "failed",
         [MOUNT_CLEANING]           = "cleaning",
 };

diff --git a/src/basic/unit-def.h b/src/basic/unit-def.h
index 169e1f719ec4ffcd85ae6ef79664c99abd7dc079..7bd86cb1d2ff4af2ec9215ffc90c7091c2ac0cb5 100644 (file)
--- a/src/basic/unit-def.h
+++ b/src/basic/unit-def.h
@@ -97,6 +97,7 @@ typedef enum MountState {
         MOUNT_REMOUNTING_SIGKILL,
         MOUNT_UNMOUNTING_SIGTERM,
         MOUNT_UNMOUNTING_SIGKILL,
+        MOUNT_UNMOUNTING_CATCHUP,
         MOUNT_FAILED,
         MOUNT_CLEANING,
         _MOUNT_STATE_MAX,

diff --git a/src/core/mount.c b/src/core/mount.c
index 4a0dc7ebacf9afeb05adc69c7439efa76185c6fe..295e4c66c790b02c1a0e48c2283a9a60166368f1 100644 (file)
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -48,6 +48,7 @@ static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = {
         [MOUNT_REMOUNTING_SIGKILL] = UNIT_RELOADING,
         [MOUNT_UNMOUNTING_SIGTERM] = UNIT_DEACTIVATING,
         [MOUNT_UNMOUNTING_SIGKILL] = UNIT_DEACTIVATING,
+        [MOUNT_UNMOUNTING_CATCHUP] = UNIT_DEACTIVATING,
         [MOUNT_FAILED] = UNIT_FAILED,
         [MOUNT_CLEANING] = UNIT_MAINTENANCE,
 };
@@ -1070,11 +1071,37 @@ static void mount_enter_unmounting(Mount *m) {
 
         assert(m);
 
+        r = path_is_mount_point(m->where, NULL, 0);
+        if (IN_SET(r, 0, -ENOENT)) {
+                /* Not a mount point anymore ? Then we raced against something else which unmounted it? Let's
+                 * handle this gracefully, and wait until we get the kernel notification about it. */
+
+                log_unit_debug(UNIT(m), "Path '%s' is not a mount point anymore, waiting for mount table refresh.", m->where);
+
+                /* Apparently our idea of the kernel mount table is out of date. Make sure we re-read it
+                 * again, soon, so that we don't delay mount handling unnecessarily. */
+                (void) sd_event_source_leave_ratelimit(UNIT(m)->manager->mount_event_source);
+
+                m->control_command_id = _MOUNT_EXEC_COMMAND_INVALID;
+                mount_unwatch_control_pid(m);
+
+                r = mount_arm_timer(m, usec_add(now(CLOCK_MONOTONIC), m->timeout_usec));
+                if (r < 0)
+                        goto fail;
+
+                /* Let's enter a distinct state where we just wait for the mount table to catch up */
+                mount_set_state(m, MOUNT_UNMOUNTING_CATCHUP);
+                return;
+        }
+        if (r < 0)
+                log_unit_debug_errno(UNIT(m), r, "Unable to determine if '%s' is a mount point, ignoring: %m", m->where);
+
         /* Start counting our attempts */
         if (!IN_SET(m->state,
                     MOUNT_UNMOUNTING,
                     MOUNT_UNMOUNTING_SIGTERM,
-                    MOUNT_UNMOUNTING_SIGKILL))
+                    MOUNT_UNMOUNTING_SIGKILL,
+                    MOUNT_UNMOUNTING_CATCHUP))
                 m->n_retry_umount = 0;
 
         m->control_command_id = MOUNT_EXEC_UNMOUNT;
@@ -1095,7 +1122,6 @@ static void mount_enter_unmounting(Mount *m) {
                 goto fail;
 
         mount_set_state(m, MOUNT_UNMOUNTING);
-
         return;
 
 fail:
@@ -1268,6 +1294,7 @@ static int mount_start(Unit *u) {
                    MOUNT_UNMOUNTING,
                    MOUNT_UNMOUNTING_SIGTERM,
                    MOUNT_UNMOUNTING_SIGKILL,
+                   MOUNT_UNMOUNTING_CATCHUP,
                    MOUNT_CLEANING))
                 return -EAGAIN;
 
@@ -1297,6 +1324,7 @@ static int mount_stop(Unit *u) {
         case MOUNT_UNMOUNTING:
         case MOUNT_UNMOUNTING_SIGKILL:
         case MOUNT_UNMOUNTING_SIGTERM:
+        case MOUNT_UNMOUNTING_CATCHUP:
                 /* Already on it */
                 return 0;
 
@@ -1562,6 +1590,10 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                 mount_enter_dead_or_mounted(m, f);
                 break;
 
+        case MOUNT_UNMOUNTING_CATCHUP:
+                log_unit_debug(u, "Was notified about control process death, but wasn't expecting it. Ignoring.");
+                break;
+
         case MOUNT_CLEANING:
                 if (m->clean_result == MOUNT_SUCCESS)
                         m->clean_result = f;
@@ -1636,6 +1668,11 @@ static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *user
                 mount_enter_dead_or_mounted(m, MOUNT_FAILURE_TIMEOUT);
                 break;
 
+        case MOUNT_UNMOUNTING_CATCHUP:
+                log_unit_warning(UNIT(m), "Waiting for unmount notification timed out. Giving up.");
+                mount_enter_dead_or_mounted(m, MOUNT_FAILURE_TIMEOUT);
+                break;
+
         case MOUNT_CLEANING:
                 log_unit_warning(UNIT(m), "Cleaning timed out. killing.");
 
@@ -2077,8 +2114,7 @@ static int mount_process_proc_self_mountinfo(Manager *m) {
 
                 if (!mount_is_mounted(mount)) {
 
-                        /* A mount point is not around right now. It
-                         * might be gone, or might never have
+                        /* A mount point is not around right now. It might be gone, or might never have
                          * existed. */
 
                         if (mount->from_proc_self_mountinfo &&
@@ -2093,6 +2129,7 @@ static int mount_process_proc_self_mountinfo(Manager *m) {
                         switch (mount->state) {
 
                         case MOUNT_MOUNTED:
+                        case MOUNT_UNMOUNTING_CATCHUP:
                                 /* This has just been unmounted by somebody else, follow the state change. */
                                 mount_enter_dead(mount, MOUNT_SUCCESS);
                                 break;
@@ -2131,11 +2168,9 @@ static int mount_process_proc_self_mountinfo(Manager *m) {
                                 break;
 
                         default:
-                                /* Nothing really changed, but let's
-                                 * issue an notification call
-                                 * nonetheless, in case somebody is
-                                 * waiting for this. (e.g. file system
-                                 * ro/rw remounts.) */
+                                /* Nothing really changed, but let's issue an notification call nonetheless,
+                                 * in case somebody is waiting for this. (e.g. file system ro/rw
+                                 * remounts.) */
                                 mount_set_state(mount, mount->state);
                                 break;
                         }