From 0079651876aa5df73fd787b272e91ae4a7898853 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 22 Dec 2024 00:11:38 +0900 Subject: [PATCH 1/2] udev: split manager_init() and manager_main() into small pieces No functional change, just refactoring. Co-authored-by: David Tardon --- src/udev/udev-manager.c | 233 +++++++++++++++++++++++++++++----------- 1 file changed, 169 insertions(+), 64 deletions(-) diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index 4fc316e106..d9bd9d6d7b 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -1232,6 +1232,48 @@ static int listen_fds(int *ret_ctrl, int *ret_netlink) { return 0; } +static int manager_init_ctrl(Manager *manager, int fd_ctrl) { + _cleanup_(udev_ctrl_unrefp) UdevCtrl *ctrl = NULL; + _cleanup_close_ int fd = fd_ctrl; + int r; + + assert(manager); + + /* This consumes passed file descriptor. */ + + r = udev_ctrl_new_from_fd(&ctrl, fd); + if (r < 0) + return log_error_errno(r, "Failed to initialize udev control socket: %m"); + TAKE_FD(fd); + + r = udev_ctrl_enable_receiving(ctrl); + if (r < 0) + return log_error_errno(r, "Failed to bind udev control socket: %m"); + + manager->ctrl = TAKE_PTR(ctrl); + return 0; +} + +static int manager_init_device_monitor(Manager *manager, int fd_uevent) { + _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor = NULL; + _cleanup_close_ int fd = fd_uevent; + int r; + + assert(manager); + + /* This consumes passed file descriptor. */ + + r = device_monitor_new_full(&monitor, MONITOR_GROUP_KERNEL, fd); + if (r < 0) + return log_error_errno(r, "Failed to initialize device monitor: %m"); + TAKE_FD(fd); + + (void) sd_device_monitor_set_description(monitor, "manager"); + + manager->monitor = TAKE_PTR(monitor); + return 0; +} + int manager_init(Manager *manager) { _cleanup_close_ int fd_ctrl = -EBADF, fd_uevent = -EBADF; _cleanup_free_ char *cgroup = NULL; @@ -1243,21 +1285,13 @@ int manager_init(Manager *manager) { if (r < 0) return log_error_errno(r, "Failed to listen on fds: %m"); - r = udev_ctrl_new_from_fd(&manager->ctrl, fd_ctrl); + r = manager_init_ctrl(manager, TAKE_FD(fd_ctrl)); if (r < 0) - return log_error_errno(r, "Failed to initialize udev control socket: %m"); - TAKE_FD(fd_ctrl); + return r; - r = udev_ctrl_enable_receiving(manager->ctrl); + r = manager_init_device_monitor(manager, TAKE_FD(fd_uevent)); if (r < 0) - return log_error_errno(r, "Failed to bind udev control socket: %m"); - - r = device_monitor_new_full(&manager->monitor, MONITOR_GROUP_KERNEL, fd_uevent); - if (r < 0) - return log_error_errno(r, "Failed to initialize device monitor: %m"); - TAKE_FD(fd_uevent); - - (void) sd_device_monitor_set_description(manager->monitor, "manager"); + return r; r = cg_pid_get_path(SYSTEMD_CGROUP_CONTROLLER, 0, &cgroup); if (r < 0) @@ -1270,48 +1304,11 @@ int manager_init(Manager *manager) { return 0; } -int manager_main(Manager *manager) { - int fd_worker, r; +static int manager_start_ctrl(Manager *manager) { + int r; - /* unnamed socket from workers to the main daemon */ - r = socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, manager->worker_watch); - if (r < 0) - return log_error_errno(errno, "Failed to create socketpair for communicating with workers: %m"); - - fd_worker = manager->worker_watch[READ_END]; - - r = setsockopt_int(fd_worker, SOL_SOCKET, SO_PASSCRED, true); - if (r < 0) - return log_error_errno(r, "Failed to enable SO_PASSCRED: %m"); - - manager->inotify_fd = inotify_init1(IN_CLOEXEC); - if (manager->inotify_fd < 0) - return log_error_errno(errno, "Failed to create inotify descriptor: %m"); - - udev_watch_restore(manager->inotify_fd); - - /* block SIGCHLD for listening child events. */ - assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD) >= 0); - - r = sd_event_default(&manager->event); - if (r < 0) - return log_error_errno(r, "Failed to allocate event loop: %m"); - - r = sd_event_add_signal(manager->event, NULL, SIGINT | SD_EVENT_SIGNAL_PROCMASK, on_sigterm, manager); - if (r < 0) - return log_error_errno(r, "Failed to create SIGINT event source: %m"); - - r = sd_event_add_signal(manager->event, NULL, SIGTERM | SD_EVENT_SIGNAL_PROCMASK, on_sigterm, manager); - if (r < 0) - return log_error_errno(r, "Failed to create SIGTERM event source: %m"); - - r = sd_event_add_signal(manager->event, NULL, SIGHUP | SD_EVENT_SIGNAL_PROCMASK, on_sighup, manager); - if (r < 0) - return log_error_errno(r, "Failed to create SIGHUP event source: %m"); - - r = sd_event_set_watchdog(manager->event, true); - if (r < 0) - return log_error_errno(r, "Failed to create watchdog event source: %m"); + assert(manager); + assert(manager->ctrl); r = udev_ctrl_attach_event(manager->ctrl, manager->event); if (r < 0) @@ -1321,17 +1318,21 @@ int manager_main(Manager *manager) { if (r < 0) return log_error_errno(r, "Failed to start udev control: %m"); - /* This needs to be after the inotify and uevent handling, to make sure - * that the ping is send back after fully processing the pending uevents - * (including the synthetic ones we may create due to inotify events). - */ + /* This needs to be after the inotify and uevent handling, to make sure that the ping is send back + * after fully processing the pending uevents (including the synthetic ones we may create due to + * inotify events). */ r = sd_event_source_set_priority(udev_ctrl_get_event_source(manager->ctrl), SD_EVENT_PRIORITY_IDLE); if (r < 0) return log_error_errno(r, "Failed to set IDLE event priority for udev control event source: %m"); - r = sd_event_add_io(manager->event, &manager->inotify_event, manager->inotify_fd, EPOLLIN, on_inotify, manager); - if (r < 0) - return log_error_errno(r, "Failed to create inotify event source: %m"); + return 0; +} + +static int manager_start_device_monitor(Manager *manager) { + int r; + + assert(manager); + assert(manager->monitor); r = sd_device_monitor_attach_event(manager->monitor, manager->event); if (r < 0) @@ -1341,25 +1342,129 @@ int manager_main(Manager *manager) { if (r < 0) return log_error_errno(r, "Failed to start device monitor: %m"); - r = sd_event_add_io(manager->event, NULL, fd_worker, EPOLLIN, on_worker, manager); + return 0; +} + +static int manager_start_inotify(Manager *manager) { + _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; + _cleanup_close_ int fd = -EBADF; + int r; + + assert(manager); + assert(manager->event); + + fd = inotify_init1(IN_CLOEXEC); + if (fd < 0) + return log_error_errno(errno, "Failed to create inotify descriptor: %m"); + + udev_watch_restore(fd); + + r = sd_event_add_io(manager->event, &s, fd, EPOLLIN, on_inotify, manager); + if (r < 0) + return log_error_errno(r, "Failed to create inotify event source: %m"); + + (void) sd_event_source_set_description(s, "manager-inotify"); + + manager->inotify_fd = TAKE_FD(fd); + manager->inotify_event = TAKE_PTR(s); + return 0; +} + +static int manager_start_worker_event(Manager *manager) { + int r; + + assert(manager); + assert(manager->event); + + /* unnamed socket from workers to the main daemon */ + r = socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, manager->worker_watch); + if (r < 0) + return log_error_errno(errno, "Failed to create socketpair for communicating with workers: %m"); + + r = setsockopt_int(manager->worker_watch[READ_END], SOL_SOCKET, SO_PASSCRED, true); + if (r < 0) + return log_error_errno(r, "Failed to enable SO_PASSCRED: %m"); + + r = sd_event_add_io(manager->event, NULL, manager->worker_watch[READ_END], EPOLLIN, on_worker, manager); if (r < 0) return log_error_errno(r, "Failed to create worker event source: %m"); - r = sd_event_add_post(manager->event, NULL, on_post, manager); + return 0; +} + +static int manager_setup_event(Manager *manager) { + _cleanup_(sd_event_unrefp) sd_event *e = NULL; + int r; + + assert(manager); + + /* block SIGCHLD for listening child events. */ + assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD) >= 0); + + r = sd_event_default(&e); + if (r < 0) + return log_error_errno(r, "Failed to allocate event loop: %m"); + + r = sd_event_add_signal(e, /* ret_event_source = */ NULL, SIGINT | SD_EVENT_SIGNAL_PROCMASK, on_sigterm, manager); + if (r < 0) + return log_error_errno(r, "Failed to create SIGINT event source: %m"); + + r = sd_event_add_signal(e, /* ret_event_source = */ NULL, SIGTERM | SD_EVENT_SIGNAL_PROCMASK, on_sigterm, manager); + if (r < 0) + return log_error_errno(r, "Failed to create SIGTERM event source: %m"); + + r = sd_event_add_signal(e, /* ret_event_source = */ NULL, SIGHUP | SD_EVENT_SIGNAL_PROCMASK, on_sighup, manager); + if (r < 0) + return log_error_errno(r, "Failed to create SIGHUP event source: %m"); + + r = sd_event_add_post(e, /* ret_event_source = */ NULL, on_post, manager); if (r < 0) return log_error_errno(r, "Failed to create post event source: %m"); /* Eventually, we probably want to do more here on memory pressure, for example, kill idle workers immediately */ - r = sd_event_add_memory_pressure(manager->event, /* ret_event_source= */ NULL, /* callback= */ NULL, /* userdata= */ NULL); + r = sd_event_add_memory_pressure(e, /* ret_event_source= */ NULL, /* callback= */ NULL, /* userdata= */ NULL); if (r < 0) log_full_errno(ERRNO_IS_NOT_SUPPORTED(r) || ERRNO_IS_PRIVILEGE(r) || (r == -EHOSTDOWN) ? LOG_DEBUG : LOG_WARNING, r, "Failed to allocate memory pressure watch, ignoring: %m"); - r = sd_event_add_signal(manager->event, /* ret_event_source= */ NULL, + r = sd_event_add_signal(e, /* ret_event_source= */ NULL, (SIGRTMIN+18) | SD_EVENT_SIGNAL_PROCMASK, sigrtmin18_handler, /* userdata= */ NULL); if (r < 0) return log_error_errno(r, "Failed to allocate SIGRTMIN+18 event source, ignoring: %m"); + r = sd_event_set_watchdog(e, true); + if (r < 0) + return log_error_errno(r, "Failed to create watchdog event source: %m"); + + manager->event = TAKE_PTR(e); + return 0; +} + +int manager_main(Manager *manager) { + int r; + + assert(manager); + + r = manager_setup_event(manager); + if (r < 0) + return r; + + r = manager_start_ctrl(manager); + if (r < 0) + return r; + + r = manager_start_device_monitor(manager); + if (r < 0) + return r; + + r = manager_start_inotify(manager); + if (r < 0) + return r; + + r = manager_start_worker_event(manager); + if (r < 0) + return r; + manager->last_usec = now(CLOCK_MONOTONIC); udev_builtin_init(); From 77eb6f73053a71ddd7e9156a409b138eb87c4cd6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 22 Dec 2024 00:14:52 +0900 Subject: [PATCH 2/2] udev: make worker event source take file descriptor No functional change, just refactoring. --- src/udev/udev-manager.c | 27 +++++++++++++++++++++------ src/udev/udev-manager.h | 2 +- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index d9bd9d6d7b..b0f542a11a 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -144,7 +144,7 @@ Manager* manager_free(Manager *manager) { event_queue_cleanup(manager, EVENT_UNDEF); safe_close(manager->inotify_fd); - safe_close_pair(manager->worker_watch); + safe_close(manager->worker_notify_fd); sd_device_monitor_unref(manager->monitor); udev_ctrl_unref(manager->ctrl); @@ -406,7 +406,7 @@ static int worker_spawn(Manager *manager, Event *event) { .monitor = TAKE_PTR(worker_monitor), .properties = TAKE_PTR(manager->properties), .rules = TAKE_PTR(manager->rules), - .pipe_fd = TAKE_FD(manager->worker_watch[WRITE_END]), + .pipe_fd = TAKE_FD(manager->worker_notify_fd), .inotify_fd = TAKE_FD(manager->inotify_fd), .config = manager->config, }; @@ -1173,7 +1173,7 @@ Manager* manager_new(void) { *manager = (Manager) { .inotify_fd = -EBADF, - .worker_watch = EBADF_PAIR, + .worker_notify_fd = -EBADF, .config_by_udev_conf = UDEV_CONFIG_INIT, .config_by_command = UDEV_CONFIG_INIT, .config_by_kernel = UDEV_CONFIG_INIT, @@ -1371,24 +1371,39 @@ static int manager_start_inotify(Manager *manager) { } static int manager_start_worker_event(Manager *manager) { + _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; + _cleanup_close_pair_ int pair[2] = EBADF_PAIR; int r; assert(manager); assert(manager->event); /* unnamed socket from workers to the main daemon */ - r = socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, manager->worker_watch); + r = socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, pair); if (r < 0) return log_error_errno(errno, "Failed to create socketpair for communicating with workers: %m"); - r = setsockopt_int(manager->worker_watch[READ_END], SOL_SOCKET, SO_PASSCRED, true); + r = setsockopt_int(pair[READ_END], SOL_SOCKET, SO_PASSCRED, true); if (r < 0) return log_error_errno(r, "Failed to enable SO_PASSCRED: %m"); - r = sd_event_add_io(manager->event, NULL, manager->worker_watch[READ_END], EPOLLIN, on_worker, manager); + r = sd_event_add_io(manager->event, &s, pair[READ_END], EPOLLIN, on_worker, manager); if (r < 0) return log_error_errno(r, "Failed to create worker event source: %m"); + (void) sd_event_source_set_description(s, "manager-worker-event"); + + r = sd_event_source_set_io_fd_own(s, true); + if (r < 0) + return log_error_errno(r, "Failed to make worker event source own file descriptor: %m"); + + TAKE_FD(pair[READ_END]); + + r = sd_event_source_set_floating(s, true); + if (r < 0) + return log_error_errno(r, "Failed to make worker event source floating: %m"); + + manager->worker_notify_fd = TAKE_FD(pair[WRITE_END]); return 0; } diff --git a/src/udev/udev-manager.h b/src/udev/udev-manager.h index 13c7242ea8..4f0294a94f 100644 --- a/src/udev/udev-manager.h +++ b/src/udev/udev-manager.h @@ -28,7 +28,7 @@ typedef struct Manager { sd_device_monitor *monitor; UdevCtrl *ctrl; - int worker_watch[2]; + int worker_notify_fd; /* used by udev-watch */ int inotify_fd;