From 05e6d9c64be181c2369e6ca68d2b78ca35647042 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 6 Nov 2018 15:55:09 +0900 Subject: [PATCH 1/5] udevd: merge conditions to decrease indentation --- src/udev/udevd.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 8bc0705012..0aec3acd07 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1250,15 +1250,13 @@ static int on_sigchld(sd_event_source *s, const struct signalfd_siginfo *si, voi } else log_warning("worker ["PID_FMT"] exit with status 0x%04x", pid, status); - if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) { - if (worker->event) { - log_error("worker ["PID_FMT"] failed while handling '%s'", pid, worker->event->devpath); - /* delete state from disk */ - udev_device_delete_db(worker->event->dev); - udev_device_tag_index(worker->event->dev, NULL, false); - /* forward kernel event without amending it */ - udev_monitor_send_device(manager->monitor, NULL, worker->event->dev_kernel); - } + if ((!WIFEXITED(status) || WEXITSTATUS(status) != 0) && worker->event) { + log_error("worker ["PID_FMT"] failed while handling '%s'", pid, worker->event->devpath); + /* delete state from disk */ + udev_device_delete_db(worker->event->dev); + udev_device_tag_index(worker->event->dev, NULL, false); + /* forward kernel event without amending it */ + udev_monitor_send_device(manager->monitor, NULL, worker->event->dev_kernel); } worker_free(worker); From b6107f010a1568715a4d4dbb634e01518bd8a336 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 6 Nov 2018 15:58:41 +0900 Subject: [PATCH 2/5] udevd: decrease indentation --- src/udev/udevd.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 0aec3acd07..9ab10d9d6f 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1270,28 +1270,30 @@ static int on_sigchld(sd_event_source *s, const struct signalfd_siginfo *si, voi static int on_post(sd_event_source *s, void *userdata) { Manager *manager = userdata; - int r; assert(manager); - if (LIST_IS_EMPTY(manager->events)) { - /* no pending events */ - if (!hashmap_isempty(manager->workers)) { - /* there are idle workers */ - log_debug("cleanup idle workers"); - manager_kill_workers(manager); - } else { - /* we are idle */ - if (manager->exit) { - r = sd_event_exit(manager->event, 0); - if (r < 0) - return r; - } else if (manager->cgroup) - /* cleanup possible left-over processes in our cgroup */ - cg_kill(SYSTEMD_CGROUP_CONTROLLER, manager->cgroup, SIGKILL, CGROUP_IGNORE_SELF, NULL, NULL, NULL); - } + if (!LIST_IS_EMPTY(manager->events)) + return 1; + + /* There are no pending events. Let's cleanup idle process. */ + + if (!hashmap_isempty(manager->workers)) { + /* There are idle workers */ + log_debug("Cleanup idle workers"); + manager_kill_workers(manager); + return 1; } + /* There are no idle workers. */ + + if (manager->exit) + return sd_event_exit(manager->event, 0); + + if (manager->cgroup) + /* cleanup possible left-over processes in our cgroup */ + (void) cg_kill(SYSTEMD_CGROUP_CONTROLLER, manager->cgroup, SIGKILL, CGROUP_IGNORE_SELF, NULL, NULL, NULL); + return 1; } From da14313418f1bbbdf52bd9435e6a183b1f59779a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 6 Nov 2018 17:46:00 +0900 Subject: [PATCH 3/5] udevd: drop redundant logic of receiving uevent If there exists pending uevents, then sd-event invokes uevent handler. So, it is not necessary to receive the next uevent in inotify event. --- src/udev/udevd.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 9ab10d9d6f..728b45343d 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1178,16 +1178,9 @@ static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userda continue; log_device_debug(dev, "Inotify event: %x for %s", e->mask, devnode); - if (e->mask & IN_CLOSE_WRITE) { + if (e->mask & IN_CLOSE_WRITE) synthesize_change(dev); - - /* settle might be waiting on us to determine the queue - * state. If we just handled an inotify event, we might have - * generated a "change" event, but we won't have queued up - * the resultant uevent yet. Do that. - */ - on_uevent(NULL, -1, 0, manager); - } else if (e->mask & IN_IGNORED) + else if (e->mask & IN_IGNORED) udev_watch_end(dev); } From eca195ec23fb4ed3ece26b55f85c5eefa662e967 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 1 Nov 2018 10:37:51 +0900 Subject: [PATCH 4/5] udevd: wait 3 seconds before killing worker processes During boot process, many worker processes are forked and killed. To decrease cycles of forking and killing worker, let's wait 3 seconds before killing workers. If new uevent or inotify event comes within the delay, the killing porcess will be cancelled. --- src/udev/udevd.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 2 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 728b45343d..cff0afdd3d 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -86,6 +86,7 @@ typedef struct Manager { sd_event_source *ctrl_event; sd_event_source *uevent_event; sd_event_source *inotify_event; + sd_event_source *kill_workers_event; usec_t last_usec; @@ -284,6 +285,7 @@ static void manager_free(Manager *manager) { sd_event_source_unref(manager->ctrl_event); sd_event_source_unref(manager->uevent_event); sd_event_source_unref(manager->inotify_event); + sd_event_source_unref(manager->kill_workers_event); sd_event_unref(manager->event); manager_workers_free(manager); @@ -364,6 +366,7 @@ static void worker_spawn(Manager *manager, struct event *event) { manager->ctrl_event = sd_event_source_unref(manager->ctrl_event); manager->uevent_event = sd_event_source_unref(manager->uevent_event); manager->inotify_event = sd_event_source_unref(manager->inotify_event); + manager->kill_workers_event = sd_event_source_unref(manager->kill_workers_event); manager->event = sd_event_unref(manager->event); @@ -767,6 +770,73 @@ static void manager_reload(Manager *manager) { "STATUS=Processing with %u children at max", arg_children_max); } +static int on_kill_workers_event(sd_event_source *s, uint64_t usec, void *userdata) { + Manager *manager = userdata; + + assert(manager); + + log_debug("Cleanup idle workers"); + manager_kill_workers(manager); + + return 1; +} + +static int manager_enable_kill_workers_event(Manager *manager) { + int enabled, r; + + assert(manager); + + if (!manager->kill_workers_event) + goto create_new; + + r = sd_event_source_get_enabled(manager->kill_workers_event, &enabled); + if (r < 0) { + log_debug_errno(r, "Failed to query whether event source for killing idle workers is enabled or not, trying to create new event source: %m"); + manager->kill_workers_event = sd_event_source_unref(manager->kill_workers_event); + goto create_new; + } + + if (enabled == SD_EVENT_ONESHOT) + return 0; + + r = sd_event_source_set_time(manager->kill_workers_event, now(CLOCK_MONOTONIC) + 3 * USEC_PER_SEC); + if (r < 0) { + log_debug_errno(r, "Failed to set time to event source for killing idle workers, trying to create new event source: %m"); + manager->kill_workers_event = sd_event_source_unref(manager->kill_workers_event); + goto create_new; + } + + r = sd_event_source_set_enabled(manager->kill_workers_event, SD_EVENT_ONESHOT); + if (r < 0) { + log_debug_errno(r, "Failed to enable event source for killing idle workers, trying to create new event source: %m"); + manager->kill_workers_event = sd_event_source_unref(manager->kill_workers_event); + goto create_new; + } + + return 0; + +create_new: + r = sd_event_add_time(manager->event, &manager->kill_workers_event, CLOCK_MONOTONIC, + now(CLOCK_MONOTONIC) + 3 * USEC_PER_SEC, USEC_PER_SEC, on_kill_workers_event, manager); + if (r < 0) + return log_warning_errno(r, "Failed to create timer event for killing idle workers: %m"); + + return 0; +} + +static int manager_disable_kill_workers_event(Manager *manager) { + int r; + + if (!manager->kill_workers_event) + return 0; + + r = sd_event_source_set_enabled(manager->kill_workers_event, SD_EVENT_OFF); + if (r < 0) + return log_warning_errno(r, "Failed to disable event source for cleaning up idle workers, ignoring: %m"); + + return 0; +} + static void event_queue_start(Manager *manager) { struct event *event; usec_t usec; @@ -788,6 +858,8 @@ static void event_queue_start(Manager *manager) { manager->last_usec = usec; } + (void) manager_disable_kill_workers_event(manager); + udev_builtin_init(); if (!manager->rules) { @@ -1159,6 +1231,8 @@ static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userda assert(manager); + (void) manager_disable_kill_workers_event(manager); + l = read(fd, &buffer, sizeof(buffer)); if (l < 0) { if (IN_SET(errno, EAGAIN, EINTR)) @@ -1258,6 +1332,10 @@ static int on_sigchld(sd_event_source *s, const struct signalfd_siginfo *si, voi /* we can start new workers, try to schedule events */ event_queue_start(manager); + /* Disable unnecessary cleanup event */ + if (hashmap_isempty(manager->workers) && manager->kill_workers_event) + (void) sd_event_source_set_enabled(manager->kill_workers_event, SD_EVENT_OFF); + return 1; } @@ -1273,8 +1351,7 @@ static int on_post(sd_event_source *s, void *userdata) { if (!hashmap_isempty(manager->workers)) { /* There are idle workers */ - log_debug("Cleanup idle workers"); - manager_kill_workers(manager); + (void) manager_enable_kill_workers_event(manager); return 1; } From faae64fa3dc22738b3af669579a33055b9d71432 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 1 Nov 2018 16:54:10 +0900 Subject: [PATCH 5/5] udevd: increase default number of workers The uevent handling in udevd is not cpu hungry. So, let's increase the default number of workers per cpu. This decrease the number of queued uevents. Without this commit (children_max is 16 on my laptop) ``` $ journalctl -b -u systemd-udevd.service | grep reached | wc -l 1544 ``` With this commit (children_max is 30 on my laptop) ``` $ journalctl -b -u systemd-udevd.service | grep reached | wc -l 7 ``` --- src/udev/udevd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index cff0afdd3d..53a77ee037 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1786,7 +1786,7 @@ int main(int argc, char *argv[]) { arg_children_max = 8; if (sched_getaffinity(0, sizeof(cpu_set), &cpu_set) == 0) - arg_children_max += CPU_COUNT(&cpu_set) * 2; + arg_children_max += CPU_COUNT(&cpu_set) * 8; mem_limit = physical_memory() / (128LU*1024*1024); arg_children_max = MAX(10U, MIN(arg_children_max, mem_limit));