From d9239923c1de3f10f1598567e8bebcb798c4bd27 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Jun 2021 19:05:39 +0900 Subject: [PATCH 01/13] udev: rename type name e.g. struct worker -> Worker --- src/udev/udevd.c | 97 +++++++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 47 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 5a4657de14a..6baedd2f2e6 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -77,10 +77,13 @@ static usec_t arg_event_timeout_usec = 180 * USEC_PER_SEC; static int arg_timeout_signal = SIGKILL; static bool arg_blockdev_read_only = false; +typedef struct Event Event; +typedef struct Worker Worker; + typedef struct Manager { sd_event *event; Hashmap *workers; - LIST_HEAD(struct event, events); + LIST_HEAD(Event, events); const char *cgroup; pid_t pid; /* the process that originally allocated the manager object */ int log_level; @@ -106,16 +109,16 @@ typedef struct Manager { bool exit; } Manager; -enum event_state { +typedef enum EventState { EVENT_UNDEF, EVENT_QUEUED, EVENT_RUNNING, -}; +} EventState; -struct event { +typedef struct Event { Manager *manager; - struct worker *worker; - enum event_state state; + Worker *worker; + EventState state; sd_device *dev; sd_device *dev_kernel; /* clone of originally received device */ @@ -126,32 +129,32 @@ struct event { sd_event_source *timeout_warning_event; sd_event_source *timeout_event; - LIST_FIELDS(struct event, event); -}; + LIST_FIELDS(Event, event); +} Event; -static void event_queue_cleanup(Manager *manager, enum event_state type); +static void event_queue_cleanup(Manager *manager, EventState match_state); -enum worker_state { +typedef enum WorkerState { WORKER_UNDEF, WORKER_RUNNING, WORKER_IDLE, WORKER_KILLED, WORKER_KILLING, -}; +} WorkerState; -struct worker { +typedef struct Worker { Manager *manager; pid_t pid; sd_device_monitor *monitor; - enum worker_state state; - struct event *event; -}; + WorkerState state; + Event *event; +} Worker; /* passed from worker to main process */ -struct worker_message { -}; +typedef struct WorkerMessage { +} WorkerMessage; -static void event_free(struct event *event) { +static void event_free(Event *event) { if (!event) return; @@ -176,7 +179,7 @@ static void event_free(struct event *event) { free(event); } -static struct worker* worker_free(struct worker *worker) { +static Worker *worker_free(Worker *worker) { if (!worker) return NULL; @@ -189,11 +192,11 @@ static struct worker* worker_free(struct worker *worker) { return mfree(worker); } -DEFINE_TRIVIAL_CLEANUP_FUNC(struct worker *, worker_free); -DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(worker_hash_op, void, trivial_hash_func, trivial_compare_func, struct worker, worker_free); +DEFINE_TRIVIAL_CLEANUP_FUNC(Worker*, worker_free); +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(worker_hash_op, void, trivial_hash_func, trivial_compare_func, Worker, worker_free); -static int worker_new(struct worker **ret, Manager *manager, sd_device_monitor *worker_monitor, pid_t pid) { - _cleanup_(worker_freep) struct worker *worker = NULL; +static int worker_new(Worker **ret, Manager *manager, sd_device_monitor *worker_monitor, pid_t pid) { + _cleanup_(worker_freep) Worker *worker = NULL; int r; assert(ret); @@ -204,11 +207,11 @@ static int worker_new(struct worker **ret, Manager *manager, sd_device_monitor * /* close monitor, but keep address around */ device_monitor_disconnect(worker_monitor); - worker = new(struct worker, 1); + worker = new(Worker, 1); if (!worker) return -ENOMEM; - *worker = (struct worker) { + *worker = (Worker) { .manager = manager, .monitor = sd_device_monitor_ref(worker_monitor), .pid = pid, @@ -224,7 +227,7 @@ static int worker_new(struct worker **ret, Manager *manager, sd_device_monitor * } static int on_event_timeout(sd_event_source *s, uint64_t usec, void *userdata) { - struct event *event = userdata; + Event *event = userdata; assert(event); assert(event->worker); @@ -238,7 +241,7 @@ static int on_event_timeout(sd_event_source *s, uint64_t usec, void *userdata) { } static int on_event_timeout_warning(sd_event_source *s, uint64_t usec, void *userdata) { - struct event *event = userdata; + Event *event = userdata; assert(event); assert(event->worker); @@ -248,7 +251,7 @@ static int on_event_timeout_warning(sd_event_source *s, uint64_t usec, void *use return 1; } -static void worker_attach_event(struct worker *worker, struct event *event) { +static void worker_attach_event(Worker *worker, Event *event) { sd_event *e; assert(worker); @@ -315,7 +318,7 @@ static Manager* manager_free(Manager *manager) { DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); static int worker_send_message(int fd) { - struct worker_message message = {}; + WorkerMessage message = {}; return loop_write(fd, &message, sizeof(message), false); } @@ -591,9 +594,9 @@ static int worker_main(Manager *_manager, sd_device_monitor *monitor, sd_device return 0; } -static int worker_spawn(Manager *manager, struct event *event) { +static int worker_spawn(Manager *manager, Event *event) { _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *worker_monitor = NULL; - struct worker *worker; + Worker *worker; pid_t pid; int r; @@ -635,9 +638,9 @@ static int worker_spawn(Manager *manager, struct event *event) { return 0; } -static void event_run(Manager *manager, struct event *event) { +static void event_run(Manager *manager, Event *event) { static bool log_children_max_reached = true; - struct worker *worker; + Worker *worker; int r; assert(manager); @@ -685,7 +688,7 @@ static void event_run(Manager *manager, struct event *event) { static int event_queue_insert(Manager *manager, sd_device *dev) { _cleanup_(sd_device_unrefp) sd_device *clone = NULL; - struct event *event; + Event *event; uint64_t seqnum; int r; @@ -709,11 +712,11 @@ static int event_queue_insert(Manager *manager, sd_device *dev) { if (r < 0) return r; - event = new(struct event, 1); + event = new(Event, 1); if (!event) return -ENOMEM; - *event = (struct event) { + *event = (Event) { .manager = manager, .dev = sd_device_ref(dev), .dev_kernel = TAKE_PTR(clone), @@ -735,7 +738,7 @@ static int event_queue_insert(Manager *manager, sd_device *dev) { } static void manager_kill_workers(Manager *manager, bool force) { - struct worker *worker; + Worker *worker; assert(manager); @@ -754,10 +757,10 @@ static void manager_kill_workers(Manager *manager, bool force) { } /* lookup event for identical, parent, child device */ -static int is_device_busy(Manager *manager, struct event *event) { +static int is_device_busy(Manager *manager, Event *event) { const char *subsystem, *devpath, *devpath_old = NULL; dev_t devnum = makedev(0, 0); - struct event *loop_event; + Event *loop_event; size_t devpath_len; int r, ifindex = 0; bool is_block; @@ -916,7 +919,7 @@ static int on_kill_workers_event(sd_event_source *s, uint64_t usec, void *userda } static void event_queue_start(Manager *manager) { - struct event *event; + Event *event; usec_t usec; int r; @@ -963,11 +966,11 @@ static void event_queue_start(Manager *manager) { } } -static void event_queue_cleanup(Manager *manager, enum event_state match_type) { - struct event *event, *tmp; +static void event_queue_cleanup(Manager *manager, EventState match_state) { + Event *event, *tmp; LIST_FOREACH_SAFE(event, event, tmp, manager->events) { - if (match_type != EVENT_UNDEF && match_type != event->state) + if (match_state != EVENT_UNDEF && match_state != event->state) continue; event_free(event); @@ -980,7 +983,7 @@ static int on_worker(sd_event_source *s, int fd, uint32_t revents, void *userdat assert(manager); for (;;) { - struct worker_message msg; + WorkerMessage msg; struct iovec iovec = { .iov_base = &msg, .iov_len = sizeof(msg), @@ -994,7 +997,7 @@ static int on_worker(sd_event_source *s, int fd, uint32_t revents, void *userdat }; ssize_t size; struct ucred *ucred; - struct worker *worker; + Worker *worker; size = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT); if (size == -EINTR) @@ -1007,7 +1010,7 @@ static int on_worker(sd_event_source *s, int fd, uint32_t revents, void *userdat cmsg_close_all(&msghdr); - if (size != sizeof(struct worker_message)) { + if (size != sizeof(WorkerMessage)) { log_warning("Ignoring worker message with invalid size %zi bytes", size); continue; } @@ -1357,7 +1360,7 @@ static int on_sigchld(sd_event_source *s, const struct signalfd_siginfo *si, voi for (;;) { pid_t pid; int status; - struct worker *worker; + Worker *worker; pid = waitpid(-1, &status, WNOHANG); if (pid <= 0) From e0d61dac3324abc90f61014a98b1bc5a9a1f60ae Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Jun 2021 19:18:56 +0900 Subject: [PATCH 02/13] udev: also rename struct udev_ctrl -> UdevCtrl --- src/udev/udev-ctrl.c | 52 ++++++++++++++++++------------------ src/udev/udev-ctrl.h | 54 +++++++++++++++++++------------------- src/udev/udevadm-control.c | 2 +- src/udev/udevadm-settle.c | 2 +- src/udev/udevadm-trigger.c | 2 +- src/udev/udevd.c | 4 +-- 6 files changed, 58 insertions(+), 58 deletions(-) diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c index 3d563547190..00279ba3d87 100644 --- a/src/udev/udev-ctrl.c +++ b/src/udev/udev-ctrl.c @@ -23,14 +23,14 @@ /* wire protocol magic must match */ #define UDEV_CTRL_MAGIC 0xdead1dea -struct udev_ctrl_msg_wire { +typedef struct UdevCtrlMessageWire { char version[16]; unsigned magic; - enum udev_ctrl_msg_type type; - union udev_ctrl_msg_value value; -}; + UdevCtrlMessageType type; + UdevCtrlMessageValue value; +} UdevCtrlMessageWire; -struct udev_ctrl { +struct UdevCtrl { unsigned n_ref; int sock; int sock_connect; @@ -47,9 +47,9 @@ struct udev_ctrl { void *userdata; }; -int udev_ctrl_new_from_fd(struct udev_ctrl **ret, int fd) { +int udev_ctrl_new_from_fd(UdevCtrl **ret, int fd) { _cleanup_close_ int sock = -1; - struct udev_ctrl *uctrl; + UdevCtrl *uctrl; assert(ret); @@ -59,11 +59,11 @@ int udev_ctrl_new_from_fd(struct udev_ctrl **ret, int fd) { return log_error_errno(errno, "Failed to create socket: %m"); } - uctrl = new(struct udev_ctrl, 1); + uctrl = new(UdevCtrl, 1); if (!uctrl) return -ENOMEM; - *uctrl = (struct udev_ctrl) { + *uctrl = (UdevCtrl) { .n_ref = 1, .sock = fd >= 0 ? fd : TAKE_FD(sock), .sock_connect = -1, @@ -81,7 +81,7 @@ int udev_ctrl_new_from_fd(struct udev_ctrl **ret, int fd) { return 0; } -int udev_ctrl_enable_receiving(struct udev_ctrl *uctrl) { +int udev_ctrl_enable_receiving(UdevCtrl *uctrl) { int r; assert(uctrl); @@ -107,7 +107,7 @@ int udev_ctrl_enable_receiving(struct udev_ctrl *uctrl) { return 0; } -static void udev_ctrl_disconnect(struct udev_ctrl *uctrl) { +static void udev_ctrl_disconnect(UdevCtrl *uctrl) { if (!uctrl) return; @@ -115,7 +115,7 @@ static void udev_ctrl_disconnect(struct udev_ctrl *uctrl) { uctrl->sock_connect = safe_close(uctrl->sock_connect); } -static struct udev_ctrl *udev_ctrl_free(struct udev_ctrl *uctrl) { +static UdevCtrl *udev_ctrl_free(UdevCtrl *uctrl) { assert(uctrl); udev_ctrl_disconnect(uctrl); @@ -127,9 +127,9 @@ static struct udev_ctrl *udev_ctrl_free(struct udev_ctrl *uctrl) { return mfree(uctrl); } -DEFINE_TRIVIAL_REF_UNREF_FUNC(struct udev_ctrl, udev_ctrl, udev_ctrl_free); +DEFINE_TRIVIAL_REF_UNREF_FUNC(UdevCtrl, udev_ctrl, udev_ctrl_free); -int udev_ctrl_cleanup(struct udev_ctrl *uctrl) { +int udev_ctrl_cleanup(UdevCtrl *uctrl) { if (!uctrl) return 0; if (uctrl->cleanup_socket) @@ -137,7 +137,7 @@ int udev_ctrl_cleanup(struct udev_ctrl *uctrl) { return 0; } -int udev_ctrl_attach_event(struct udev_ctrl *uctrl, sd_event *event) { +int udev_ctrl_attach_event(UdevCtrl *uctrl, sd_event *event) { int r; assert_return(uctrl, -EINVAL); @@ -154,25 +154,25 @@ int udev_ctrl_attach_event(struct udev_ctrl *uctrl, sd_event *event) { return 0; } -sd_event_source *udev_ctrl_get_event_source(struct udev_ctrl *uctrl) { +sd_event_source *udev_ctrl_get_event_source(UdevCtrl *uctrl) { assert(uctrl); return uctrl->event_source; } -static void udev_ctrl_disconnect_and_listen_again(struct udev_ctrl *uctrl) { +static void udev_ctrl_disconnect_and_listen_again(UdevCtrl *uctrl) { udev_ctrl_disconnect(uctrl); udev_ctrl_unref(uctrl); (void) sd_event_source_set_enabled(uctrl->event_source, SD_EVENT_ON); /* We don't return NULL here because uctrl is not freed */ } -DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct udev_ctrl*, udev_ctrl_disconnect_and_listen_again, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(UdevCtrl*, udev_ctrl_disconnect_and_listen_again, NULL); static int udev_ctrl_connection_event_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) { - _cleanup_(udev_ctrl_disconnect_and_listen_againp) struct udev_ctrl *uctrl = NULL; - struct udev_ctrl_msg_wire msg_wire; - struct iovec iov = IOVEC_MAKE(&msg_wire, sizeof(struct udev_ctrl_msg_wire)); + _cleanup_(udev_ctrl_disconnect_and_listen_againp) UdevCtrl *uctrl = NULL; + UdevCtrlMessageWire msg_wire; + struct iovec iov = IOVEC_MAKE(&msg_wire, sizeof(UdevCtrlMessageWire)); CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; struct msghdr smsg = { .msg_iov = &iov, @@ -235,7 +235,7 @@ static int udev_ctrl_connection_event_handler(sd_event_source *s, int fd, uint32 } static int udev_ctrl_event_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) { - struct udev_ctrl *uctrl = userdata; + UdevCtrl *uctrl = userdata; _cleanup_close_ int sock = -1; struct ucred ucred; int r; @@ -282,7 +282,7 @@ static int udev_ctrl_event_handler(sd_event_source *s, int fd, uint32_t revents, return 0; } -int udev_ctrl_start(struct udev_ctrl *uctrl, udev_ctrl_handler_t callback, void *userdata) { +int udev_ctrl_start(UdevCtrl *uctrl, udev_ctrl_handler_t callback, void *userdata) { int r; assert(uctrl); @@ -309,8 +309,8 @@ int udev_ctrl_start(struct udev_ctrl *uctrl, udev_ctrl_handler_t callback, void return 0; } -int udev_ctrl_send(struct udev_ctrl *uctrl, enum udev_ctrl_msg_type type, int intval, const char *buf) { - struct udev_ctrl_msg_wire ctrl_msg_wire = { +int udev_ctrl_send(UdevCtrl *uctrl, UdevCtrlMessageType type, int intval, const char *buf) { + UdevCtrlMessageWire ctrl_msg_wire = { .version = "udev-" STRINGIFY(PROJECT_VERSION), .magic = UDEV_CTRL_MAGIC, .type = type, @@ -339,7 +339,7 @@ int udev_ctrl_send(struct udev_ctrl *uctrl, enum udev_ctrl_msg_type type, int in return 0; } -int udev_ctrl_wait(struct udev_ctrl *uctrl, usec_t timeout) { +int udev_ctrl_wait(UdevCtrl *uctrl, usec_t timeout) { _cleanup_(sd_event_source_unrefp) sd_event_source *source_io = NULL, *source_timeout = NULL; int r; diff --git a/src/udev/udev-ctrl.h b/src/udev/udev-ctrl.h index 680fbf7bff1..ca80c2aa4e0 100644 --- a/src/udev/udev-ctrl.h +++ b/src/udev/udev-ctrl.h @@ -6,9 +6,9 @@ #include "macro.h" #include "time-util.h" -struct udev_ctrl; +typedef struct UdevCtrl UdevCtrl; -enum udev_ctrl_msg_type { +typedef enum UdevCtrlMessageType { _UDEV_CTRL_END_MESSAGES, UDEV_CTRL_SET_LOG_LEVEL, UDEV_CTRL_STOP_EXEC_QUEUE, @@ -18,62 +18,62 @@ enum udev_ctrl_msg_type { UDEV_CTRL_SET_CHILDREN_MAX, UDEV_CTRL_PING, UDEV_CTRL_EXIT, -}; +} UdevCtrlMessageType; -union udev_ctrl_msg_value { +typedef union UdevCtrlMessageValue { int intval; char buf[256]; -}; +} UdevCtrlMessageValue; -typedef int (*udev_ctrl_handler_t)(struct udev_ctrl *udev_ctrl, enum udev_ctrl_msg_type type, - const union udev_ctrl_msg_value *value, void *userdata); +typedef int (*udev_ctrl_handler_t)(UdevCtrl *udev_ctrl, UdevCtrlMessageType type, + const UdevCtrlMessageValue *value, void *userdata); -int udev_ctrl_new_from_fd(struct udev_ctrl **ret, int fd); -static inline int udev_ctrl_new(struct udev_ctrl **ret) { +int udev_ctrl_new_from_fd(UdevCtrl **ret, int fd); +static inline int udev_ctrl_new(UdevCtrl **ret) { return udev_ctrl_new_from_fd(ret, -1); } -int udev_ctrl_enable_receiving(struct udev_ctrl *uctrl); -struct udev_ctrl *udev_ctrl_ref(struct udev_ctrl *uctrl); -struct udev_ctrl *udev_ctrl_unref(struct udev_ctrl *uctrl); -int udev_ctrl_cleanup(struct udev_ctrl *uctrl); -int udev_ctrl_attach_event(struct udev_ctrl *uctrl, sd_event *event); -int udev_ctrl_start(struct udev_ctrl *uctrl, udev_ctrl_handler_t callback, void *userdata); -sd_event_source *udev_ctrl_get_event_source(struct udev_ctrl *uctrl); +int udev_ctrl_enable_receiving(UdevCtrl *uctrl); +UdevCtrl *udev_ctrl_ref(UdevCtrl *uctrl); +UdevCtrl *udev_ctrl_unref(UdevCtrl *uctrl); +int udev_ctrl_cleanup(UdevCtrl *uctrl); +int udev_ctrl_attach_event(UdevCtrl *uctrl, sd_event *event); +int udev_ctrl_start(UdevCtrl *uctrl, udev_ctrl_handler_t callback, void *userdata); +sd_event_source *udev_ctrl_get_event_source(UdevCtrl *uctrl); -int udev_ctrl_wait(struct udev_ctrl *uctrl, usec_t timeout); +int udev_ctrl_wait(UdevCtrl *uctrl, usec_t timeout); -int udev_ctrl_send(struct udev_ctrl *uctrl, enum udev_ctrl_msg_type type, int intval, const char *buf); -static inline int udev_ctrl_send_set_log_level(struct udev_ctrl *uctrl, int priority) { +int udev_ctrl_send(UdevCtrl *uctrl, UdevCtrlMessageType type, int intval, const char *buf); +static inline int udev_ctrl_send_set_log_level(UdevCtrl *uctrl, int priority) { return udev_ctrl_send(uctrl, UDEV_CTRL_SET_LOG_LEVEL, priority, NULL); } -static inline int udev_ctrl_send_stop_exec_queue(struct udev_ctrl *uctrl) { +static inline int udev_ctrl_send_stop_exec_queue(UdevCtrl *uctrl) { return udev_ctrl_send(uctrl, UDEV_CTRL_STOP_EXEC_QUEUE, 0, NULL); } -static inline int udev_ctrl_send_start_exec_queue(struct udev_ctrl *uctrl) { +static inline int udev_ctrl_send_start_exec_queue(UdevCtrl *uctrl) { return udev_ctrl_send(uctrl, UDEV_CTRL_START_EXEC_QUEUE, 0, NULL); } -static inline int udev_ctrl_send_reload(struct udev_ctrl *uctrl) { +static inline int udev_ctrl_send_reload(UdevCtrl *uctrl) { return udev_ctrl_send(uctrl, UDEV_CTRL_RELOAD, 0, NULL); } -static inline int udev_ctrl_send_set_env(struct udev_ctrl *uctrl, const char *key) { +static inline int udev_ctrl_send_set_env(UdevCtrl *uctrl, const char *key) { return udev_ctrl_send(uctrl, UDEV_CTRL_SET_ENV, 0, key); } -static inline int udev_ctrl_send_set_children_max(struct udev_ctrl *uctrl, int count) { +static inline int udev_ctrl_send_set_children_max(UdevCtrl *uctrl, int count) { return udev_ctrl_send(uctrl, UDEV_CTRL_SET_CHILDREN_MAX, count, NULL); } -static inline int udev_ctrl_send_ping(struct udev_ctrl *uctrl) { +static inline int udev_ctrl_send_ping(UdevCtrl *uctrl) { return udev_ctrl_send(uctrl, UDEV_CTRL_PING, 0, NULL); } -static inline int udev_ctrl_send_exit(struct udev_ctrl *uctrl) { +static inline int udev_ctrl_send_exit(UdevCtrl *uctrl) { return udev_ctrl_send(uctrl, UDEV_CTRL_EXIT, 0, NULL); } -DEFINE_TRIVIAL_CLEANUP_FUNC(struct udev_ctrl*, udev_ctrl_unref); +DEFINE_TRIVIAL_CLEANUP_FUNC(UdevCtrl*, udev_ctrl_unref); diff --git a/src/udev/udevadm-control.c b/src/udev/udevadm-control.c index 20820dd6472..06c61e5c07c 100644 --- a/src/udev/udevadm-control.c +++ b/src/udev/udevadm-control.c @@ -48,7 +48,7 @@ static int help(void) { } int control_main(int argc, char *argv[], void *userdata) { - _cleanup_(udev_ctrl_unrefp) struct udev_ctrl *uctrl = NULL; + _cleanup_(udev_ctrl_unrefp) UdevCtrl *uctrl = NULL; usec_t timeout = 60 * USEC_PER_SEC; int c, r; diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c index 84b4f9ca458..6da9439bd28 100644 --- a/src/udev/udevadm-settle.c +++ b/src/udev/udevadm-settle.c @@ -176,7 +176,7 @@ int settle_main(int argc, char *argv[], void *userdata) { /* guarantee that the udev daemon isn't pre-processing */ if (getuid() == 0) { - _cleanup_(udev_ctrl_unrefp) struct udev_ctrl *uctrl = NULL; + _cleanup_(udev_ctrl_unrefp) UdevCtrl *uctrl = NULL; if (udev_ctrl_new(&uctrl) >= 0) { r = udev_ctrl_send_ping(uctrl); diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index 8acf3d9b118..a24073fb734 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -421,7 +421,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { } if (ping) { - _cleanup_(udev_ctrl_unrefp) struct udev_ctrl *uctrl = NULL; + _cleanup_(udev_ctrl_unrefp) UdevCtrl *uctrl = NULL; r = udev_ctrl_new(&uctrl); if (r < 0) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 6baedd2f2e6..a35b095dd14 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -94,7 +94,7 @@ typedef struct Manager { sd_netlink *rtnl; sd_device_monitor *monitor; - struct udev_ctrl *ctrl; + UdevCtrl *ctrl; int worker_watch[2]; /* used by udev-watch */ @@ -1067,7 +1067,7 @@ static int on_uevent(sd_device_monitor *monitor, sd_device *dev, void *userdata) } /* receive the udevd message from userspace */ -static int on_ctrl_msg(struct udev_ctrl *uctrl, enum udev_ctrl_msg_type type, const union udev_ctrl_msg_value *value, void *userdata) { +static int on_ctrl_msg(UdevCtrl *uctrl, UdevCtrlMessageType type, const UdevCtrlMessageValue *value, void *userdata) { Manager *manager = userdata; int r; From 419ec631358c8bf7013db01ae42763e6971d8765 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Jun 2021 15:14:59 +0900 Subject: [PATCH 03/13] udev: move several functions No functional chage. --- src/udev/udevd.c | 462 +++++++++++++++++++++++------------------------ 1 file changed, 230 insertions(+), 232 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index a35b095dd14..546bfe039e1 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -132,8 +132,6 @@ typedef struct Event { LIST_FIELDS(Event, event); } Event; -static void event_queue_cleanup(Manager *manager, EventState match_state); - typedef enum WorkerState { WORKER_UNDEF, WORKER_RUNNING, @@ -179,6 +177,17 @@ static void event_free(Event *event) { free(event); } +static void event_queue_cleanup(Manager *manager, EventState match_state) { + Event *event, *tmp; + + LIST_FOREACH_SAFE(event, event, tmp, manager->events) { + if (match_state != EVENT_UNDEF && match_state != event->state) + continue; + + event_free(event); + } +} + static Worker *worker_free(Worker *worker) { if (!worker) return NULL; @@ -195,87 +204,6 @@ static Worker *worker_free(Worker *worker) { DEFINE_TRIVIAL_CLEANUP_FUNC(Worker*, worker_free); DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(worker_hash_op, void, trivial_hash_func, trivial_compare_func, Worker, worker_free); -static int worker_new(Worker **ret, Manager *manager, sd_device_monitor *worker_monitor, pid_t pid) { - _cleanup_(worker_freep) Worker *worker = NULL; - int r; - - assert(ret); - assert(manager); - assert(worker_monitor); - assert(pid > 1); - - /* close monitor, but keep address around */ - device_monitor_disconnect(worker_monitor); - - worker = new(Worker, 1); - if (!worker) - return -ENOMEM; - - *worker = (Worker) { - .manager = manager, - .monitor = sd_device_monitor_ref(worker_monitor), - .pid = pid, - }; - - r = hashmap_ensure_put(&manager->workers, &worker_hash_op, PID_TO_PTR(pid), worker); - if (r < 0) - return r; - - *ret = TAKE_PTR(worker); - - return 0; -} - -static int on_event_timeout(sd_event_source *s, uint64_t usec, void *userdata) { - Event *event = userdata; - - assert(event); - assert(event->worker); - - kill_and_sigcont(event->worker->pid, arg_timeout_signal); - event->worker->state = WORKER_KILLED; - - log_device_error(event->dev, "Worker ["PID_FMT"] processing SEQNUM=%"PRIu64" killed", event->worker->pid, event->seqnum); - - return 1; -} - -static int on_event_timeout_warning(sd_event_source *s, uint64_t usec, void *userdata) { - Event *event = userdata; - - assert(event); - assert(event->worker); - - log_device_warning(event->dev, "Worker ["PID_FMT"] processing SEQNUM=%"PRIu64" is taking a long time", event->worker->pid, event->seqnum); - - return 1; -} - -static void worker_attach_event(Worker *worker, Event *event) { - sd_event *e; - - assert(worker); - assert(worker->manager); - assert(event); - assert(!event->worker); - assert(!worker->event); - - worker->state = WORKER_RUNNING; - worker->event = event; - event->state = EVENT_RUNNING; - event->worker = worker; - - e = worker->manager->event; - - (void) sd_event_add_time_relative(e, &event->timeout_warning_event, CLOCK_MONOTONIC, - udev_warn_timeout(arg_event_timeout_usec), USEC_PER_SEC, - on_event_timeout_warning, event); - - (void) sd_event_add_time_relative(e, &event->timeout_event, CLOCK_MONOTONIC, - arg_event_timeout_usec, USEC_PER_SEC, - on_event_timeout, event); -} - static void manager_clear_for_worker(Manager *manager) { assert(manager); @@ -317,6 +245,107 @@ static Manager* manager_free(Manager *manager) { DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); +static int worker_new(Worker **ret, Manager *manager, sd_device_monitor *worker_monitor, pid_t pid) { + _cleanup_(worker_freep) Worker *worker = NULL; + int r; + + assert(ret); + assert(manager); + assert(worker_monitor); + assert(pid > 1); + + /* close monitor, but keep address around */ + device_monitor_disconnect(worker_monitor); + + worker = new(Worker, 1); + if (!worker) + return -ENOMEM; + + *worker = (Worker) { + .manager = manager, + .monitor = sd_device_monitor_ref(worker_monitor), + .pid = pid, + }; + + r = hashmap_ensure_put(&manager->workers, &worker_hash_op, PID_TO_PTR(pid), worker); + if (r < 0) + return r; + + *ret = TAKE_PTR(worker); + + return 0; +} + +static void manager_kill_workers(Manager *manager, bool force) { + Worker *worker; + + assert(manager); + + HASHMAP_FOREACH(worker, manager->workers) { + if (worker->state == WORKER_KILLED) + continue; + + if (worker->state == WORKER_RUNNING && !force) { + worker->state = WORKER_KILLING; + continue; + } + + worker->state = WORKER_KILLED; + (void) kill(worker->pid, SIGTERM); + } +} + +static void manager_exit(Manager *manager) { + assert(manager); + + manager->exit = true; + + sd_notify(false, + "STOPPING=1\n" + "STATUS=Starting shutdown..."); + + /* close sources of new events and discard buffered events */ + manager->ctrl = udev_ctrl_unref(manager->ctrl); + + manager->inotify_event = sd_event_source_unref(manager->inotify_event); + manager->inotify_fd = safe_close(manager->inotify_fd); + + manager->monitor = sd_device_monitor_unref(manager->monitor); + + /* discard queued events and kill workers */ + event_queue_cleanup(manager, EVENT_QUEUED); + manager_kill_workers(manager, true); +} + +/* reload requested, HUP signal received, rules changed, builtin changed */ +static void manager_reload(Manager *manager) { + + assert(manager); + + sd_notify(false, + "RELOADING=1\n" + "STATUS=Flushing configuration..."); + + manager_kill_workers(manager, false); + manager->rules = udev_rules_free(manager->rules); + udev_builtin_exit(); + + sd_notifyf(false, + "READY=1\n" + "STATUS=Processing with %u children at max", arg_children_max); +} + +static int on_kill_workers_event(sd_event_source *s, uint64_t usec, void *userdata) { + Manager *manager = userdata; + + assert(manager); + + log_debug("Cleanup idle workers"); + manager_kill_workers(manager, false); + + return 1; +} + static int worker_send_message(int fd) { WorkerMessage message = {}; @@ -594,6 +623,56 @@ static int worker_main(Manager *_manager, sd_device_monitor *monitor, sd_device return 0; } +static int on_event_timeout(sd_event_source *s, uint64_t usec, void *userdata) { + Event *event = userdata; + + assert(event); + assert(event->worker); + + kill_and_sigcont(event->worker->pid, arg_timeout_signal); + event->worker->state = WORKER_KILLED; + + log_device_error(event->dev, "Worker ["PID_FMT"] processing SEQNUM=%"PRIu64" killed", event->worker->pid, event->seqnum); + + return 1; +} + +static int on_event_timeout_warning(sd_event_source *s, uint64_t usec, void *userdata) { + Event *event = userdata; + + assert(event); + assert(event->worker); + + log_device_warning(event->dev, "Worker ["PID_FMT"] processing SEQNUM=%"PRIu64" is taking a long time", event->worker->pid, event->seqnum); + + return 1; +} + +static void worker_attach_event(Worker *worker, Event *event) { + sd_event *e; + + assert(worker); + assert(worker->manager); + assert(event); + assert(!event->worker); + assert(!worker->event); + + worker->state = WORKER_RUNNING; + worker->event = event; + event->state = EVENT_RUNNING; + event->worker = worker; + + e = worker->manager->event; + + (void) sd_event_add_time_relative(e, &event->timeout_warning_event, CLOCK_MONOTONIC, + udev_warn_timeout(arg_event_timeout_usec), USEC_PER_SEC, + on_event_timeout_warning, event); + + (void) sd_event_add_time_relative(e, &event->timeout_event, CLOCK_MONOTONIC, + arg_event_timeout_usec, USEC_PER_SEC, + on_event_timeout, event); +} + static int worker_spawn(Manager *manager, Event *event) { _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *worker_monitor = NULL; Worker *worker; @@ -686,76 +765,6 @@ static void event_run(Manager *manager, Event *event) { worker_spawn(manager, event); } -static int event_queue_insert(Manager *manager, sd_device *dev) { - _cleanup_(sd_device_unrefp) sd_device *clone = NULL; - Event *event; - uint64_t seqnum; - int r; - - assert(manager); - assert(dev); - - /* only one process can add events to the queue */ - assert(manager->pid == getpid_cached()); - - /* We only accepts devices received by device monitor. */ - r = sd_device_get_seqnum(dev, &seqnum); - if (r < 0) - return r; - - /* Save original device to restore the state on failures. */ - r = device_shallow_clone(dev, &clone); - if (r < 0) - return r; - - r = device_copy_properties(clone, dev); - if (r < 0) - return r; - - event = new(Event, 1); - if (!event) - return -ENOMEM; - - *event = (Event) { - .manager = manager, - .dev = sd_device_ref(dev), - .dev_kernel = TAKE_PTR(clone), - .seqnum = seqnum, - .state = EVENT_QUEUED, - }; - - if (LIST_IS_EMPTY(manager->events)) { - r = touch("/run/udev/queue"); - if (r < 0) - log_warning_errno(r, "Failed to touch /run/udev/queue: %m"); - } - - LIST_APPEND(event, manager->events, event); - - log_device_uevent(dev, "Device is queued"); - - return 0; -} - -static void manager_kill_workers(Manager *manager, bool force) { - Worker *worker; - - assert(manager); - - HASHMAP_FOREACH(worker, manager->workers) { - if (worker->state == WORKER_KILLED) - continue; - - if (worker->state == WORKER_RUNNING && !force) { - worker->state = WORKER_KILLING; - continue; - } - - worker->state = WORKER_KILLED; - (void) kill(worker->pid, SIGTERM); - } -} - /* lookup event for identical, parent, child device */ static int is_device_busy(Manager *manager, Event *event) { const char *subsystem, *devpath, *devpath_old = NULL; @@ -867,57 +876,6 @@ set_delaying_seqnum: return true; } -static void manager_exit(Manager *manager) { - assert(manager); - - manager->exit = true; - - sd_notify(false, - "STOPPING=1\n" - "STATUS=Starting shutdown..."); - - /* close sources of new events and discard buffered events */ - manager->ctrl = udev_ctrl_unref(manager->ctrl); - - manager->inotify_event = sd_event_source_unref(manager->inotify_event); - manager->inotify_fd = safe_close(manager->inotify_fd); - - manager->monitor = sd_device_monitor_unref(manager->monitor); - - /* discard queued events and kill workers */ - event_queue_cleanup(manager, EVENT_QUEUED); - manager_kill_workers(manager, true); -} - -/* reload requested, HUP signal received, rules changed, builtin changed */ -static void manager_reload(Manager *manager) { - - assert(manager); - - sd_notify(false, - "RELOADING=1\n" - "STATUS=Flushing configuration..."); - - manager_kill_workers(manager, false); - manager->rules = udev_rules_free(manager->rules); - udev_builtin_exit(); - - sd_notifyf(false, - "READY=1\n" - "STATUS=Processing with %u children at max", arg_children_max); -} - -static int on_kill_workers_event(sd_event_source *s, uint64_t usec, void *userdata) { - Manager *manager = userdata; - - assert(manager); - - log_debug("Cleanup idle workers"); - manager_kill_workers(manager, false); - - return 1; -} - static void event_queue_start(Manager *manager) { Event *event; usec_t usec; @@ -966,15 +924,77 @@ static void event_queue_start(Manager *manager) { } } -static void event_queue_cleanup(Manager *manager, EventState match_state) { - Event *event, *tmp; +static int event_queue_insert(Manager *manager, sd_device *dev) { + _cleanup_(sd_device_unrefp) sd_device *clone = NULL; + Event *event; + uint64_t seqnum; + int r; - LIST_FOREACH_SAFE(event, event, tmp, manager->events) { - if (match_state != EVENT_UNDEF && match_state != event->state) - continue; + assert(manager); + assert(dev); - event_free(event); + /* only one process can add events to the queue */ + assert(manager->pid == getpid_cached()); + + /* We only accepts devices received by device monitor. */ + r = sd_device_get_seqnum(dev, &seqnum); + if (r < 0) + return r; + + /* Save original device to restore the state on failures. */ + r = device_shallow_clone(dev, &clone); + if (r < 0) + return r; + + r = device_copy_properties(clone, dev); + if (r < 0) + return r; + + event = new(Event, 1); + if (!event) + return -ENOMEM; + + *event = (Event) { + .manager = manager, + .dev = sd_device_ref(dev), + .dev_kernel = TAKE_PTR(clone), + .seqnum = seqnum, + .state = EVENT_QUEUED, + }; + + if (LIST_IS_EMPTY(manager->events)) { + r = touch("/run/udev/queue"); + if (r < 0) + log_warning_errno(r, "Failed to touch /run/udev/queue: %m"); } + + LIST_APPEND(event, manager->events, event); + + log_device_uevent(dev, "Device is queued"); + + return 0; +} + +static int on_uevent(sd_device_monitor *monitor, sd_device *dev, void *userdata) { + Manager *manager = userdata; + int r; + + assert(manager); + + DEVICE_TRACE_POINT(kernel_uevent_received, dev); + + device_ensure_usec_initialized(dev, NULL); + + r = event_queue_insert(manager, dev); + if (r < 0) { + log_device_error_errno(dev, r, "Failed to insert device into event queue: %m"); + return 1; + } + + /* we have fresh events, try to schedule them */ + event_queue_start(manager); + + return 1; } static int on_worker(sd_event_source *s, int fd, uint32_t revents, void *userdata) { @@ -1044,28 +1064,6 @@ static int on_worker(sd_event_source *s, int fd, uint32_t revents, void *userdat return 1; } -static int on_uevent(sd_device_monitor *monitor, sd_device *dev, void *userdata) { - Manager *manager = userdata; - int r; - - assert(manager); - - DEVICE_TRACE_POINT(kernel_uevent_received, dev); - - device_ensure_usec_initialized(dev, NULL); - - r = event_queue_insert(manager, dev); - if (r < 0) { - log_device_error_errno(dev, r, "Failed to insert device into event queue: %m"); - return 1; - } - - /* we have fresh events, try to schedule them */ - event_queue_start(manager); - - return 1; -} - /* receive the udevd message from userspace */ static int on_ctrl_msg(UdevCtrl *uctrl, UdevCtrlMessageType type, const UdevCtrlMessageValue *value, void *userdata) { Manager *manager = userdata; From 6be97d67c82ef5f45360c4323616739816b8f833 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Jun 2021 21:02:01 +0900 Subject: [PATCH 04/13] udev: update log message to clarify that the error is ignored --- src/udev/udevd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 546bfe039e1..34a5c9d5d8e 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -171,8 +171,8 @@ static void event_free(Event *event) { /* only clean up the queue from the process that created it */ if (LIST_IS_EMPTY(event->manager->events) && event->manager->pid == getpid_cached()) - if (unlink("/run/udev/queue") < 0) - log_warning_errno(errno, "Failed to unlink /run/udev/queue: %m"); + if (unlink("/run/udev/queue") < 0 && errno != ENOENT) + log_warning_errno(errno, "Failed to unlink /run/udev/queue, ignoring: %m"); free(event); } @@ -965,7 +965,7 @@ static int event_queue_insert(Manager *manager, sd_device *dev) { if (LIST_IS_EMPTY(manager->events)) { r = touch("/run/udev/queue"); if (r < 0) - log_warning_errno(r, "Failed to touch /run/udev/queue: %m"); + log_warning_errno(r, "Failed to touch /run/udev/queue, ignoring: %m"); } LIST_APPEND(event, manager->events, event); From 5393c52897ff5b57686c867fcab77f9740f4af24 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Jun 2021 15:21:27 +0900 Subject: [PATCH 05/13] udev: make event_free() return NULL --- src/udev/udevd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 34a5c9d5d8e..bb7c0eabe42 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -152,9 +152,9 @@ typedef struct Worker { typedef struct WorkerMessage { } WorkerMessage; -static void event_free(Event *event) { +static Event *event_free(Event *event) { if (!event) - return; + return NULL; assert(event->manager); @@ -174,7 +174,7 @@ static void event_free(Event *event) { if (unlink("/run/udev/queue") < 0 && errno != ENOENT) log_warning_errno(errno, "Failed to unlink /run/udev/queue, ignoring: %m"); - free(event); + return mfree(event); } static void event_queue_cleanup(Manager *manager, EventState match_state) { From 0744e74c526814e28f2fbcea128f40ed36341fcd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Jun 2021 15:29:02 +0900 Subject: [PATCH 06/13] udev: make event_queue_start() return negative errno on error --- src/udev/udevd.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index bb7c0eabe42..800ca294bd0 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -876,7 +876,7 @@ set_delaying_seqnum: return true; } -static void event_queue_start(Manager *manager) { +static int event_queue_start(Manager *manager) { Event *event; usec_t usec; int r; @@ -885,7 +885,7 @@ static void event_queue_start(Manager *manager) { if (LIST_IS_EMPTY(manager->events) || manager->exit || manager->stop_exec_queue) - return; + return 0; assert_se(sd_event_now(manager->event, CLOCK_MONOTONIC, &usec) >= 0); /* check for changed config, every 3 seconds at most */ @@ -906,10 +906,8 @@ static void event_queue_start(Manager *manager) { if (!manager->rules) { r = udev_rules_load(&manager->rules, arg_resolve_name_timing); - if (r < 0) { - log_warning_errno(r, "Failed to read udev rules: %m"); - return; - } + if (r < 0) + return log_warning_errno(r, "Failed to read udev rules: %m"); } LIST_FOREACH(event, event, manager->events) { @@ -922,6 +920,8 @@ static void event_queue_start(Manager *manager) { event_run(manager, event); } + + return 0; } static int event_queue_insert(Manager *manager, sd_device *dev) { From 92fd70addf25d4f301ba43ca3e6ede96d9564295 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Jun 2021 15:41:20 +0900 Subject: [PATCH 07/13] udev: add usec_add() at one more place --- src/udev/udevd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 800ca294bd0..8f0e669d2a0 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -890,7 +890,7 @@ static int event_queue_start(Manager *manager) { assert_se(sd_event_now(manager->event, CLOCK_MONOTONIC, &usec) >= 0); /* check for changed config, every 3 seconds at most */ if (manager->last_usec == 0 || - usec - manager->last_usec > 3 * USEC_PER_SEC) { + usec > usec_add(manager->last_usec, 3 * USEC_PER_SEC)) { if (udev_rules_check_timestamp(manager->rules) || udev_builtin_validate()) manager_reload(manager); From f2a5412bf286cabc047dc96395c2dae978e722b4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Jun 2021 15:47:34 +0900 Subject: [PATCH 08/13] udev: propagate error on spawning a worker --- src/udev/udevd.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 8f0e669d2a0..be975020c99 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -717,16 +717,18 @@ static int worker_spawn(Manager *manager, Event *event) { return 0; } -static void event_run(Manager *manager, Event *event) { +static int event_run(Event *event) { static bool log_children_max_reached = true; + Manager *manager; Worker *worker; int r; - assert(manager); assert(event); + assert(event->manager); log_device_uevent(event->dev, "Device ready for processing"); + manager = event->manager; HASHMAP_FOREACH(worker, manager->workers) { if (worker->state != WORKER_IDLE) continue; @@ -740,29 +742,32 @@ static void event_run(Manager *manager, Event *event) { continue; } worker_attach_event(worker, event); - return; + return 1; /* event is now processing. */ } if (hashmap_size(manager->workers) >= arg_children_max) { - /* Avoid spamming the debug logs if the limit is already reached and * many events still need to be processed */ if (log_children_max_reached && arg_children_max > 1) { log_debug("Maximum number (%u) of children reached.", hashmap_size(manager->workers)); log_children_max_reached = false; } - return; + return 0; /* no free worker */ } /* Re-enable the debug message for the next batch of events */ log_children_max_reached = true; /* fork with up-to-date SELinux label database, so the child inherits the up-to-date db - and, until the next SELinux policy changes, we safe further reloads in future children */ + * and, until the next SELinux policy changes, we safe further reloads in future children */ mac_selinux_maybe_reload(); /* start new worker and pass initial device */ - worker_spawn(manager, event); + r = worker_spawn(manager, event); + if (r < 0) + return r; + + return 1; /* event is now processing. */ } /* lookup event for identical, parent, child device */ @@ -918,7 +923,9 @@ static int event_queue_start(Manager *manager) { if (is_device_busy(manager, event) != 0) continue; - event_run(manager, event); + r = event_run(event); + if (r < 0) + return r; } return 0; From 5f4bca9dccdd9e9a888587c6224b08ae5fbe3bdb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Jun 2021 15:51:34 +0900 Subject: [PATCH 09/13] udev: do not try to process events if there is no free worker --- src/udev/udevd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index be975020c99..c61da46c370 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -924,7 +924,7 @@ static int event_queue_start(Manager *manager) { continue; r = event_run(event); - if (r < 0) + if (r <= 0) return r; } From a1fa99d84124cdcd4a306113ebe4febc1251c41c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Jun 2021 16:14:01 +0900 Subject: [PATCH 10/13] udev: rename is_device_busy() -> event_is_blocked() Also this rename delaying_seqnum -> blocker_seqnum. --- src/udev/udevd.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index c61da46c370..f5bdd372316 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -124,7 +124,7 @@ typedef struct Event { sd_device *dev_kernel; /* clone of originally received device */ uint64_t seqnum; - uint64_t delaying_seqnum; + uint64_t blocker_seqnum; sd_event_source *timeout_warning_event; sd_event_source *timeout_event; @@ -770,8 +770,7 @@ static int event_run(Event *event) { return 1; /* event is now processing. */ } -/* lookup event for identical, parent, child device */ -static int is_device_busy(Manager *manager, Event *event) { +static int event_is_blocked(Event *event) { const char *subsystem, *devpath, *devpath_old = NULL; dev_t devnum = makedev(0, 0); Event *loop_event; @@ -779,6 +778,8 @@ static int is_device_busy(Manager *manager, Event *event) { int r, ifindex = 0; bool is_block; + /* lookup event for identical, parent, child device */ + r = sd_device_get_subsystem(event->dev, &subsystem); if (r < 0) return r; @@ -804,21 +805,21 @@ static int is_device_busy(Manager *manager, Event *event) { return r; /* check if queue contains events we depend on */ - LIST_FOREACH(event, loop_event, manager->events) { + LIST_FOREACH(event, loop_event, event->manager->events) { size_t loop_devpath_len, common; const char *loop_devpath; /* we already found a later event, earlier cannot block us, no need to check again */ - if (loop_event->seqnum < event->delaying_seqnum) + if (loop_event->seqnum < event->blocker_seqnum) continue; /* event we checked earlier still exists, no need to check again */ - if (loop_event->seqnum == event->delaying_seqnum) + if (loop_event->seqnum == event->blocker_seqnum) return true; /* found ourself, no later event can block us */ if (loop_event->seqnum >= event->seqnum) - break; + return false; /* check major/minor */ if (major(devnum) != 0) { @@ -830,7 +831,7 @@ static int is_device_busy(Manager *manager, Event *event) { if (sd_device_get_devnum(loop_event->dev, &d) >= 0 && devnum == d && is_block == streq(s, "block")) - goto set_delaying_seqnum; + break; } /* check network device ifindex */ @@ -839,7 +840,7 @@ static int is_device_busy(Manager *manager, Event *event) { if (sd_device_get_ifindex(loop_event->dev, &i) >= 0 && ifindex == i) - goto set_delaying_seqnum; + break; } if (sd_device_get_devpath(loop_event->dev, &loop_devpath) < 0) @@ -847,7 +848,7 @@ static int is_device_busy(Manager *manager, Event *event) { /* check our old name */ if (devpath_old && streq(devpath_old, loop_devpath)) - goto set_delaying_seqnum; + break; loop_devpath_len = strlen(loop_devpath); @@ -860,24 +861,23 @@ static int is_device_busy(Manager *manager, Event *event) { /* identical device event found */ if (devpath_len == loop_devpath_len) - goto set_delaying_seqnum; + break; /* parent device event found */ if (devpath[common] == '/') - goto set_delaying_seqnum; + break; /* child device event found */ if (loop_devpath[common] == '/') - goto set_delaying_seqnum; + break; } - return false; + assert(loop_event); -set_delaying_seqnum: log_device_debug(event->dev, "SEQNUM=%" PRIu64 " blocked by SEQNUM=%" PRIu64, event->seqnum, loop_event->seqnum); - event->delaying_seqnum = loop_event->seqnum; + event->blocker_seqnum = loop_event->seqnum; return true; } @@ -920,7 +920,7 @@ static int event_queue_start(Manager *manager) { continue; /* do not start event if parent or child event is still running */ - if (is_device_busy(manager, event) != 0) + if (event_is_blocked(event) != 0) continue; r = event_run(event); From bd335c961fed6982e5ad8c2322414ff33a46e92e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Jun 2021 16:12:06 +0900 Subject: [PATCH 11/13] list: introduce LIST_FOREACH_BACKWARDS() macro and drop LIST_FOREACH_AFTER/BEFORE() --- src/basic/list.h | 7 ++----- src/core/device.c | 8 ++++---- src/core/swap.c | 4 ++-- src/udev/udev-rules.c | 2 +- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/basic/list.h b/src/basic/list.h index 256b7187c2e..e488fff9f02 100644 --- a/src/basic/list.h +++ b/src/basic/list.h @@ -142,11 +142,8 @@ #define LIST_FOREACH_SAFE(name,i,n,head) \ for ((i) = (head); (i) && (((n) = (i)->name##_next), 1); (i) = (n)) -#define LIST_FOREACH_BEFORE(name,i,p) \ - for ((i) = (p)->name##_prev; (i); (i) = (i)->name##_prev) - -#define LIST_FOREACH_AFTER(name,i,p) \ - for ((i) = (p)->name##_next; (i); (i) = (i)->name##_next) +#define LIST_FOREACH_BACKWARDS(name,i,p) \ + for ((i) = (p); (i); (i) = (i)->name##_prev) /* Iterate through all the members of the list p is included in, but skip over p */ #define LIST_FOREACH_OTHERS(name,i,p) \ diff --git a/src/core/device.c b/src/core/device.c index 5ed5ceb2904..66841984fea 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -771,11 +771,11 @@ static Unit *device_following(Unit *u) { return NULL; /* Make everybody follow the unit that's named after the sysfs path */ - LIST_FOREACH_AFTER(same_sysfs, other, d) + LIST_FOREACH(same_sysfs, other, d->same_sysfs_next) if (startswith(UNIT(other)->id, "sys-")) return UNIT(other); - LIST_FOREACH_BEFORE(same_sysfs, other, d) { + LIST_FOREACH_BACKWARDS(same_sysfs, other, d->same_sysfs_prev) { if (startswith(UNIT(other)->id, "sys-")) return UNIT(other); @@ -802,13 +802,13 @@ static int device_following_set(Unit *u, Set **_set) { if (!set) return -ENOMEM; - LIST_FOREACH_AFTER(same_sysfs, other, d) { + LIST_FOREACH(same_sysfs, other, d->same_sysfs_next) { r = set_put(set, other); if (r < 0) return r; } - LIST_FOREACH_BEFORE(same_sysfs, other, d) { + LIST_FOREACH_BACKWARDS(same_sysfs, other, d->same_sysfs_prev) { r = set_put(set, other); if (r < 0) return r; diff --git a/src/core/swap.c b/src/core/swap.c index 42d2e0242ec..48ba5c76649 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -1323,11 +1323,11 @@ static Unit *swap_following(Unit *u) { if (streq_ptr(s->what, s->devnode)) return NULL; - LIST_FOREACH_AFTER(same_devnode, other, s) + LIST_FOREACH(same_devnode, other, s->same_devnode_next) if (streq_ptr(other->what, other->devnode)) return UNIT(other); - LIST_FOREACH_BEFORE(same_devnode, other, s) { + LIST_FOREACH_BACKWARDS(same_devnode, other, s->same_devnode_prev) { if (streq_ptr(other->what, other->devnode)) return UNIT(other); diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index d050134aef5..cb7de261d5a 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1154,7 +1154,7 @@ static void rule_resolve_goto(UdevRuleFile *rule_file) { if (!FLAGS_SET(line->type, LINE_HAS_GOTO)) continue; - LIST_FOREACH_AFTER(rule_lines, i, line) + LIST_FOREACH(rule_lines, i, line->rule_lines_next) if (streq_ptr(i->label, line->goto_label)) { line->goto_line = i; break; From 044ac33c35ab1aeb35fc8b84462a9549cbbac294 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Jun 2021 16:57:32 +0900 Subject: [PATCH 12/13] udev: do not try to find blocker again when no blocker found previously --- src/udev/udevd.c | 45 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index f5bdd372316..411d90e67b9 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -780,6 +780,35 @@ static int event_is_blocked(Event *event) { /* lookup event for identical, parent, child device */ + assert(event); + assert(event->manager); + assert(event->blocker_seqnum <= event->seqnum); + + if (event->blocker_seqnum == event->seqnum) + /* we have checked previously and no blocker found */ + return false; + + LIST_FOREACH(event, loop_event, event->manager->events) { + /* we already found a later event, earlier cannot block us, no need to check again */ + if (loop_event->seqnum < event->blocker_seqnum) + continue; + + /* event we checked earlier still exists, no need to check again */ + if (loop_event->seqnum == event->blocker_seqnum) + return true; + + /* found ourself, no later event can block us */ + if (loop_event->seqnum >= event->seqnum) + goto no_blocker; + + /* found event we have not checked */ + break; + } + + assert(loop_event); + assert(loop_event->seqnum > event->blocker_seqnum && + loop_event->seqnum < event->seqnum); + r = sd_device_get_subsystem(event->dev, &subsystem); if (r < 0) return r; @@ -805,21 +834,13 @@ static int event_is_blocked(Event *event) { return r; /* check if queue contains events we depend on */ - LIST_FOREACH(event, loop_event, event->manager->events) { + LIST_FOREACH(event, loop_event, loop_event) { size_t loop_devpath_len, common; const char *loop_devpath; - /* we already found a later event, earlier cannot block us, no need to check again */ - if (loop_event->seqnum < event->blocker_seqnum) - continue; - - /* event we checked earlier still exists, no need to check again */ - if (loop_event->seqnum == event->blocker_seqnum) - return true; - /* found ourself, no later event can block us */ if (loop_event->seqnum >= event->seqnum) - return false; + goto no_blocker; /* check major/minor */ if (major(devnum) != 0) { @@ -879,6 +900,10 @@ static int event_is_blocked(Event *event) { event->blocker_seqnum = loop_event->seqnum; return true; + +no_blocker: + event->blocker_seqnum = event->seqnum; + return false; } static int event_queue_start(Manager *manager) { From c6f78234d1d1c6065ecc56240f217d1fdbeb1771 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Jun 2021 17:14:10 +0900 Subject: [PATCH 13/13] udev: skip event when its dependency cannot be checked --- src/udev/udevd.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 411d90e67b9..4df08f21c5c 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -907,7 +907,7 @@ no_blocker: } static int event_queue_start(Manager *manager) { - Event *event; + Event *event, *event_next; usec_t usec; int r; @@ -940,12 +940,26 @@ static int event_queue_start(Manager *manager) { return log_warning_errno(r, "Failed to read udev rules: %m"); } - LIST_FOREACH(event, event, manager->events) { + LIST_FOREACH_SAFE(event, event, event_next, manager->events) { if (event->state != EVENT_QUEUED) continue; - /* do not start event if parent or child event is still running */ - if (event_is_blocked(event) != 0) + /* do not start event if parent or child event is still running or queued */ + r = event_is_blocked(event); + if (r < 0) { + sd_device_action_t a = _SD_DEVICE_ACTION_INVALID; + + (void) sd_device_get_action(event->dev, &a); + log_device_warning_errno(event->dev, r, + "Failed to check event dependency, " + "skipping event (SEQNUM=%"PRIu64", ACTION=%s)", + event->seqnum, + strna(device_action_to_string(a))); + + event_free(event); + return r; + } + if (r > 0) continue; r = event_run(event);