From 0f72af536f957b5755551de5f1815baef8f377b7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 4 Dec 2024 04:31:31 +0900 Subject: [PATCH 1/3] udev: reload .rules files and builtins only when necessary Previously, even if e.g. .rules files are unchanged, all .rules files are reloaded when other kind of config files like .link files or .hwdb.bin are changed, vice versa. --- src/udev/udev-builtin.c | 23 ++++++++++++++++++++--- src/udev/udev-builtin.h | 3 ++- src/udev/udev-def.h | 23 +++++++++++++++++++++++ src/udev/udev-manager.c | 15 +++++++++------ 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/udev/udev-builtin.c b/src/udev/udev-builtin.c index 41a39b5cd62..b47ca37d55b 100644 --- a/src/udev/udev-builtin.c +++ b/src/udev/udev-builtin.c @@ -42,11 +42,28 @@ void udev_builtin_exit(void) { (*b)->exit(); } -bool udev_builtin_should_reload(void) { +UdevReloadFlags udev_builtin_should_reload(void) { + UdevReloadFlags flags = 0; + for (UdevBuiltinCommand i = 0; i < _UDEV_BUILTIN_MAX; i++) if (builtins[i] && builtins[i]->should_reload && builtins[i]->should_reload()) - return true; - return false; + flags |= 1u << i; + + if (flags != 0) + flags |= UDEV_RELOAD_KILL_WORKERS; + + return flags; +} + +void udev_builtin_reload(UdevReloadFlags flags) { + for (UdevBuiltinCommand i = 0; i < _UDEV_BUILTIN_MAX; i++) { + if (!FLAGS_SET(flags, 1u << i) || !builtins[i]) + continue; + if (builtins[i]->exit) + builtins[i]->exit(); + if (builtins[i]->init) + builtins[i]->init(); + } } void udev_builtin_list(void) { diff --git a/src/udev/udev-builtin.h b/src/udev/udev-builtin.h index 8c2016e5f0c..3b5f3bd120b 100644 --- a/src/udev/udev-builtin.h +++ b/src/udev/udev-builtin.h @@ -59,7 +59,8 @@ const char* udev_builtin_name(UdevBuiltinCommand cmd); bool udev_builtin_run_once(UdevBuiltinCommand cmd); int udev_builtin_run(UdevEvent *event, UdevBuiltinCommand cmd, const char *command); void udev_builtin_list(void); -bool udev_builtin_should_reload(void); +UdevReloadFlags udev_builtin_should_reload(void); +void udev_builtin_reload(UdevReloadFlags flags); int udev_builtin_add_property(UdevEvent *event, const char *key, const char *val); int udev_builtin_add_propertyf(UdevEvent *event, const char *key, const char *valf, ...) _printf_(3, 4); int udev_builtin_import_property(UdevEvent *event, const char *key); diff --git a/src/udev/udev-def.h b/src/udev/udev-def.h index 6ff3feacec1..9d9fc78247d 100644 --- a/src/udev/udev-def.h +++ b/src/udev/udev-def.h @@ -55,3 +55,26 @@ typedef enum UdevBuiltinCommand { _UDEV_BUILTIN_MAX, _UDEV_BUILTIN_INVALID = -EINVAL, } UdevBuiltinCommand; + +typedef enum UdevReloadFlags { +#if HAVE_BLKID + UDEV_RELOAD_BUILTIN_BLKID = 1u << UDEV_BUILTIN_BLKID, +#endif + UDEV_RELOAD_BUILTIN_BTRFS = 1u << UDEV_BUILTIN_BTRFS, + UDEV_RELOAD_BUILTIN_HWDB = 1u << UDEV_BUILTIN_HWDB, + UDEV_RELOAD_BUILTIN_INPUT_ID = 1u << UDEV_BUILTIN_INPUT_ID, + UDEV_RELOAD_BUILTIN_KEYBOARD = 1u << UDEV_BUILTIN_KEYBOARD, +#if HAVE_KMOD + UDEV_RELOAD_BUILTIN_KMOD = 1u << UDEV_BUILTIN_KMOD, +#endif + UDEV_RELOAD_BUILTIN_DRIVER = 1u << UDEV_BUILTIN_NET_DRIVER, + UDEV_RELOAD_BUILTIN_NET_ID = 1u << UDEV_BUILTIN_NET_ID, + UDEV_RELOAD_BUILTIN_NET_LINK = 1u << UDEV_BUILTIN_NET_LINK, + UDEV_RELOAD_BUILTIN_PATH_ID = 1u << UDEV_BUILTIN_PATH_ID, + UDEV_RELOAD_BUILTIN_USB_ID = 1u << UDEV_BUILTIN_USB_ID, +#if HAVE_ACL + UDEV_RELOAD_BUILTIN_UACCESS = 1u << UDEV_BUILTIN_UACCESS, +#endif + UDEV_RELOAD_KILL_WORKERS = 1u << (_UDEV_BUILTIN_MAX + 0), + UDEV_RELOAD_RULES = 1u << (_UDEV_BUILTIN_MAX + 1), +} UdevReloadFlags; diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index 7f7079bcd2c..c691b8ebed6 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -262,22 +262,25 @@ static void manager_reload(Manager *manager, bool force) { /* Reload SELinux label database, to make the child inherit the up-to-date database. */ mac_selinux_maybe_reload(); - /* Nothing changed. It is not necessary to reload. */ - if (!udev_rules_should_reload(manager->rules) && !udev_builtin_should_reload()) { - + UdevReloadFlags flags = udev_builtin_should_reload(); + if (udev_rules_should_reload(manager->rules)) + flags |= UDEV_RELOAD_RULES | UDEV_RELOAD_KILL_WORKERS; + if (flags == 0) { + /* Nothing changed. It is not necessary to reload. */ if (!force) return; /* If we eat this up, then tell our service manager to just continue */ (void) notify_reloading_full("Skipping configuration reloading, nothing changed."); - } else { + } else (void) notify_reloading(); + if (FLAGS_SET(flags, UDEV_RELOAD_KILL_WORKERS)) manager_kill_workers(manager, false); - udev_builtin_exit(); - udev_builtin_init(); + udev_builtin_reload(flags); + if (FLAGS_SET(flags, UDEV_RELOAD_RULES)) { r = udev_rules_load(&rules, manager->config.resolve_name_timing); if (r < 0) log_warning_errno(r, "Failed to read udev rules, using the previously loaded rules, ignoring: %m"); From e95861d909e17d56eddc20331a62f67f0d37d950 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 4 Dec 2024 04:29:13 +0900 Subject: [PATCH 2/3] udev: also reload udev.conf when explicitly requested When reloading is explicitly requested, e.g. by 'udevadm control --reload', then also reload udev.conf. --- src/udev/udev-config.c | 24 ++++++++++++++++++++++++ src/udev/udev-config.h | 1 + src/udev/udev-manager.c | 16 ++++++++-------- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/udev/udev-config.c b/src/udev/udev-config.c index eced0805470..891cf925358 100644 --- a/src/udev/udev-config.c +++ b/src/udev/udev-config.c @@ -339,3 +339,27 @@ int manager_load(Manager *manager, int argc, char *argv[]) { manager_adjust_config(&manager->config); return 1; } + +UdevReloadFlags manager_reload_config(Manager *manager) { + assert(manager); + + UdevConfig old = manager->config; + + manager->config_by_udev_conf = UDEV_CONFIG_INIT; + manager_parse_udev_config(&manager->config_by_udev_conf); + manager_merge_config(manager); + log_set_max_level(manager->config.log_level); + manager_adjust_config(&manager->config); + + if (manager->config.resolve_name_timing != old.resolve_name_timing) + return UDEV_RELOAD_RULES | UDEV_RELOAD_KILL_WORKERS; + + if (manager->config.log_level != old.log_level || + manager->config.exec_delay_usec != old.exec_delay_usec || + manager->config.timeout_usec != old.timeout_usec || + manager->config.timeout_signal != old.timeout_signal || + manager->config.blockdev_read_only != old.blockdev_read_only) + return UDEV_RELOAD_KILL_WORKERS; + + return 0; +} diff --git a/src/udev/udev-config.h b/src/udev/udev-config.h index 3b0997eeb07..1c7a74b1067 100644 --- a/src/udev/udev-config.h +++ b/src/udev/udev-config.h @@ -27,4 +27,5 @@ typedef struct UdevConfig { } int manager_load(Manager *manager, int argc, char *argv[]); +UdevReloadFlags manager_reload_config(Manager *manager); void udev_config_set_default_children_max(UdevConfig *c); diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index c691b8ebed6..4fc316e1069 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -265,15 +265,15 @@ static void manager_reload(Manager *manager, bool force) { UdevReloadFlags flags = udev_builtin_should_reload(); if (udev_rules_should_reload(manager->rules)) flags |= UDEV_RELOAD_RULES | UDEV_RELOAD_KILL_WORKERS; - if (flags == 0) { - /* Nothing changed. It is not necessary to reload. */ - if (!force) - return; + if (flags == 0 && !force) + /* Neither .rules files nor config files for builtins e.g. .link files changed. It is not + * necessary to reload configs. Note, udev.conf is not checked in the above, hence reloaded + * when explicitly requested or at least one .rules file or friend is updated. */ + return; - /* If we eat this up, then tell our service manager to just continue */ - (void) notify_reloading_full("Skipping configuration reloading, nothing changed."); - } else - (void) notify_reloading(); + (void) notify_reloading(); + + flags |= manager_reload_config(manager); if (FLAGS_SET(flags, UDEV_RELOAD_KILL_WORKERS)) manager_kill_workers(manager, false); From ced0ef3b35faadc5c8c07f6dd24f69bd836d8399 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 4 Dec 2024 06:34:43 +0900 Subject: [PATCH 3/3] TEST-17: use 'udevadm control --reload' or 'systemctl reload systemd-udevd.service' for reloading udev.conf These should be equivalent. For coverage, one subtest uses systemctl and another uses udevadm. --- test/units/TEST-17-UDEV.03.sh | 4 ++-- test/units/TEST-17-UDEV.device_is_processing.sh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/units/TEST-17-UDEV.03.sh b/test/units/TEST-17-UDEV.03.sh index d6b31622588..56ecf49e3b0 100755 --- a/test/units/TEST-17-UDEV.03.sh +++ b/test/units/TEST-17-UDEV.03.sh @@ -27,7 +27,7 @@ event_timeout=10 timeout_signal=SIGABRT EOF - systemctl restart systemd-udevd.service + systemctl reload systemd-udevd.service } # shellcheck disable=SC2317 @@ -40,7 +40,7 @@ teardown() { rm -rf "$TMPDIR" rm -f "$TEST_RULE" "$TEST_CONF" - systemctl restart systemd-udevd.service + systemctl reload systemd-udevd.service } run_test_timeout() { diff --git a/test/units/TEST-17-UDEV.device_is_processing.sh b/test/units/TEST-17-UDEV.device_is_processing.sh index 88e4b5a6bd3..d3b48e780e4 100755 --- a/test/units/TEST-17-UDEV.device_is_processing.sh +++ b/test/units/TEST-17-UDEV.device_is_processing.sh @@ -18,7 +18,7 @@ at_exit() { # Forcibly kills sleep command invoked by the udev rule before restarting, # otherwise systemctl restart below will takes longer. killall -KILL sleep - systemctl restart systemd-udevd.service + udevadm control --reload ip link del "$IFNAME" } @@ -36,7 +36,7 @@ cat >/run/udev/rules.d/99-testsuite.rules <