SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
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
This commit is contained in:
Lennart Poettering
2023-05-24 10:57:16 +02:00
parent 2fdc274c66
commit 1483892a42
3 changed files with 46 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/unit-def.c
+1
View File
@@ -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",
};
src/basic/unit-def.h
+1
View File
@@ -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,
src/core/mount.c
+44 -9
View File
@@ -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;
}