From e36db5007517698ea07e9e506fcc0fd68820f714 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 19 Dec 2018 11:32:17 +0100 Subject: [PATCH 1/2] Revert "mount: disable mount-storm protection while mount unit is starting." This reverts commit fcfb1f775ed0e9d282607bb118ba788b98952855. --- src/core/manager.h | 1 - src/core/mount.c | 25 +------------------------ 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/src/core/manager.h b/src/core/manager.h index 18219a184b..9f8fc46434 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -230,7 +230,6 @@ struct Manager { sd_event_source *mount_timeout_source; usec_t mount_last_read_usec; usec_t mount_last_duration_usec; - unsigned mount_pending_count; /* Data specific to the swap filesystem */ FILE *proc_swaps; diff --git a/src/core/mount.c b/src/core/mount.c index 823024b416..cfdcc6e6f5 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -218,12 +218,6 @@ static void mount_done(Unit *u) { assert(m); - if (!IN_SET(m->state, MOUNT_DEAD, MOUNT_MOUNTED, MOUNT_FAILED)) { - /* This was pending, so need to udpate the count */ - assert(u->manager->mount_pending_count > 0); - u->manager->mount_pending_count--; - } - m->where = mfree(m->where); mount_parameters_done(&m->parameters_proc_self_mountinfo); @@ -656,7 +650,6 @@ static int mount_load(Unit *u) { static void mount_set_state(Mount *m, MountState state) { MountState old_state; - int was_pending, is_pending; assert(m); if (m->state != state) @@ -665,17 +658,6 @@ static void mount_set_state(Mount *m, MountState state) { old_state = m->state; m->state = state; - was_pending = !IN_SET(old_state, MOUNT_DEAD, MOUNT_MOUNTED, MOUNT_FAILED); - is_pending = !IN_SET(state, MOUNT_DEAD, MOUNT_MOUNTED, MOUNT_FAILED); - - if (was_pending && !is_pending) { - assert(UNIT(m)->manager->mount_pending_count > 0); - UNIT(m)->manager->mount_pending_count--; - } - - if (is_pending && !was_pending) - UNIT(m)->manager->mount_pending_count++; - if (!MOUNT_STATE_WITH_PROCESS(state)) { m->timer_event_source = sd_event_source_unref(m->timer_event_source); mount_unwatch_control_pid(m); @@ -1808,12 +1790,7 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, usec_t next_read = usec_add(m->mount_last_read_usec, m->mount_last_duration_usec * 10); - /* If there are pending mounts initiated by systemd, then - * we need to process changes promptly, otherwise we - * rate limit re-reading the file. - */ - if (m->mount_pending_count == 0 && - now(CLOCK_MONOTONIC) < next_read) { + if (now(CLOCK_MONOTONIC) < next_read) { /* The (current) API for getting mount events from the Linux kernel * involves getting a "something changed" notification, and then having * to re-read the entire /proc/self/mountinfo file. When there are lots From ec8126d72378a25152dc7e4a80b65fc9d12f951e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 19 Dec 2018 11:32:26 +0100 Subject: [PATCH 2/2] Revert "core/mount: minimize impact on mount storm." This reverts commit 89f9752ea08f516b5d77f8e577bb772073c70c01. This patch causes various problems during boot, where a "mount storm" occurs naturally. Current approach is flakey, and it seems very risky to push a feature like this which impacts boot right before a release. So let's revert for now, and consider a more robust solution after later. Fixes #11209. > https://github.com/systemd/systemd/pull/11196#issuecomment-448523186: "Reverting 89f9752ea08f516b5d77f8e577bb772073c70c01 and fcfb1f775ed0e9d282607bb118ba788b98952855 fixes this test." --- src/core/manager.h | 3 -- src/core/mount.c | 80 ++++------------------------------------------ 2 files changed, 7 insertions(+), 76 deletions(-) diff --git a/src/core/manager.h b/src/core/manager.h index 9f8fc46434..bce8020cfd 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -227,9 +227,6 @@ struct Manager { /* Data specific to the mount subsystem */ struct libmnt_monitor *mount_monitor; sd_event_source *mount_event_source; - sd_event_source *mount_timeout_source; - usec_t mount_last_read_usec; - usec_t mount_last_duration_usec; /* Data specific to the swap filesystem */ FILE *proc_swaps; diff --git a/src/core/mount.c b/src/core/mount.c index cfdcc6e6f5..ead9bc1f44 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -55,7 +55,6 @@ static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = { static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *userdata); static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata); -static int mount_dispatch_proc_self_mountinfo_timer(sd_event_source *source, usec_t usec, void *userdata); static bool MOUNT_STATE_WITH_PROCESS(MountState state) { return IN_SET(state, @@ -1666,7 +1665,6 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) { static void mount_shutdown(Manager *m) { assert(m); - m->mount_timeout_source = sd_event_source_unref(m->mount_timeout_source); m->mount_event_source = sd_event_source_unref(m->mount_event_source); mnt_unref_monitor(m->mount_monitor); @@ -1782,50 +1780,13 @@ fail: mount_shutdown(m); } -static void mount_process_proc_self_mountinfo(Manager *m); - static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata) { + _cleanup_set_free_free_ Set *around = NULL, *gone = NULL; Manager *m = userdata; + const char *what; + Iterator i; + Unit *u; int r; - usec_t next_read = usec_add(m->mount_last_read_usec, - m->mount_last_duration_usec * 10); - - if (now(CLOCK_MONOTONIC) < next_read) { - /* The (current) API for getting mount events from the Linux kernel - * involves getting a "something changed" notification, and then having - * to re-read the entire /proc/self/mountinfo file. When there are lots - * of mountpoints, this file is large and parsing it can take noticeable - * time. As most of the file won't have changed, this can be seen as wasted time. - * If there is a "mount storm" such as 1000 mount points being created - * in quick succession, this will result in 1000 successive notification. - * If we respond to every notification, we will do quadratically more work - * than if we respond just once after all the notifications have arrived. - * In this (pathological) case, a delay in scheduling would actually - * improve throughput as we would combine notifications and parse - * the file less often. We cannot expect the scheduler to notice - * this pathology without help. - * So when the rate of notifications means we are spending more than - * 10% of real time handling them, we set a timer and stop listening - * to notifications for a while. - * If/when Linux provides an API which provides only details of what - * has changed, this rate-limiting can be removed. - */ - - r = sd_event_source_set_enabled(source, SD_EVENT_OFF); - if (r < 0) - log_warning_errno(r, "Failed to disable monitoring of /proc/self/mounting, ignoring: %m"); - if (!m->mount_timeout_source) { - r = sd_event_add_time(m->event, &m->mount_timeout_source, - CLOCK_MONOTONIC, - next_read, - 0, - mount_dispatch_proc_self_mountinfo_timer, - m); - if (r < 0) - log_warning_errno(r, "Failed to set timeout to reread /proc/self/mounting, ignoring: %m"); - } - return 0; - } assert(m); assert(revents & EPOLLIN); @@ -1853,40 +1814,13 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, return 0; } - mount_process_proc_self_mountinfo(m); - return 0; -} - -static int mount_dispatch_proc_self_mountinfo_timer(sd_event_source *source, usec_t usec, void *userdata) { - Manager *m = userdata; - int r; - - r = sd_event_source_set_enabled(m->mount_event_source, SD_EVENT_ON); - if (r < 0) - log_warning_errno(r, "Failed to reenable /proc/self/mountinfo monitor, ignoring: %m"); - m->mount_timeout_source = sd_event_source_unref(source); - mount_process_proc_self_mountinfo(m); - return 0; -} - -static void mount_process_proc_self_mountinfo(Manager *m) { - _cleanup_set_free_free_ Set *around = NULL, *gone = NULL; - const char *what; - Iterator i; - Unit *u; - int r; - - m->mount_last_read_usec = now(CLOCK_MONOTONIC); - /* If an error occurs, assume 10ms */ - m->mount_last_duration_usec = 10 * USEC_PER_MSEC; - r = mount_load_proc_self_mountinfo(m, true); if (r < 0) { /* Reset flags, just in case, for later calls */ LIST_FOREACH(units_by_type, u, m->units_by_type[UNIT_MOUNT]) MOUNT(u)->proc_flags = 0; - return; + return 0; } manager_dispatch_load_queue(m); @@ -1974,8 +1908,8 @@ static void mount_process_proc_self_mountinfo(Manager *m) { /* Let the device units know that the device is no longer mounted */ device_found_node(m, what, 0, DEVICE_FOUND_MOUNT); } - m->mount_last_duration_usec = usec_sub_unsigned(now(CLOCK_MONOTONIC), - m->mount_last_read_usec); + + return 0; } static void mount_reset_failed(Unit *u) {