Based on dd5eddd28a74a49607a8fffcaf960040dba98479 Mon Sep 17 00:00:00 2001 From: Kay Sievers Date: Tue, 29 Jul 2014 15:18:27 +0200 Subject: [PATCH] udev: unify event timeout handling --- man/systemd-udevd.service.xml | 7 +++--- man/udev.xml | 7 ------ src/test/test-udev.c | 5 ++-- src/udev/udev-event.c | 53 ++++++++++++++++++++++------------------- src/udev/udev-rules.c | 38 ++++++++--------------------- src/udev/udev.h | 8 +++---- src/udev/udevadm-test.c | 2 +- src/udev/udevd.c | 32 ++++++++++--------------- 8 files changed, 61 insertions(+), 91 deletions(-) diff --git man/systemd-udevd.service.xml man/systemd-udevd.service.xml index 8de43b1..3053dc7 100644 --- man/systemd-udevd.service.xml +++ man/systemd-udevd.service.xml @@ -99,9 +99,8 @@ - Wait for the event to finish up to the given - number of seconds. After this time the event will - be terminated. Default is 30. + Set the number of seconds to wait for events to finish. After + this time the event will be terminated. The default is 30 seconds. @@ -171,7 +170,7 @@ Wait for events to finish up to the given number of seconds. This option might be useful if events are - terminated due to a timeout in large configurations. + terminated due to kernel drivers taking too long to initialize. diff --git man/udev.xml man/udev.xml index 4e5f8f0..123c073 100644 --- man/udev.xml +++ man/udev.xml @@ -516,13 +516,6 @@ - - - Number of seconds an event waits for operations to finish before - giving up and terminating itself. - - - Usually control and other possibly unsafe characters are replaced diff --git src/test/test-udev.c src/test/test-udev.c index b057cc8..26d6537 100644 --- src/test/test-udev.c +++ src/test/test-udev.c @@ -80,7 +80,6 @@ out: return err; } - int main(int argc, char *argv[]) { _cleanup_udev_unref_ struct udev *udev = NULL; _cleanup_udev_event_unref_ struct udev_event *event = NULL; @@ -155,8 +154,8 @@ int main(int argc, char *argv[]) { } } - udev_event_execute_rules(event, rules, &sigmask_orig); - udev_event_execute_run(event, NULL); + udev_event_execute_rules(event, USEC_PER_SEC, rules, &sigmask_orig); + udev_event_execute_run(event, USEC_PER_SEC, NULL); out: if (event != NULL && event->fd_signal >= 0) close(event->fd_signal); diff --git src/udev/udev-event.c src/udev/udev-event.c index 5213a4a..6ad80d5 100644 --- src/udev/udev-event.c +++ src/udev/udev-event.c @@ -48,7 +48,6 @@ struct udev_event *udev_event_new(struct udev_device *dev) udev_list_init(udev, &event->seclabel_list, false); event->fd_signal = -1; event->birth_usec = now(CLOCK_MONOTONIC); - event->timeout_usec = 30 * 1000 * 1000; return event; } @@ -422,9 +421,10 @@ static int spawn_exec(struct udev_event *event, } static void spawn_read(struct udev_event *event, - const char *cmd, - int fd_stdout, int fd_stderr, - char *result, size_t ressize) + usec_t timeout_usec, + const char *cmd, + int fd_stdout, int fd_stderr, + char *result, size_t ressize) { size_t respos = 0; int fd_ep = -1; @@ -467,15 +467,15 @@ static void spawn_read(struct udev_event *event, struct epoll_event ev[4]; int i; - if (event->timeout_usec > 0) { + if (timeout_usec > 0) { usec_t age_usec; age_usec = now(CLOCK_MONOTONIC) - event->birth_usec; - if (age_usec >= event->timeout_usec) { + if (age_usec >= timeout_usec) { log_error("timeout '%s'", cmd); goto out; } - timeout = ((event->timeout_usec - age_usec) / 1000) + 1000; + timeout = ((timeout_usec - age_usec) / USEC_PER_MSEC) + MSEC_PER_SEC; } else { timeout = -1; } @@ -543,8 +543,9 @@ out: close(fd_ep); } -static int spawn_wait(struct udev_event *event, const char *cmd, pid_t pid) -{ +static int spawn_wait(struct udev_event *event, + usec_t timeout_usec, + const char *cmd, pid_t pid) { struct pollfd pfd[1]; int err = 0; @@ -555,14 +556,14 @@ static int spawn_wait(struct udev_event *event, const char *cmd, pid_t pid) int timeout; int fdcount; - if (event->timeout_usec > 0) { + if (timeout_usec > 0) { usec_t age_usec; age_usec = now(CLOCK_MONOTONIC) - event->birth_usec; - if (age_usec >= event->timeout_usec) + if (age_usec >= timeout_usec) timeout = 1000; else - timeout = ((event->timeout_usec - age_usec) / 1000) + 1000; + timeout = ((timeout_usec - age_usec) / USEC_PER_MSEC) + MSEC_PER_SEC; } else { timeout = -1; } @@ -657,9 +658,9 @@ out: } int udev_event_spawn(struct udev_event *event, + usec_t timeout_usec, const char *cmd, char **envp, const sigset_t *sigmask, - char *result, size_t ressize) -{ + char *result, size_t ressize) { struct udev *udev = event->udev; int outpipe[2] = {-1, -1}; int errpipe[2] = {-1, -1}; @@ -728,11 +729,13 @@ int udev_event_spawn(struct udev_event *event, errpipe[WRITE_END] = -1; } - spawn_read(event, cmd, - outpipe[READ_END], errpipe[READ_END], - result, ressize); + spawn_read(event, + timeout_usec, + cmd, + outpipe[READ_END], errpipe[READ_END], + result, ressize); - err = spawn_wait(event, cmd, pid); + err = spawn_wait(event, timeout_usec, cmd, pid); } out: @@ -776,8 +779,9 @@ static int rename_netif(struct udev_event *event) return r; } -void udev_event_execute_rules(struct udev_event *event, struct udev_rules *rules, const sigset_t *sigmask) -{ +void udev_event_execute_rules(struct udev_event *event, + usec_t timeout_usec, + struct udev_rules *rules, const sigset_t *sigmask) { struct udev_device *dev = event->dev; if (udev_device_get_subsystem(dev) == NULL) @@ -791,7 +795,7 @@ void udev_event_execute_rules(struct udev_event *event, struct udev_rules *rules if (major(udev_device_get_devnum(dev)) != 0) udev_watch_end(event->udev, dev); - udev_rules_apply_to_event(rules, event, sigmask); + udev_rules_apply_to_event(rules, event, timeout_usec, sigmask); if (major(udev_device_get_devnum(dev)) != 0) udev_node_remove(dev); @@ -808,7 +812,7 @@ void udev_event_execute_rules(struct udev_event *event, struct udev_rules *rules udev_watch_end(event->udev, event->dev_db); } - udev_rules_apply_to_event(rules, event, sigmask); + udev_rules_apply_to_event(rules, event, timeout_usec, sigmask); /* rename a new network interface, if needed */ if (udev_device_get_ifindex(dev) > 0 && streq(udev_device_get_action(dev), "add") && @@ -883,8 +887,7 @@ void udev_event_execute_rules(struct udev_event *event, struct udev_rules *rules } } -void udev_event_execute_run(struct udev_event *event, const sigset_t *sigmask) -{ +void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec, const sigset_t *sigmask) { struct udev_list_entry *list_entry; udev_list_entry_foreach(list_entry, udev_list_get_entry(&event->run_list)) { @@ -907,7 +910,7 @@ void udev_event_execute_run(struct udev_event *event, const sigset_t *sigmask) udev_event_apply_format(event, cmd, program, sizeof(program)); envp = udev_device_get_properties_envp(event->dev); - udev_event_spawn(event, program, envp, sigmask, NULL, 0); + udev_event_spawn(event, timeout_usec, program, envp, sigmask, NULL, 0); } } } diff --git src/udev/udev-rules.c src/udev/udev-rules.c index 9864016..aacde38 100644 --- src/udev/udev-rules.c +++ src/udev/udev-rules.c @@ -137,7 +137,6 @@ enum token_type { TK_M_PARENTS_MAX, TK_M_TEST, /* val, mode_t */ - TK_M_EVENT_TIMEOUT, /* int */ TK_M_PROGRAM, /* val */ TK_M_IMPORT_FILE, /* val */ TK_M_IMPORT_PROG, /* val */ @@ -201,7 +200,6 @@ struct token { uid_t uid; gid_t gid; int devlink_prio; - int event_timeout; int watch; enum udev_builtin_cmd builtin_cmd; }; @@ -275,7 +273,6 @@ static const char *token_str(enum token_type type) [TK_M_PARENTS_MAX] = "M PARENTS_MAX", [TK_M_TEST] = "M TEST", - [TK_M_EVENT_TIMEOUT] = "M EVENT_TIMEOUT", [TK_M_PROGRAM] = "M PROGRAM", [TK_M_IMPORT_FILE] = "M IMPORT_FILE", [TK_M_IMPORT_PROG] = "M IMPORT_PROG", @@ -409,9 +406,6 @@ static void dump_token(struct udev_rules *rules, struct token *token) case TK_A_SECLABEL: log_debug("%s %s '%s' '%s'", token_str(type), operation_str(op), attr, value); break; - case TK_M_EVENT_TIMEOUT: - log_debug("%s %u", token_str(type), token->key.event_timeout); - break; case TK_A_GOTO: log_debug("%s '%s' %u", token_str(type), value, token->key.rule_goto); break; @@ -627,8 +621,9 @@ static int import_file_into_properties(struct udev_device *dev, const char *file return 0; } -static int import_program_into_properties(struct udev_event *event, const char *program, const sigset_t *sigmask) -{ +static int import_program_into_properties(struct udev_event *event, + usec_t timeout_usec, + const char *program, const sigset_t *sigmask) { struct udev_device *dev = event->dev; char **envp; char result[UTIL_LINE_SIZE]; @@ -636,7 +631,7 @@ static int import_program_into_properties(struct udev_event *event, const char * int err; envp = udev_device_get_properties_envp(dev); - err = udev_event_spawn(event, program, envp, sigmask, result, sizeof(result)); + err = udev_event_spawn(event, timeout_usec, program, envp, sigmask, result, sizeof(result)); if (err < 0) return err; @@ -942,9 +937,6 @@ static int rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, case TK_A_MODE_ID: token->key.mode = *(mode_t *)data; break; - case TK_M_EVENT_TIMEOUT: - token->key.event_timeout = *(int *)data; - break; case TK_RULE: case TK_M_PARENTS_MIN: case TK_M_PARENTS_MAX: @@ -1462,14 +1454,6 @@ static int add_rule(struct udev_rules *rules, char *line, rule_add_key(&rule_tmp, TK_A_DEVLINK_PRIO, op, NULL, &prio); } - pos = strstr(value, "event_timeout="); - if (pos != NULL) { - int tout = atoi(&pos[strlen("event_timeout=")]); - - rule_add_key(&rule_tmp, TK_M_EVENT_TIMEOUT, op, NULL, &tout); - } - - pos = strstr(value, "string_escape="); if (pos != NULL) { pos = &pos[strlen("string_escape=")]; if (startswith(pos, "none")) @@ -1829,8 +1813,10 @@ enum escape_type { ESCAPE_REPLACE, }; -int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event, const sigset_t *sigmask) -{ +int udev_rules_apply_to_event(struct udev_rules *rules, + struct udev_event *event, + usec_t timeout_usec, + const sigset_t *sigmask) { struct token *cur; struct token *rule; enum escape_type esc = ESCAPE_UNSET; @@ -2024,10 +2010,6 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event goto nomatch; break; } - case TK_M_EVENT_TIMEOUT: - log_debug("OPTIONS event_timeout=%u", cur->key.event_timeout); - event->timeout_usec = cur->key.event_timeout * 1000 * 1000; - break; case TK_M_PROGRAM: { char program[UTIL_PATH_SIZE]; char **envp; @@ -2042,7 +2024,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event rules_str(rules, rule->rule.filename_off), rule->rule.filename_line); - if (udev_event_spawn(event, program, envp, sigmask, result, sizeof(result)) < 0) { + if (udev_event_spawn(event, timeout_usec, program, envp, sigmask, result, sizeof(result)) < 0) { if (cur->key.op != OP_NOMATCH) goto nomatch; } else { @@ -2078,7 +2060,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event rules_str(rules, rule->rule.filename_off), rule->rule.filename_line); - if (import_program_into_properties(event, import, sigmask) != 0) + if (import_program_into_properties(event, timeout_usec, import, sigmask) != 0) if (cur->key.op != OP_NOMATCH) goto nomatch; break; diff --git src/udev/udev.h src/udev/udev.h index 62538bc..4aca70b 100644 --- src/udev/udev.h +++ src/udev/udev.h @@ -43,7 +43,6 @@ struct udev_event { struct udev_list run_list; int exec_delay; usec_t birth_usec; - usec_t timeout_usec; int fd_signal; unsigned int builtin_run; unsigned int builtin_ret; @@ -72,7 +71,7 @@ struct udev_rules; struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names); 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, const sigset_t *sigmask); +int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event, usec_t timeout_usec, const sigset_t *sigmask); int udev_rules_apply_static_dev_perms(struct udev_rules *rules); /* udev-event.c */ @@ -82,10 +81,11 @@ size_t udev_event_apply_format(struct udev_event *event, const char *src, char * int udev_event_apply_subsys_kernel(struct udev_event *event, const char *string, char *result, size_t maxsize, int read_value); int udev_event_spawn(struct udev_event *event, + usec_t timeout_usec, const char *cmd, char **envp, const sigset_t *sigmask, char *result, size_t ressize); -void udev_event_execute_rules(struct udev_event *event, struct udev_rules *rules, const sigset_t *sigset); -void udev_event_execute_run(struct udev_event *event, const sigset_t *sigset); +void udev_event_execute_rules(struct udev_event *event, usec_t timeout_usec, struct udev_rules *rules, const sigset_t *sigset); +void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec, const sigset_t *sigset); int udev_build_argv(struct udev *udev, char *cmd, int *argc, char *argv[]); /* udev-watch.c */ diff --git src/udev/udevadm-test.c src/udev/udevadm-test.c index 6a2f548..52cc26c 100644 --- src/udev/udevadm-test.c +++ src/udev/udevadm-test.c @@ -138,7 +138,7 @@ static int adm_test(struct udev *udev, int argc, char *argv[]) goto out; } - udev_event_execute_rules(event, rules, &sigmask_orig); + udev_event_execute_rules(event, 30 * USEC_PER_SEC, rules, &sigmask_orig); udev_list_entry_foreach(entry, udev_device_get_properties_list_entry(dev)) printf("%s=%s\n", udev_list_entry_get_name(entry), udev_list_entry_get_value(entry)); diff --git src/udev/udevd.c src/udev/udevd.c index c5dd739..dee7a87 100644 --- src/udev/udevd.c +++ src/udev/udevd.c @@ -74,7 +74,7 @@ static bool reload; static int children; static int children_max; static int exec_delay; -static int event_timeout = 30; +static usec_t event_timeout_usec = 30 * USEC_PER_SEC; static sigset_t sigmask_orig; static UDEV_LIST(event_list); static UDEV_LIST(worker_list); @@ -313,13 +313,10 @@ static void worker_new(struct event *event) } } - if (event_timeout != 30) - udev_event->timeout_usec = event_timeout * USEC_PER_SEC; - /* apply rules, create node, symlinks */ - udev_event_execute_rules(udev_event, rules, &sigmask_orig); + udev_event_execute_rules(udev_event, event_timeout_usec, rules, &sigmask_orig); - udev_event_execute_run(udev_event, &sigmask_orig); + udev_event_execute_run(udev_event, event_timeout_usec, &sigmask_orig); /* apply/restore inotify watch */ if (udev_event->inotify_watch) { @@ -1014,15 +1011,14 @@ static void kernel_cmdline_options(struct udev *udev) } else if (startswith(opt, "udev.exec-delay=")) { exec_delay = strtoul(opt + 16, NULL, 0); } else if (startswith(opt, "udev.event-timeout=")) { - event_timeout = strtoul(opt + 16, NULL, 0); + event_timeout_usec = strtoul(opt + 16, NULL, 0) * USEC_PER_SEC; } free(s); } } -int main(int argc, char *argv[]) -{ +int main(int argc, char *argv[]) { struct udev *udev; sigset_t mask; int daemonize = false; @@ -1077,7 +1073,7 @@ int main(int argc, char *argv[]) exec_delay = strtoul(optarg, NULL, 0); break; case 't': - event_timeout = strtoul(optarg, NULL, 0); + event_timeout_usec = strtoul(optarg, NULL, 0) * USEC_PER_SEC; break; case 'D': debug = true; @@ -1103,6 +1099,7 @@ int main(int argc, char *argv[]) " --debug\n" " --children-max=\n" " --exec-delay=\n" + " --event-timeout=\n" " --resolve-names=early|late|never\n" " --version\n" " --help\n" @@ -1416,20 +1413,17 @@ int main(int argc, char *argv[]) if (worker->state != WORKER_RUNNING) continue; - if ((now(CLOCK_MONOTONIC) - worker->event_start_usec) > event_timeout * USEC_PER_SEC) { - log_error("worker [%u] %s timeout; kill it", worker->pid, - worker->event ? worker->event->devpath : ""); + if ((now(CLOCK_MONOTONIC) - worker->event_start_usec) > event_timeout_usec) { + log_error("worker [%u] %s timeout; kill it", worker->pid, worker->event->devpath); kill(worker->pid, SIGKILL); worker->state = WORKER_KILLED; /* drop reference taken for state 'running' */ worker_unref(worker); - if (worker->event) { - log_error("seq %llu '%s' killed", udev_device_get_seqnum(worker->event->dev), worker->event->devpath); - worker->event->exitcode = -64; - event_queue_delete(worker->event, true); - worker->event = NULL; - } + log_error("seq %llu '%s' killed", udev_device_get_seqnum(worker->event->dev), worker->event->devpath); + worker->event->exitcode = -64; + event_queue_delete(worker->event, true); + worker->event = NULL; } } -- 1.7.9.2