From 66f737b415e61f150c49480a74344877a63ac016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 13 Nov 2018 10:37:05 +0100 Subject: [PATCH] udev: do not pass timeout_warn_usec around, calculate it on demand It was always set to one third of timeout_usec, so let's simplify things by calculating it using a helper function right before it is used. Before 9d9264ba39f797d20100c8acfda3df895ab5aaa2, udevd.c would avoid setting timeout_warn_usec to 0, using 1 instead. This wasn't necessary, because when timeout_warn_usec is finally used in spawn_wait(), it is ignored if timeout_usec is 0 or timeout_warn_usec is 0. So there was no need to handle this case specially. --- src/test/test-udev.c | 4 ++-- src/udev/udev-event.c | 21 ++++++++------------- src/udev/udev-rules.c | 8 +++----- src/udev/udev.h | 11 +++++++---- src/udev/udevadm-test.c | 5 +---- src/udev/udevd.c | 16 +++------------- 6 files changed, 24 insertions(+), 41 deletions(-) diff --git a/src/test/test-udev.c b/src/test/test-udev.c index 7c70336e1b..b93876d26f 100644 --- a/src/test/test-udev.c +++ b/src/test/test-udev.c @@ -119,8 +119,8 @@ int main(int argc, char *argv[]) { } } - udev_event_execute_rules(event, 3 * USEC_PER_SEC, USEC_PER_SEC, NULL, rules); - udev_event_execute_run(event, 3 * USEC_PER_SEC, USEC_PER_SEC); + udev_event_execute_rules(event, 3 * USEC_PER_SEC, NULL, rules); + udev_event_execute_run(event, 3 * USEC_PER_SEC); out: mac_selinux_finish(); diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index d5666c1079..40f4dac05e 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -596,7 +596,6 @@ static int spawn_wait(Spawn *spawn) { int udev_event_spawn(struct udev_event *event, usec_t timeout_usec, - usec_t timeout_warn_usec, bool accept_failure, const char *cmd, char *result, size_t ressize) { @@ -667,7 +666,7 @@ int udev_event_spawn(struct udev_event *event, .cmd = cmd, .pid = pid, .accept_failure = accept_failure, - .timeout_warn_usec = timeout_warn_usec, + .timeout_warn_usec = udev_warn_timeout(timeout_usec), .timeout_usec = timeout_usec, .event_birth_usec = event->birth_usec, .fd_stdout = outpipe[READ_END], @@ -780,7 +779,7 @@ static int update_devnode(struct udev_event *event) { static void event_execute_rules_on_remove( struct udev_event *event, - usec_t timeout_usec, usec_t timeout_warn_usec, + usec_t timeout_usec, Hashmap *properties_list, struct udev_rules *rules) { @@ -802,16 +801,14 @@ static void event_execute_rules_on_remove( if (sd_device_get_devnum(dev, NULL) >= 0) (void) udev_watch_end(dev); - (void) udev_rules_apply_to_event(rules, event, - timeout_usec, timeout_warn_usec, - properties_list); + (void) udev_rules_apply_to_event(rules, event, timeout_usec, properties_list); if (sd_device_get_devnum(dev, NULL) >= 0) (void) udev_node_remove(dev); } int udev_event_execute_rules(struct udev_event *event, - usec_t timeout_usec, usec_t timeout_warn_usec, + usec_t timeout_usec, Hashmap *properties_list, struct udev_rules *rules) { sd_device *dev = event->dev; @@ -830,7 +827,7 @@ int udev_event_execute_rules(struct udev_event *event, return log_device_error_errno(dev, r, "Failed to get property 'ACTION': %m"); if (streq(action, "remove")) { - event_execute_rules_on_remove(event, timeout_usec, timeout_warn_usec, properties_list, rules); + event_execute_rules_on_remove(event, timeout_usec, properties_list, rules); return 0; } @@ -854,9 +851,7 @@ int udev_event_execute_rules(struct udev_event *event, (void) udev_watch_end(event->dev_db_clone); } - (void) udev_rules_apply_to_event(rules, event, - timeout_usec, timeout_warn_usec, - properties_list); + (void) udev_rules_apply_to_event(rules, event, timeout_usec, properties_list); (void) rename_netif(event); (void) update_devnode(event); @@ -882,7 +877,7 @@ int udev_event_execute_rules(struct udev_event *event, return 0; } -void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec, usec_t timeout_warn_usec) { +void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec) { const char *cmd; void *val; Iterator i; @@ -901,7 +896,7 @@ void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec, usec_ (void) usleep(event->exec_delay_usec); } - udev_event_spawn(event, timeout_usec, timeout_warn_usec, false, command, NULL, 0); + udev_event_spawn(event, timeout_usec, false, command, NULL, 0); } } } diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index c8b905a873..e96bc46d62 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -641,13 +641,12 @@ static int import_file_into_properties(sd_device *dev, const char *filename) { static int import_program_into_properties(struct udev_event *event, usec_t timeout_usec, - usec_t timeout_warn_usec, const char *program) { char result[UTIL_LINE_SIZE]; char *line; int err; - err = udev_event_spawn(event, timeout_usec, timeout_warn_usec, true, program, result, sizeof(result)); + err = udev_event_spawn(event, timeout_usec, true, program, result, sizeof(result)); if (err < 0) return err; @@ -1728,7 +1727,6 @@ int udev_rules_apply_to_event( struct udev_rules *rules, struct udev_event *event, usec_t timeout_usec, - usec_t timeout_warn_usec, Hashmap *properties_list) { sd_device *dev = event->dev; enum escape_type esc = ESCAPE_UNSET; @@ -1965,7 +1963,7 @@ int udev_rules_apply_to_event( rules_str(rules, rule->rule.filename_off), rule->rule.filename_line); - if (udev_event_spawn(event, timeout_usec, timeout_warn_usec, true, program, result, sizeof(result)) < 0) { + if (udev_event_spawn(event, timeout_usec, true, program, result, sizeof(result)) < 0) { if (cur->key.op != OP_NOMATCH) goto nomatch; } else { @@ -2001,7 +1999,7 @@ int udev_rules_apply_to_event( rules_str(rules, rule->rule.filename_off), rule->rule.filename_line); - if (import_program_into_properties(event, timeout_usec, timeout_warn_usec, import) != 0) + if (import_program_into_properties(event, timeout_usec, import) != 0) if (cur->key.op != OP_NOMATCH) goto nomatch; break; diff --git a/src/udev/udev.h b/src/udev/udev.h index 5c6ca6bf9f..f61d90e71a 100644 --- a/src/udev/udev.h +++ b/src/udev/udev.h @@ -62,13 +62,17 @@ struct udev_rules *udev_rules_new(ResolveNameTiming resolve_name_timing); struct udev_rules *udev_rules_unref(struct udev_rules *rules); bool udev_rules_check_timestamp(struct udev_rules *rules); int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event, - usec_t timeout_usec, usec_t timeout_warn_usec, + usec_t timeout_usec, Hashmap *properties_list); int udev_rules_apply_static_dev_perms(struct udev_rules *rules); ResolveNameTiming resolve_name_timing_from_string(const char *s) _pure_; const char *resolve_name_timing_to_string(ResolveNameTiming i) _const_; +static inline usec_t udev_warn_timeout(usec_t timeout_usec) { + return DIV_ROUND_UP(timeout_usec, 3); +} + /* udev-event.c */ struct udev_event *udev_event_new(sd_device *dev, usec_t exec_delay_usec, sd_netlink *rtnl); struct udev_event *udev_event_free(struct udev_event *event); @@ -77,14 +81,13 @@ ssize_t udev_event_apply_format(struct udev_event *event, bool replace_whitespace); int udev_event_spawn(struct udev_event *event, usec_t timeout_usec, - usec_t timeout_warn_usec, bool accept_failure, const char *cmd, char *result, size_t ressize); int udev_event_execute_rules(struct udev_event *event, - usec_t timeout_usec, usec_t timeout_warn_usec, + usec_t timeout_usec, Hashmap *properties_list, struct udev_rules *rules); -void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec, usec_t timeout_warn_usec); +void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec); /* Cleanup functions */ DEFINE_TRIVIAL_CLEANUP_FUNC(struct udev_event*, udev_event_free); diff --git a/src/udev/udevadm-test.c b/src/udev/udevadm-test.c index dd0fc4f91f..6f3b6099a9 100644 --- a/src/udev/udevadm-test.c +++ b/src/udev/udevadm-test.c @@ -131,10 +131,7 @@ int test_main(int argc, char *argv[], void *userdata) { sigfillset(&mask); sigprocmask(SIG_SETMASK, &mask, &sigmask_orig); - udev_event_execute_rules(event, - 60 * USEC_PER_SEC, 20 * USEC_PER_SEC, - NULL, - rules); + udev_event_execute_rules(event, 60 * USEC_PER_SEC, NULL, rules); FOREACH_DEVICE_PROPERTY(dev, key, value) printf("%s=%s\n", key, value); diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 05030dc81e..c77ca57d93 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -66,7 +66,6 @@ static ResolveNameTiming arg_resolve_name_timing = RESOLVE_NAME_EARLY; static unsigned arg_children_max = 0; static usec_t arg_exec_delay_usec = 0; static usec_t arg_event_timeout_usec = 180 * USEC_PER_SEC; -static usec_t arg_event_timeout_warn_usec = 180 * USEC_PER_SEC / 3; typedef struct Manager { sd_event *event; @@ -271,7 +270,7 @@ static void worker_attach_event(struct worker *worker, struct event *event) { assert_se(sd_event_now(e, CLOCK_MONOTONIC, &usec) >= 0); (void) sd_event_add_time(e, &event->timeout_warning, CLOCK_MONOTONIC, - usec + arg_event_timeout_warn_usec, USEC_PER_SEC, on_event_timeout_warning, event); + usec + udev_warn_timeout(arg_event_timeout_usec), USEC_PER_SEC, on_event_timeout_warning, event); (void) sd_event_add_time(e, &event->timeout, CLOCK_MONOTONIC, usec + arg_event_timeout_usec, USEC_PER_SEC, on_event_timeout, event); @@ -442,13 +441,8 @@ static void worker_spawn(Manager *manager, struct event *event) { } /* apply rules, create node, symlinks */ - udev_event_execute_rules(udev_event, - arg_event_timeout_usec, arg_event_timeout_warn_usec, - manager->properties, - manager->rules); - - udev_event_execute_run(udev_event, - arg_event_timeout_usec, arg_event_timeout_warn_usec); + udev_event_execute_rules(udev_event, arg_event_timeout_usec, manager->properties, manager->rules); + udev_event_execute_run(udev_event, arg_event_timeout_usec); if (!rtnl) /* in case rtnl was initialized */ @@ -1421,8 +1415,6 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat return 0; r = parse_sec(value, &arg_event_timeout_usec); - if (r >= 0) - arg_event_timeout_warn_usec = DIV_ROUND_UP(arg_event_timeout_usec, 3); } else if (proc_cmdline_key_streq(key, "udev.children_max")) { @@ -1514,8 +1506,6 @@ static int parse_argv(int argc, char *argv[]) { r = parse_sec(optarg, &arg_event_timeout_usec); if (r < 0) log_warning_errno(r, "Failed to parse --event-timeout= value '%s', ignoring: %m", optarg); - - arg_event_timeout_warn_usec = DIV_ROUND_UP(arg_event_timeout_usec, 3); break; case 'D': arg_debug = true;