From ef0f8397a57d963b86082aaeb50dca5bdbbc5f98 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Apr 2025 02:55:36 +0900 Subject: [PATCH 1/6] udev: move inotify watch related functions to udev-watch.c --- src/udev/udev-manager.c | 185 +------------------------------------- src/udev/udev-manager.h | 2 + src/udev/udev-watch.c | 193 ++++++++++++++++++++++++++++++++++++++-- src/udev/udev-watch.h | 8 +- 4 files changed, 194 insertions(+), 194 deletions(-) diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index e319277c2b..5303c4d253 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ -#include "blockdev-util.h" #include "cgroup-util.h" #include "common-signal.h" #include "daemon-util.h" @@ -12,7 +11,6 @@ #include "fd-util.h" #include "fs-util.h" #include "hashmap.h" -#include "inotify-util.h" #include "iovec-util.h" #include "list.h" #include "mkdir.h" @@ -705,7 +703,7 @@ fail: event_free(event); } -static int event_queue_assume_block_device_unlocked(Manager *manager, sd_device *dev) { +int event_queue_assume_block_device_unlocked(Manager *manager, sd_device *dev) { const char *devname; int r; @@ -858,162 +856,6 @@ static int on_worker_notify(sd_event_source *s, int fd, uint32_t revents, void * return 0; } -static int synthesize_change_one(sd_device *dev, sd_device *target) { - int r; - - assert(dev); - assert(target); - - if (DEBUG_LOGGING) { - const char *syspath = NULL; - (void) sd_device_get_syspath(target, &syspath); - log_device_debug(dev, "device is closed, synthesising 'change' on %s", strna(syspath)); - } - - r = sd_device_trigger(target, SD_DEVICE_CHANGE); - if (r < 0) - return log_device_debug_errno(target, r, "Failed to trigger 'change' uevent: %m"); - - DEVICE_TRACE_POINT(synthetic_change_event, dev); - - return 0; -} - -static int synthesize_change_all(sd_device *dev) { - int r; - - assert(dev); - - r = blockdev_reread_partition_table(dev); - if (r < 0) - log_device_debug_errno(dev, r, "Failed to re-read partition table, ignoring: %m"); - bool part_table_read = r >= 0; - - /* search for partitions */ - _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; - r = partition_enumerator_new(dev, &e); - if (r < 0) - return log_device_debug_errno(dev, r, "Failed to initialize partition enumerator, ignoring: %m"); - - /* We have partitions and re-read the table, the kernel already sent out a "change" - * event for the disk, and "remove/add" for all partitions. */ - if (part_table_read && sd_device_enumerator_get_device_first(e)) - return 0; - - /* We have partitions but re-reading the partition table did not work, synthesize - * "change" for the disk and all partitions. */ - r = synthesize_change_one(dev, dev); - FOREACH_DEVICE(e, d) - RET_GATHER(r, synthesize_change_one(dev, d)); - - return r; -} - -static int synthesize_change_child_handler(sd_event_source *s, const siginfo_t *si, void *userdata) { - Manager *manager = ASSERT_PTR(userdata); - assert(s); - - sd_event_source_unref(set_remove(manager->synthesize_change_child_event_sources, s)); - return 0; -} - -static int synthesize_change(Manager *manager, sd_device *dev) { - int r; - - assert(manager); - assert(dev); - - const char *sysname; - r = sd_device_get_sysname(dev, &sysname); - if (r < 0) - return r; - - if (startswith(sysname, "dm-") || block_device_is_whole_disk(dev) <= 0) - return synthesize_change_one(dev, dev); - - _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; - r = pidref_safe_fork( - "(udev-synth)", - FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_RLIMIT_NOFILE_SAFE, - &pidref); - if (r < 0) - return r; - if (r == 0) { - /* child */ - (void) synthesize_change_all(dev); - _exit(EXIT_SUCCESS); - } - - _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; - r = event_add_child_pidref(manager->event, &s, &pidref, WEXITED, synthesize_change_child_handler, manager); - if (r < 0) { - log_debug_errno(r, "Failed to add child event source for "PID_FMT", ignoring: %m", pidref.pid); - return 0; - } - - r = sd_event_source_set_child_pidfd_own(s, true); - if (r < 0) - return r; - TAKE_PIDREF(pidref); - - r = set_ensure_put(&manager->synthesize_change_child_event_sources, &event_source_hash_ops, s); - if (r < 0) - return r; - TAKE_PTR(s); - - return 0; -} - -static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userdata) { - Manager *manager = ASSERT_PTR(userdata); - union inotify_event_buffer buffer; - ssize_t l; - int r; - - l = read(fd, &buffer, sizeof(buffer)); - if (l < 0) { - if (ERRNO_IS_TRANSIENT(errno)) - return 0; - - return log_error_errno(errno, "Failed to read inotify fd: %m"); - } - - FOREACH_INOTIFY_EVENT_WARN(e, buffer, l) { - _cleanup_(sd_device_unrefp) sd_device *dev = NULL; - const char *devnode; - - /* Do not handle IN_IGNORED here. Especially, do not try to call udev_watch_end() from the - * main process. Otherwise, the pair of the symlinks may become inconsistent, and several - * garbage may remain. The old symlinks are removed by a worker that processes the - * corresponding 'remove' uevent; - * udev_event_execute_rules() -> event_execute_rules_on_remove() -> udev_watch_end(). */ - - if (!FLAGS_SET(e->mask, IN_CLOSE_WRITE)) - continue; - - r = device_new_from_watch_handle(&dev, e->wd); - if (r < 0) { - /* Device may be removed just after closed. */ - log_debug_errno(r, "Failed to create sd_device object from watch handle, ignoring: %m"); - continue; - } - - r = sd_device_get_devname(dev, &devnode); - if (r < 0) { - /* Also here, device may be already removed. */ - log_device_debug_errno(dev, r, "Failed to get device node, ignoring: %m"); - continue; - } - - log_device_debug(dev, "Received inotify event for %s.", devnode); - - (void) event_queue_assume_block_device_unlocked(manager, dev); - (void) synthesize_change(manager, dev); - } - - return 0; -} - static int on_sigterm(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) { Manager *manager = ASSERT_PTR(userdata); @@ -1252,31 +1094,6 @@ static int manager_start_device_monitor(Manager *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_notify(Manager *manager) { int r; diff --git a/src/udev/udev-manager.h b/src/udev/udev-manager.h index dab271abaa..3ed393c1f0 100644 --- a/src/udev/udev-manager.h +++ b/src/udev/udev-manager.h @@ -66,3 +66,5 @@ void notify_ready(Manager *manager); void manager_kill_workers(Manager *manager, bool force); bool devpath_conflict(const char *a, const char *b); + +int event_queue_assume_block_device_unlocked(Manager *manager, sd_device *dev); diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index 0254cf6ed9..c4b63a4d2f 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -4,23 +4,25 @@ * Copyright © 2009 Scott James Remnant */ -#include - #include "alloc-util.h" -#include "device-private.h" +#include "blockdev-util.h" #include "device-util.h" #include "dirent-util.h" +#include "event-util.h" #include "fd-util.h" #include "fs-util.h" +#include "inotify-util.h" #include "mkdir.h" #include "parse-util.h" #include "rm-rf.h" #include "stdio-util.h" #include "string-util.h" +#include "udev-manager.h" +#include "udev-trace.h" #include "udev-util.h" #include "udev-watch.h" -int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd) { +static int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd) { char path_wd[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; _cleanup_free_ char *id = NULL; int r; @@ -43,7 +45,163 @@ int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd) { return sd_device_new_from_device_id(ret, id); } -int udev_watch_restore(int inotify_fd) { +static int synthesize_change_one(sd_device *dev, sd_device *target) { + int r; + + assert(dev); + assert(target); + + if (DEBUG_LOGGING) { + const char *syspath = NULL; + (void) sd_device_get_syspath(target, &syspath); + log_device_debug(dev, "device is closed, synthesising 'change' on %s", strna(syspath)); + } + + r = sd_device_trigger(target, SD_DEVICE_CHANGE); + if (r < 0) + return log_device_debug_errno(target, r, "Failed to trigger 'change' uevent: %m"); + + DEVICE_TRACE_POINT(synthetic_change_event, dev); + + return 0; +} + +static int synthesize_change_all(sd_device *dev) { + int r; + + assert(dev); + + r = blockdev_reread_partition_table(dev); + if (r < 0) + log_device_debug_errno(dev, r, "Failed to re-read partition table, ignoring: %m"); + bool part_table_read = r >= 0; + + /* search for partitions */ + _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; + r = partition_enumerator_new(dev, &e); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to initialize partition enumerator, ignoring: %m"); + + /* We have partitions and re-read the table, the kernel already sent out a "change" + * event for the disk, and "remove/add" for all partitions. */ + if (part_table_read && sd_device_enumerator_get_device_first(e)) + return 0; + + /* We have partitions but re-reading the partition table did not work, synthesize + * "change" for the disk and all partitions. */ + r = synthesize_change_one(dev, dev); + FOREACH_DEVICE(e, d) + RET_GATHER(r, synthesize_change_one(dev, d)); + + return r; +} + +static int synthesize_change_child_handler(sd_event_source *s, const siginfo_t *si, void *userdata) { + Manager *manager = ASSERT_PTR(userdata); + assert(s); + + sd_event_source_unref(set_remove(manager->synthesize_change_child_event_sources, s)); + return 0; +} + +static int synthesize_change(Manager *manager, sd_device *dev) { + int r; + + assert(manager); + assert(dev); + + const char *sysname; + r = sd_device_get_sysname(dev, &sysname); + if (r < 0) + return r; + + if (startswith(sysname, "dm-") || block_device_is_whole_disk(dev) <= 0) + return synthesize_change_one(dev, dev); + + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + r = pidref_safe_fork( + "(udev-synth)", + FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_RLIMIT_NOFILE_SAFE, + &pidref); + if (r < 0) + return r; + if (r == 0) { + /* child */ + (void) synthesize_change_all(dev); + _exit(EXIT_SUCCESS); + } + + _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; + r = event_add_child_pidref(manager->event, &s, &pidref, WEXITED, synthesize_change_child_handler, manager); + if (r < 0) { + log_debug_errno(r, "Failed to add child event source for "PID_FMT", ignoring: %m", pidref.pid); + return 0; + } + + r = sd_event_source_set_child_pidfd_own(s, true); + if (r < 0) + return r; + TAKE_PIDREF(pidref); + + r = set_ensure_put(&manager->synthesize_change_child_event_sources, &event_source_hash_ops, s); + if (r < 0) + return r; + TAKE_PTR(s); + + return 0; +} + +static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userdata) { + Manager *manager = ASSERT_PTR(userdata); + union inotify_event_buffer buffer; + ssize_t l; + int r; + + l = read(fd, &buffer, sizeof(buffer)); + if (l < 0) { + if (ERRNO_IS_TRANSIENT(errno)) + return 0; + + return log_error_errno(errno, "Failed to read inotify fd: %m"); + } + + FOREACH_INOTIFY_EVENT_WARN(e, buffer, l) { + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + const char *devnode; + + /* Do not handle IN_IGNORED here. Especially, do not try to call udev_watch_end() from the + * main process. Otherwise, the pair of the symlinks may become inconsistent, and several + * garbage may remain. The old symlinks are removed by a worker that processes the + * corresponding 'remove' uevent; + * udev_event_execute_rules() -> event_execute_rules_on_remove() -> udev_watch_end(). */ + + if (!FLAGS_SET(e->mask, IN_CLOSE_WRITE)) + continue; + + r = device_new_from_watch_handle_at(&dev, -EBADF, e->wd); + if (r < 0) { + /* Device may be removed just after closed. */ + log_debug_errno(r, "Failed to create sd_device object from watch handle, ignoring: %m"); + continue; + } + + r = sd_device_get_devname(dev, &devnode); + if (r < 0) { + /* Also here, device may be already removed. */ + log_device_debug_errno(dev, r, "Failed to get device node, ignoring: %m"); + continue; + } + + log_device_debug(dev, "Received inotify event for %s.", devnode); + + (void) event_queue_assume_block_device_unlocked(manager, dev); + (void) synthesize_change(manager, dev); + } + + return 0; +} + +static int udev_watch_restore(int inotify_fd) { _cleanup_(rm_rf_safep) const char *old = "/run/udev/watch.old/"; int r; @@ -88,6 +246,31 @@ int udev_watch_restore(int inotify_fd) { return 0; } +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 udev_watch_clear(sd_device *dev, int dirfd, int *ret_wd) { _cleanup_free_ char *wd_str = NULL, *buf = NULL; const char *id; diff --git a/src/udev/udev-watch.h b/src/udev/udev-watch.h index c454deead3..3976a835a1 100644 --- a/src/udev/udev-watch.h +++ b/src/udev/udev-watch.h @@ -3,11 +3,9 @@ #include "sd-device.h" -int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd); -static inline int device_new_from_watch_handle(sd_device **ret, int wd) { - return device_new_from_watch_handle_at(ret, -1, wd); -} +typedef struct Manager Manager; + +int manager_start_inotify(Manager *manager); -int udev_watch_restore(int inotify_fd); int udev_watch_begin(int inotify_fd, sd_device *dev); int udev_watch_end(int inotify_fd, sd_device *dev); From 5edb07be621cf58ba09318037159e1e61747715f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Apr 2025 03:01:03 +0900 Subject: [PATCH 2/6] udev-watch: split-out manager_process_inotify() from on_inotify() No functional change, just refactoring. --- src/udev/udev-watch.c | 71 +++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index c4b63a4d2f..4b76fd151f 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -151,13 +151,40 @@ static int synthesize_change(Manager *manager, sd_device *dev) { return 0; } -static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userdata) { - Manager *manager = ASSERT_PTR(userdata); - union inotify_event_buffer buffer; - ssize_t l; +static int manager_process_inotify(Manager *manager, const struct inotify_event *e) { int r; - l = read(fd, &buffer, sizeof(buffer)); + assert(manager); + assert(e); + + /* Do not handle IN_IGNORED here. Especially, do not try to call udev_watch_end() from the + * main process. Otherwise, the pair of the symlinks may become inconsistent, and several + * garbage may remain. The old symlinks are removed by a worker that processes the + * corresponding 'remove' uevent; + * udev_event_execute_rules() -> event_execute_rules_on_remove() -> udev_watch_end(). */ + + if (!FLAGS_SET(e->mask, IN_CLOSE_WRITE)) + return 0; + + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + r = device_new_from_watch_handle_at(&dev, -EBADF, e->wd); + if (r < 0) /* Device may be removed just after closed. */ + return log_debug_errno(r, "Failed to create sd_device object from watch handle, ignoring: %m"); + + log_device_debug(dev, "Received inotify event of watch handle %i.", e->wd); + + (void) event_queue_assume_block_device_unlocked(manager, dev); + (void) synthesize_change(manager, dev); + return 0; +} + +static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userdata) { + Manager *manager = ASSERT_PTR(userdata); + + assert(fd >= 0); + + union inotify_event_buffer buffer; + ssize_t l = read(fd, &buffer, sizeof(buffer)); if (l < 0) { if (ERRNO_IS_TRANSIENT(errno)) return 0; @@ -165,38 +192,8 @@ static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userda return log_error_errno(errno, "Failed to read inotify fd: %m"); } - FOREACH_INOTIFY_EVENT_WARN(e, buffer, l) { - _cleanup_(sd_device_unrefp) sd_device *dev = NULL; - const char *devnode; - - /* Do not handle IN_IGNORED here. Especially, do not try to call udev_watch_end() from the - * main process. Otherwise, the pair of the symlinks may become inconsistent, and several - * garbage may remain. The old symlinks are removed by a worker that processes the - * corresponding 'remove' uevent; - * udev_event_execute_rules() -> event_execute_rules_on_remove() -> udev_watch_end(). */ - - if (!FLAGS_SET(e->mask, IN_CLOSE_WRITE)) - continue; - - r = device_new_from_watch_handle_at(&dev, -EBADF, e->wd); - if (r < 0) { - /* Device may be removed just after closed. */ - log_debug_errno(r, "Failed to create sd_device object from watch handle, ignoring: %m"); - continue; - } - - r = sd_device_get_devname(dev, &devnode); - if (r < 0) { - /* Also here, device may be already removed. */ - log_device_debug_errno(dev, r, "Failed to get device node, ignoring: %m"); - continue; - } - - log_device_debug(dev, "Received inotify event for %s.", devnode); - - (void) event_queue_assume_block_device_unlocked(manager, dev); - (void) synthesize_change(manager, dev); - } + FOREACH_INOTIFY_EVENT_WARN(e, buffer, l) + (void) manager_process_inotify(manager, e); return 0; } From beaf7e04ebdaf2d82e6b13eb24590c9e4560e0e0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Apr 2025 03:09:31 +0900 Subject: [PATCH 3/6] udev: push inotify fd to file descriptor store Then, if we get inotify fd on start, it is not necessary to re-enable inotify watch. --- src/udev/udev-manager.c | 2 ++ src/udev/udev-watch.c | 47 +++++++++++++++++++++++++++++----- src/udev/udev-watch.h | 1 + units/systemd-udevd.service.in | 2 ++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index 5303c4d253..c05e475dd6 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -1041,6 +1041,8 @@ static int manager_listen_fds(Manager *manager) { r = manager_init_ctrl(manager, fd); else if (streq(names[i], "systemd-udevd-kernel.socket")) r = manager_init_device_monitor(manager, fd); + else if (streq(names[i], "inotify")) + r = manager_init_inotify(manager, fd); else r = log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Received unexpected fd (%s), ignoring.", names[i]); diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index 4b76fd151f..a9b06f5eb8 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -6,6 +6,7 @@ #include "alloc-util.h" #include "blockdev-util.h" +#include "daemon-util.h" #include "device-util.h" #include "dirent-util.h" #include "event-util.h" @@ -243,27 +244,59 @@ static int udev_watch_restore(int inotify_fd) { return 0; } -int manager_start_inotify(Manager *manager) { - _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; - _cleanup_close_ int fd = -EBADF; +int manager_init_inotify(Manager *manager, int fd) { int r; assert(manager); - assert(manager->event); + + /* This takes passed file descriptor on success. */ + + if (fd >= 0) { + if (manager->inotify_fd >= 0) + return log_warning_errno(SYNTHETIC_ERRNO(EALREADY), "Received multiple inotify fd (%i), ignoring.", fd); + + log_debug("Received inotify fd (%i) from service manager.", fd); + manager->inotify_fd = fd; + return 0; + } + + if (manager->inotify_fd >= 0) + return 0; fd = inotify_init1(IN_CLOEXEC); if (fd < 0) return log_error_errno(errno, "Failed to create inotify descriptor: %m"); - udev_watch_restore(fd); + log_debug("Initialized new inotify instance, restoring inotify watches of previous invocation."); + manager->inotify_fd = fd; + (void) udev_watch_restore(fd); - r = sd_event_add_io(manager->event, &s, fd, EPOLLIN, on_inotify, manager); + r = notify_push_fd(manager->inotify_fd, "inotify"); + if (r < 0) + log_warning_errno(r, "Failed to push inotify fd to service manager, ignoring: %m"); + else + log_debug("Pushed inotify fd to service manager."); + + return 0; +} + +int manager_start_inotify(Manager *manager) { + int r; + + assert(manager); + assert(manager->event); + + r = manager_init_inotify(manager, -EBADF); + if (r < 0) + return r; + + _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; + r = sd_event_add_io(manager->event, &s, manager->inotify_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; } diff --git a/src/udev/udev-watch.h b/src/udev/udev-watch.h index 3976a835a1..8365334e71 100644 --- a/src/udev/udev-watch.h +++ b/src/udev/udev-watch.h @@ -5,6 +5,7 @@ typedef struct Manager Manager; +int manager_init_inotify(Manager *manager, int fd); int manager_start_inotify(Manager *manager); int udev_watch_begin(int inotify_fd, sd_device *dev); diff --git a/units/systemd-udevd.service.in b/units/systemd-udevd.service.in index 13ec7088da..8e343ef836 100644 --- a/units/systemd-udevd.service.in +++ b/units/systemd-udevd.service.in @@ -27,6 +27,8 @@ Sockets=systemd-udevd-control.socket systemd-udevd-kernel.socket systemd-udevd-v Restart=always RestartSec=0 ExecStart={{LIBEXECDIR}}/systemd-udevd +FileDescriptorStoreMax=512 +FileDescriptorStorePreserve=yes KillMode=mixed TasksMax=infinity PrivateMounts=yes From 04ae22375adfb995b086be6ae0f57bd4260f221d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 4 Apr 2025 22:42:24 +0900 Subject: [PATCH 4/6] udev-watch: dump installed inotify watches on start and stop --- src/udev/udev-manager.c | 4 +- src/udev/udev-watch.c | 84 +++++++++++++++++++++++++++++++++++++++++ src/udev/udev-watch.h | 2 + 3 files changed, 89 insertions(+), 1 deletion(-) diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index c05e475dd6..eddd07854d 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -965,8 +965,10 @@ static int on_post(sd_event_source *s, void *userdata) { if (r < 0) log_warning_errno(r, "Failed to disable timer event source for cleaning up idle workers, ignoring: %m"); - if (manager->exit) + if (manager->exit) { + udev_watch_dump(); return sd_event_exit(manager->event, 0); + } if (manager->cgroup && set_isempty(manager->synthesize_change_child_event_sources)) /* cleanup possible left-over processes in our cgroup */ diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index a9b06f5eb8..84a7f542b1 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -46,6 +46,88 @@ static int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd) { return sd_device_new_from_device_id(ret, id); } +void udev_watch_dump(void) { + int r; + + _cleanup_closedir_ DIR *dir = opendir("/run/udev/watch/"); + if (!dir) + return (void) log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_WARNING, errno, + "Failed to open old watches directory '/run/udev/watch/': %m"); + + _cleanup_set_free_ Set *pending_wds = NULL, *verified_wds = NULL; + FOREACH_DIRENT(de, dir, break) { + if (safe_atoi(de->d_name, NULL) >= 0) { + /* This should be wd -> ID symlink */ + + if (set_contains(verified_wds, de->d_name)) + continue; + + r = set_put_strdup(&pending_wds, de->d_name); + if (r < 0) + log_warning_errno(r, "Failed to store pending watch handle %s, ignoring: %m", de->d_name); + continue; + } + + _cleanup_free_ char *wd = NULL; + r = readlinkat_malloc(dirfd(dir), de->d_name, &wd); + if (r < 0) { + log_warning_errno(r, "Found broken inotify watch, failed to read symlink %s, ignoring: %m", de->d_name); + continue; + } + + const char *devnode = NULL; + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + if (sd_device_new_from_device_id(&dev, de->d_name) >= 0) + (void) sd_device_get_devname(dev, &devnode); + + _cleanup_free_ char *id = NULL; + r = readlinkat_malloc(dirfd(dir), wd, &id); + if (r < 0) { + log_warning_errno(r, "Found broken inotify watch %s on %s (%s), failed to read symlink %s, ignoring: %m", + wd, strna(devnode), de->d_name, wd); + continue; + } + + if (!streq(de->d_name, id)) { + log_warning("Found broken inotify watch %s on %s (%s), broken symlink chain: %s → %s → %s", + wd, strna(devnode), de->d_name, de->d_name, wd, id); + continue; + } + + log_debug("Found inotify watch %s on %s (%s).", wd, strna(devnode), de->d_name); + + free(set_remove(pending_wds, wd)); + + r = set_ensure_put(&verified_wds, &string_hash_ops_free, wd); + if (r < 0) { + log_warning_errno(r, "Failed to store verified watch handle %s, ignoring: %m", wd); + continue; + } + TAKE_PTR(wd); + } + + const char *w; + SET_FOREACH(w, pending_wds) { + _cleanup_free_ char *id = NULL; + r = readlinkat_malloc(dirfd(dir), w, &id); + if (r < 0) { + log_warning_errno(r, "Found broken inotify watch %s, failed to read symlink %s, ignoring: %m", w, w); + continue; + } + + const char *devnode = NULL; + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + if (sd_device_new_from_device_id(&dev, id) >= 0) + (void) sd_device_get_devname(dev, &devnode); + + _cleanup_free_ char *wd = NULL; + (void) readlinkat_malloc(dirfd(dir), id, &wd); + + log_warning("Found broken inotify watch %s on %s (%s), broken symlink chain: %s → %s → %s", + wd, strna(devnode), id, w, id, wd); + } +} + static int synthesize_change_one(sd_device *dev, sd_device *target) { int r; @@ -290,6 +372,8 @@ int manager_start_inotify(Manager *manager) { if (r < 0) return r; + udev_watch_dump(); + _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; r = sd_event_add_io(manager->event, &s, manager->inotify_fd, EPOLLIN, on_inotify, manager); if (r < 0) diff --git a/src/udev/udev-watch.h b/src/udev/udev-watch.h index 8365334e71..fed5f9614c 100644 --- a/src/udev/udev-watch.h +++ b/src/udev/udev-watch.h @@ -5,6 +5,8 @@ typedef struct Manager Manager; +void udev_watch_dump(void); + int manager_init_inotify(Manager *manager, int fd); int manager_start_inotify(Manager *manager); From 40959dcc028a6884fbea00c11d89217a77716d4d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 5 Apr 2025 05:47:32 +0900 Subject: [PATCH 5/6] TEST-17-UDEV: rename subtests --- test/units/{TEST-17-UDEV.05.sh => TEST-17-UDEV.IMPORT.sh} | 0 test/units/{TEST-17-UDEV.08.sh => TEST-17-UDEV.SYSTEMD_ALIAS.sh} | 0 test/units/{TEST-17-UDEV.01.sh => TEST-17-UDEV.SYSTEMD_WANTS.sh} | 0 ...EV.07.sh => TEST-17-UDEV.SYSTEMD_WANTS_vs_StopWhenUnneeded.sh} | 0 test/units/{TEST-17-UDEV.04.sh => TEST-17-UDEV.TAG.sh} | 0 test/units/{TEST-17-UDEV.09.sh => TEST-17-UDEV.buffer-size.sh} | 0 test/units/{TEST-17-UDEV.03.sh => TEST-17-UDEV.failed-event.sh} | 0 .../units/{TEST-17-UDEV.13.sh => TEST-17-UDEV.global-property.sh} | 0 test/units/{TEST-17-UDEV.12.sh => TEST-17-UDEV.netif-altname.sh} | 0 test/units/{TEST-17-UDEV.00.sh => TEST-17-UDEV.owner-and-mode.sh} | 0 test/units/{TEST-17-UDEV.02.sh => TEST-17-UDEV.rename-netif.sh} | 0 test/units/{TEST-17-UDEV.10.sh => TEST-17-UDEV.sanity-check.sh} | 0 test/units/{TEST-17-UDEV.11.sh => TEST-17-UDEV.verify.sh} | 0 test/units/{TEST-17-UDEV.06.sh => TEST-17-UDEV.watch.sh} | 0 14 files changed, 0 insertions(+), 0 deletions(-) rename test/units/{TEST-17-UDEV.05.sh => TEST-17-UDEV.IMPORT.sh} (100%) rename test/units/{TEST-17-UDEV.08.sh => TEST-17-UDEV.SYSTEMD_ALIAS.sh} (100%) rename test/units/{TEST-17-UDEV.01.sh => TEST-17-UDEV.SYSTEMD_WANTS.sh} (100%) rename test/units/{TEST-17-UDEV.07.sh => TEST-17-UDEV.SYSTEMD_WANTS_vs_StopWhenUnneeded.sh} (100%) rename test/units/{TEST-17-UDEV.04.sh => TEST-17-UDEV.TAG.sh} (100%) rename test/units/{TEST-17-UDEV.09.sh => TEST-17-UDEV.buffer-size.sh} (100%) rename test/units/{TEST-17-UDEV.03.sh => TEST-17-UDEV.failed-event.sh} (100%) rename test/units/{TEST-17-UDEV.13.sh => TEST-17-UDEV.global-property.sh} (100%) rename test/units/{TEST-17-UDEV.12.sh => TEST-17-UDEV.netif-altname.sh} (100%) rename test/units/{TEST-17-UDEV.00.sh => TEST-17-UDEV.owner-and-mode.sh} (100%) rename test/units/{TEST-17-UDEV.02.sh => TEST-17-UDEV.rename-netif.sh} (100%) rename test/units/{TEST-17-UDEV.10.sh => TEST-17-UDEV.sanity-check.sh} (100%) rename test/units/{TEST-17-UDEV.11.sh => TEST-17-UDEV.verify.sh} (100%) rename test/units/{TEST-17-UDEV.06.sh => TEST-17-UDEV.watch.sh} (100%) diff --git a/test/units/TEST-17-UDEV.05.sh b/test/units/TEST-17-UDEV.IMPORT.sh similarity index 100% rename from test/units/TEST-17-UDEV.05.sh rename to test/units/TEST-17-UDEV.IMPORT.sh diff --git a/test/units/TEST-17-UDEV.08.sh b/test/units/TEST-17-UDEV.SYSTEMD_ALIAS.sh similarity index 100% rename from test/units/TEST-17-UDEV.08.sh rename to test/units/TEST-17-UDEV.SYSTEMD_ALIAS.sh diff --git a/test/units/TEST-17-UDEV.01.sh b/test/units/TEST-17-UDEV.SYSTEMD_WANTS.sh similarity index 100% rename from test/units/TEST-17-UDEV.01.sh rename to test/units/TEST-17-UDEV.SYSTEMD_WANTS.sh diff --git a/test/units/TEST-17-UDEV.07.sh b/test/units/TEST-17-UDEV.SYSTEMD_WANTS_vs_StopWhenUnneeded.sh similarity index 100% rename from test/units/TEST-17-UDEV.07.sh rename to test/units/TEST-17-UDEV.SYSTEMD_WANTS_vs_StopWhenUnneeded.sh diff --git a/test/units/TEST-17-UDEV.04.sh b/test/units/TEST-17-UDEV.TAG.sh similarity index 100% rename from test/units/TEST-17-UDEV.04.sh rename to test/units/TEST-17-UDEV.TAG.sh diff --git a/test/units/TEST-17-UDEV.09.sh b/test/units/TEST-17-UDEV.buffer-size.sh similarity index 100% rename from test/units/TEST-17-UDEV.09.sh rename to test/units/TEST-17-UDEV.buffer-size.sh diff --git a/test/units/TEST-17-UDEV.03.sh b/test/units/TEST-17-UDEV.failed-event.sh similarity index 100% rename from test/units/TEST-17-UDEV.03.sh rename to test/units/TEST-17-UDEV.failed-event.sh diff --git a/test/units/TEST-17-UDEV.13.sh b/test/units/TEST-17-UDEV.global-property.sh similarity index 100% rename from test/units/TEST-17-UDEV.13.sh rename to test/units/TEST-17-UDEV.global-property.sh diff --git a/test/units/TEST-17-UDEV.12.sh b/test/units/TEST-17-UDEV.netif-altname.sh similarity index 100% rename from test/units/TEST-17-UDEV.12.sh rename to test/units/TEST-17-UDEV.netif-altname.sh diff --git a/test/units/TEST-17-UDEV.00.sh b/test/units/TEST-17-UDEV.owner-and-mode.sh similarity index 100% rename from test/units/TEST-17-UDEV.00.sh rename to test/units/TEST-17-UDEV.owner-and-mode.sh diff --git a/test/units/TEST-17-UDEV.02.sh b/test/units/TEST-17-UDEV.rename-netif.sh similarity index 100% rename from test/units/TEST-17-UDEV.02.sh rename to test/units/TEST-17-UDEV.rename-netif.sh diff --git a/test/units/TEST-17-UDEV.10.sh b/test/units/TEST-17-UDEV.sanity-check.sh similarity index 100% rename from test/units/TEST-17-UDEV.10.sh rename to test/units/TEST-17-UDEV.sanity-check.sh diff --git a/test/units/TEST-17-UDEV.11.sh b/test/units/TEST-17-UDEV.verify.sh similarity index 100% rename from test/units/TEST-17-UDEV.11.sh rename to test/units/TEST-17-UDEV.verify.sh diff --git a/test/units/TEST-17-UDEV.06.sh b/test/units/TEST-17-UDEV.watch.sh similarity index 100% rename from test/units/TEST-17-UDEV.06.sh rename to test/units/TEST-17-UDEV.watch.sh From db5d89309a6df2b505c52d70bf5a638190777027 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 4 Apr 2025 23:26:03 +0900 Subject: [PATCH 6/6] TEST-17-UDEV: check journal about inotify watch --- test/units/TEST-17-UDEV.watch.sh | 53 +++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/test/units/TEST-17-UDEV.watch.sh b/test/units/TEST-17-UDEV.watch.sh index 410f5a7f77..bf82ac3712 100755 --- a/test/units/TEST-17-UDEV.watch.sh +++ b/test/units/TEST-17-UDEV.watch.sh @@ -5,6 +5,9 @@ set -o pipefail # tests for udev watch +# shellcheck source=test/units/util.sh +. "$(dirname "$0")"/util.sh + function check_validity() { local f ID_OR_HANDLE @@ -12,15 +15,28 @@ function check_validity() { ID_OR_HANDLE="$(readlink "$f")" test -L "/run/udev/watch/${ID_OR_HANDLE}" test "$(readlink "/run/udev/watch/${ID_OR_HANDLE}")" = "$(basename "$f")" + + if [[ "${1:-}" == "1" ]]; then + journalctl -n 1 -q -u systemd-udevd.service --invocation=0 --grep "Found inotify watch .*$ID_OR_HANDLE" + fi done } function check() { for _ in {1..2}; do + systemctl reset-failed systemd-udevd.service systemctl restart systemd-udevd.service - udevadm control --ping udevadm settle --timeout=30 - check_validity + + journalctl --sync + + # Check if the inotify watch fd is received from fd store. + journalctl -n 1 -q -u systemd-udevd.service --invocation=0 --grep 'Received inotify fd \(\d\) from service manager.' + + # Check if there is no broken symlink chain. + assert_eq "$(journalctl -n 1 -q -u systemd-udevd.service --invocation=0 --grep 'Found broken inotify watch' || :)" "" + + check_validity 1 for _ in {1..2}; do udevadm trigger -w --action add --subsystem-match=block @@ -34,6 +50,24 @@ function check() { done } +# Check if the first invocation (should be in initrd) pushed the inotify fd to fdstore, +# and the next invocation gained the fd from service manager. +# TNote the service may be started without generating debugging logs. Let's check failure log. +if ! journalctl -n 1 -q -u systemd-udevd.service --invocation=1 --grep 'Pushed inotify fd to service manager.'; then + assert_eq "$(journalctl -n 1 -q -u systemd-udevd.service --invocation=1 --grep 'Failed to push inotify fd to service manager.' || :)" "" +fi +if ! journalctl -n 1 -q -u systemd-udevd.service --invocation=2 --grep 'Received inotify fd \(\d\) from service manager.'; then + assert_eq "$(journalctl -n 1 -q -u systemd-udevd.service --invocation=2 --grep 'Pushed inotify fd to service manager.' || :)" "" +fi + +mkdir -p /run/systemd/system/systemd-udevd.service.d/ +cat >/run/systemd/system/systemd-udevd.service.d/10-debug.conf </run/udev/rules.d/00-debug.rules </run/udev/rules.d/50-testsuite.rules <