[systemd-commits] 10 commits - src/udev

Tom Gundersen tomegun at kemper.freedesktop.org
Wed May 6 09:36:36 PDT 2015


 src/udev/udevd.c |  280 ++++++++++++++++++++++++++-----------------------------
 1 file changed, 136 insertions(+), 144 deletions(-)

New commits:
commit c4fcf70a015e47ec894e1b9d0c1940caf652195b
Author: Tom Gundersen <teg at jklm.no>
Date:   Wed May 6 17:36:39 2015 +0200

    udevd: worker - allow passing NULL to worker_unref()

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 0458af1..661047e 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -150,8 +150,7 @@ static void worker_cleanup(struct worker *worker) {
 }
 
 static void worker_unref(struct worker *worker) {
-        worker->refcount--;
-        if (worker->refcount > 0)
+        if (!worker || (-- worker->refcount) > 0)
                 return;
         log_debug("worker ["PID_FMT"] cleaned up", worker->pid);
         worker_cleanup(worker);

commit 8b46c3fc48767e36dbfb6b6ca3d8a12aa024fd40
Author: Tom Gundersen <teg at jklm.no>
Date:   Tue Apr 21 15:53:10 2015 +0200

    udevd: worker - use _exit() rather than exit()
    
    Follow the coding style and avoid the exit handlers.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 0a09276..0458af1 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -409,7 +409,7 @@ out:
                 udev_builtin_exit(udev);
                 udev_unref(udev);
                 log_close();
-                exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS);
+                _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS);
         }
         case -1:
                 event->state = EVENT_QUEUED;

commit 6af5e6a4c918a68b196a04346732e094e5373a36
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Apr 27 14:56:21 2015 +0200

    udevd: modernize error handling
    
    We never return magic exit codes, but just EXIT_FAILUER or EXIT_SUCCESS.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 451d0f1..0a09276 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -223,7 +223,7 @@ static void worker_spawn(struct event *event) {
                 _cleanup_rtnl_unref_ sd_rtnl *rtnl = NULL;
                 struct epoll_event ep_signal, ep_monitor;
                 sigset_t mask;
-                int rc = EXIT_SUCCESS;
+                int r = 0;
 
                 /* take initial device from queue */
                 dev = event->dev;
@@ -240,15 +240,13 @@ static void worker_spawn(struct event *event) {
                 sigfillset(&mask);
                 fd_signal = signalfd(-1, &mask, SFD_NONBLOCK|SFD_CLOEXEC);
                 if (fd_signal < 0) {
-                        log_error_errno(errno, "error creating signalfd %m");
-                        rc = 2;
+                        r = log_error_errno(errno, "error creating signalfd %m");
                         goto out;
                 }
 
                 fd_ep = epoll_create1(EPOLL_CLOEXEC);
                 if (fd_ep < 0) {
-                        log_error_errno(errno, "error creating epoll fd: %m");
-                        rc = 3;
+                        r = log_error_errno(errno, "error creating epoll fd: %m");
                         goto out;
                 }
 
@@ -263,8 +261,7 @@ static void worker_spawn(struct event *event) {
 
                 if (epoll_ctl(fd_ep, EPOLL_CTL_ADD, fd_signal, &ep_signal) < 0 ||
                     epoll_ctl(fd_ep, EPOLL_CTL_ADD, fd_monitor, &ep_monitor) < 0) {
-                        log_error_errno(errno, "fail to add fds to epoll: %m");
-                        rc = 4;
+                        r = log_error_errno(errno, "fail to add fds to epoll: %m");
                         goto out;
                 }
 
@@ -277,12 +274,12 @@ static void worker_spawn(struct event *event) {
                 for (;;) {
                         struct udev_event *udev_event;
                         struct worker_message msg;
-                        int fd_lock = -1, r;
+                        int fd_lock = -1;
 
                         log_debug("seq %llu running", udev_device_get_seqnum(dev));
                         udev_event = udev_event_new(dev);
                         if (udev_event == NULL) {
-                                rc = 5;
+                                r = -ENOMEM;
                                 goto out;
                         }
 
@@ -314,6 +311,7 @@ static void worker_spawn(struct event *event) {
                                         if (fd_lock >= 0 && flock(fd_lock, LOCK_SH|LOCK_NB) < 0) {
                                                 log_debug_errno(errno, "Unable to flock(%s), skipping event handling: %m", udev_device_get_devnode(d));
                                                 fd_lock = safe_close(fd_lock);
+                                                r = -EAGAIN;
                                                 goto skip;
                                         }
                                 }
@@ -378,7 +376,7 @@ skip:
                                 if (fdcount < 0) {
                                         if (errno == EINTR)
                                                 continue;
-                                        log_error_errno(errno, "failed to poll: %m");
+                                        r = log_error_errno(errno, "failed to poll: %m");
                                         goto out;
                                 }
 
@@ -411,7 +409,7 @@ out:
                 udev_builtin_exit(udev);
                 udev_unref(udev);
                 log_close();
-                exit(rc);
+                exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS);
         }
         case -1:
                 event->state = EVENT_QUEUED;
@@ -1176,11 +1174,13 @@ int main(int argc, char *argv[]) {
         struct epoll_event ep_netlink = { .events = EPOLLIN };
         struct epoll_event ep_worker = { .events = EPOLLIN };
         struct udev_ctrl_connection *ctrl_conn = NULL;
-        int rc = 1, r, one = 1;
+        int r = 0, one = 1;
 
         udev = udev_new();
-        if (udev == NULL)
+        if (!udev) {
+                r = log_error_errno(errno, "could not allocate udev context: %m");
                 goto exit;
+        }
 
         log_set_target(LOG_TARGET_AUTO);
         log_parse_environment();
@@ -1198,7 +1198,7 @@ int main(int argc, char *argv[]) {
                 log_set_max_level(LOG_DEBUG);
 
         if (getuid() != 0) {
-                log_error("root privileges required");
+                r = log_error_errno(EPERM, "root privileges required");
                 goto exit;
         }
 
@@ -1211,7 +1211,7 @@ int main(int argc, char *argv[]) {
         /* set umask before creating any file/directory */
         r = chdir("/");
         if (r < 0) {
-                log_error_errno(errno, "could not change dir to /: %m");
+                r = log_error_errno(errno, "could not change dir to /: %m");
                 goto exit;
         }
 
@@ -1221,7 +1221,7 @@ int main(int argc, char *argv[]) {
 
         r = mkdir("/run/udev", 0755);
         if (r < 0 && errno != EEXIST) {
-                log_error_errno(errno, "could not create /run/udev: %m");
+                r = log_error_errno(errno, "could not create /run/udev: %m");
                 goto exit;
         }
 
@@ -1247,16 +1247,14 @@ int main(int argc, char *argv[]) {
         if (systemd_fds(udev, &fd_ctrl, &fd_netlink) >= 0) {
                 /* get control and netlink socket from systemd */
                 udev_ctrl = udev_ctrl_new_from_fd(udev, fd_ctrl);
-                if (udev_ctrl == NULL) {
-                        log_error("error taking over udev control socket");
-                        rc = 1;
+                if (!udev_ctrl) {
+                        r = log_error_errno(EINVAL, "error taking over udev control socket");
                         goto exit;
                 }
 
                 monitor = udev_monitor_new_from_netlink_fd(udev, "kernel", fd_netlink);
-                if (monitor == NULL) {
-                        log_error("error taking over netlink socket");
-                        rc = 3;
+                if (!monitor) {
+                        r = log_error_errno(EINVAL, "error taking over netlink socket");
                         goto exit;
                 }
 
@@ -1266,17 +1264,15 @@ int main(int argc, char *argv[]) {
         } else {
                 /* open control and netlink socket */
                 udev_ctrl = udev_ctrl_new(udev);
-                if (udev_ctrl == NULL) {
-                        log_error("error initializing udev control socket");
-                        rc = 1;
+                if (!udev_ctrl) {
+                        r = log_error_errno(EINVAL, "error initializing udev control socket");
                         goto exit;
                 }
                 fd_ctrl = udev_ctrl_get_fd(udev_ctrl);
 
                 monitor = udev_monitor_new_from_netlink(udev, "kernel");
-                if (monitor == NULL) {
-                        log_error("error initializing netlink socket");
-                        rc = 3;
+                if (!monitor) {
+                        r = log_error_errno(EINVAL, "error initializing netlink socket");
                         goto exit;
                 }
                 fd_netlink = udev_monitor_get_fd(monitor);
@@ -1285,14 +1281,12 @@ int main(int argc, char *argv[]) {
         }
 
         if (udev_monitor_enable_receiving(monitor) < 0) {
-                log_error("error binding netlink socket");
-                rc = 3;
+                r = log_error_errno(EINVAL, "error binding netlink socket");
                 goto exit;
         }
 
         if (udev_ctrl_enable_receiving(udev_ctrl) < 0) {
-                log_error("error binding udev control socket");
-                rc = 1;
+                r = log_error_errno(EINVAL, "error binding udev control socket");
                 goto exit;
         }
 
@@ -1301,14 +1295,14 @@ int main(int argc, char *argv[]) {
         udev_builtin_init(udev);
 
         rules = udev_rules_new(udev, arg_resolve_names);
-        if (rules == NULL) {
-                log_error("error reading rules");
+        if (!rules) {
+                r = log_error_errno(ENOMEM, "error reading rules");
                 goto exit;
         }
 
-        rc = udev_rules_apply_static_dev_perms(rules);
-        if (rc < 0)
-                log_error_errno(rc, "failed to apply permissions on static device nodes - %m");
+        r = udev_rules_apply_static_dev_perms(rules);
+        if (r < 0)
+                log_error_errno(r, "failed to apply permissions on static device nodes: %m");
 
         if (arg_daemonize) {
                 pid_t pid;
@@ -1318,11 +1312,9 @@ int main(int argc, char *argv[]) {
                 case 0:
                         break;
                 case -1:
-                        log_error_errno(errno, "fork of daemon failed: %m");
-                        rc = 4;
+                        r = log_error_errno(errno, "fork of daemon failed: %m");
                         goto exit;
                 default:
-                        rc = EXIT_SUCCESS;
                         goto exit_daemonize;
                 }
 
@@ -1349,8 +1341,7 @@ int main(int argc, char *argv[]) {
 
         fd_inotify = udev_watch_init(udev);
         if (fd_inotify < 0) {
-                log_error("error initializing inotify");
-                rc = 4;
+                r = log_error_errno(ENOMEM, "error initializing inotify");
                 goto exit;
         }
         udev_watch_restore(udev);
@@ -1360,15 +1351,13 @@ int main(int argc, char *argv[]) {
         sigprocmask(SIG_SETMASK, &mask, &sigmask_orig);
         fd_signal = signalfd(-1, &mask, SFD_NONBLOCK|SFD_CLOEXEC);
         if (fd_signal < 0) {
-                log_error("error creating signalfd");
-                rc = 5;
+                r = log_error_errno(errno, "error creating signalfd");
                 goto exit;
         }
 
         /* unnamed socket from workers to the main daemon */
         if (socketpair(AF_LOCAL, SOCK_DGRAM|SOCK_CLOEXEC, 0, worker_watch) < 0) {
-                log_error("error creating socketpair");
-                rc = 6;
+                r = log_error_errno(errno, "error creating socketpair");
                 goto exit;
         }
         fd_worker = worker_watch[READ_END];
@@ -1599,7 +1588,6 @@ int main(int argc, char *argv[]) {
                         ctrl_conn = handle_ctrl_msg(udev_ctrl);
         }
 
-        rc = EXIT_SUCCESS;
 exit:
         udev_ctrl_cleanup(udev_ctrl);
         unlink("/run/udev/queue");
@@ -1623,5 +1611,5 @@ exit_daemonize:
         mac_selinux_finish();
         udev_unref(udev);
         log_close();
-        return rc;
+        return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
 }

commit 614a823c4416467d324c3236f58bfac34da33979
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri Apr 17 23:18:24 2015 +0200

    udevd: use kernel cmdline parser

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index fc745e4..451d0f1 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -1025,60 +1025,51 @@ static int systemd_fds(struct udev *udev, int *rctrl, int *rnetlink) {
 
 /*
  * read the kernel command line, in case we need to get into debug mode
- *   udev.log-priority=<level>              syslog priority
- *   udev.children-max=<number of workers>  events are fully serialized if set to 1
- *   udev.exec-delay=<number of seconds>    delay execution of every executed program
+ *   udev.log-priority=<level>                 syslog priority
+ *   udev.children-max=<number of workers>     events are fully serialized if set to 1
+ *   udev.exec-delay=<number of seconds>       delay execution of every executed program
+ *   udev.event-timeout=<number of seconds>    seconds to wait before terminating an event
  */
-static void kernel_cmdline_options(struct udev *udev) {
-        _cleanup_free_ char *line = NULL;
-        const char *word, *state;
-        size_t l;
+static int parse_proc_cmdline_item(const char *key, const char *value) {
         int r;
 
-        r = proc_cmdline(&line);
-        if (r < 0) {
-                log_warning_errno(r, "Failed to read /proc/cmdline, ignoring: %m");
-                return;
-        }
+        assert(key);
 
-        FOREACH_WORD_QUOTED(word, l, line, state) {
-                char *s, *opt, *value;
+        if (!value)
+                return 0;
 
-                s = strndup(word, l);
-                if (!s)
-                        break;
+        if (startswith(key, "rd."))
+                key += strlen("rd.");
 
-                /* accept the same options for the initrd, prefixed with "rd." */
-                if (in_initrd() && startswith(s, "rd."))
-                        opt = s + 3;
-                else
-                        opt = s;
+        if (startswith(key, "udev."))
+                key += strlen("udev.");
+        else
+                return 0;
 
-                if ((value = startswith(opt, "udev.log-priority="))) {
-                        int prio;
+        if (streq(key, "log-priority")) {
+                int prio;
 
-                        prio = util_log_priority(value);
-                        log_set_max_level(prio);
-                } else if ((value = startswith(opt, "udev.children-max="))) {
-                        r = safe_atoi(value, &arg_children_max);
-                        if (r < 0)
-                                log_warning("Invalid udev.children-max ignored: %s", value);
-                } else if ((value = startswith(opt, "udev.exec-delay="))) {
-                        r = safe_atoi(value, &arg_exec_delay);
-                        if (r < 0)
-                                log_warning("Invalid udev.exec-delay ignored: %s", value);
-                } else if ((value = startswith(opt, "udev.event-timeout="))) {
-                        r = safe_atou64(value, &arg_event_timeout_usec);
-                        if (r < 0) {
-                                log_warning("Invalid udev.event-timeout ignored: %s", value);
-                                break;
-                        }
+                prio = util_log_priority(value);
+                log_set_max_level(prio);
+        } else if (streq(key, "children-max")) {
+                r = safe_atoi(value, &arg_children_max);
+                if (r < 0)
+                        log_warning("invalid udev.children-max ignored: %s", value);
+        } else if (streq(key, "exec-delay")) {
+                r = safe_atoi(value, &arg_exec_delay);
+                if (r < 0)
+                        log_warning("invalid udev.exec-delay ignored: %s", value);
+        } else if (streq(key, "event-timeout")) {
+                r = safe_atou64(value, &arg_event_timeout_usec);
+                if (r < 0)
+                        log_warning("invalid udev.event-timeout ignored: %s", value);
+                else {
                         arg_event_timeout_usec *= USEC_PER_SEC;
                         arg_event_timeout_warn_usec = (arg_event_timeout_usec / 3) ? : 1;
                 }
-
-                free(s);
         }
+
+        return 0;
 }
 
 static void help(void) {
@@ -1199,7 +1190,9 @@ int main(int argc, char *argv[]) {
         if (r <= 0)
                 goto exit;
 
-        kernel_cmdline_options(udev);
+        r = parse_proc_cmdline(parse_proc_cmdline_item);
+        if (r < 0)
+                log_warning_errno(r, "failed to parse kernel command line, ignoring: %m");
 
         if (arg_debug)
                 log_set_max_level(LOG_DEBUG);

commit 9c2dabd05f9a9d6145d1ea6a1dae8afb569ed4c5
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Apr 27 12:14:38 2015 +0200

    udevd: worker - move some fields from the worker to the event

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index fafe397..fc745e4 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -94,6 +94,8 @@ struct event {
         dev_t devnum;
         int ifindex;
         bool is_block;
+        usec_t start_usec;
+        bool warned;
 };
 
 static inline struct event *node_to_event(struct udev_list_node *node) {
@@ -117,8 +119,6 @@ struct worker {
         struct udev_monitor *monitor;
         enum worker_state state;
         struct event *event;
-        usec_t event_start_usec;
-        bool event_warned;
 };
 
 /* passed from worker to main process */
@@ -195,10 +195,10 @@ static int worker_new(struct worker **ret, struct udev *udev, struct udev_monito
 
 static void worker_attach_event(struct worker *worker, struct event *event) {
         worker->state = WORKER_RUNNING;
-        worker->event_start_usec = now(CLOCK_MONOTONIC);
-        worker->event_warned = false;
         worker->event = event;
         event->state = EVENT_RUNNING;
+        event->start_usec = now(CLOCK_MONOTONIC);
+        event->warned = false;
         worker_ref(worker);
 }
 
@@ -1476,23 +1476,26 @@ int main(int argc, char *argv[]) {
                         /* check for hanging events */
                         udev_list_node_foreach(loop, &worker_list) {
                                 struct worker *worker = node_to_worker(loop);
+                                struct event *event = worker->event;
                                 usec_t ts;
 
                                 if (worker->state != WORKER_RUNNING)
                                         continue;
 
+                                assert(event);
+
                                 ts = now(CLOCK_MONOTONIC);
 
-                                if ((ts - worker->event_start_usec) > arg_event_timeout_warn_usec) {
-                                        if ((ts - worker->event_start_usec) > arg_event_timeout_usec) {
-                                                log_error("worker ["PID_FMT"] %s timeout; kill it", worker->pid, worker->event->devpath);
+                                if ((ts - event->start_usec) > arg_event_timeout_warn_usec) {
+                                        if ((ts - event->start_usec) > arg_event_timeout_usec) {
+                                                log_error("worker ["PID_FMT"] %s timeout; kill it", worker->pid, event->devpath);
                                                 kill(worker->pid, SIGKILL);
                                                 worker->state = WORKER_KILLED;
 
-                                                log_error("seq %llu '%s' killed", udev_device_get_seqnum(worker->event->dev), worker->event->devpath);
-                                        } else if (!worker->event_warned) {
-                                                log_warning("worker ["PID_FMT"] %s is taking a long time", worker->pid, worker->event->devpath);
-                                                worker->event_warned = true;
+                                                log_error("seq %llu '%s' killed", udev_device_get_seqnum(event->dev), event->devpath);
+                                        } else if (!event->warned) {
+                                                log_warning("worker ["PID_FMT"] %s is taking a long time", worker->pid, event->devpath);
+                                                event->warned = true;
                                         }
                                 }
                         }

commit 39c19cf13a5d32887977f06cb34e4c95e6da3f56
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Apr 27 11:33:41 2015 +0200

    udevd: worker - introduce worker_attach_event()

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index f8dbe26..fafe397 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -179,8 +179,7 @@ static int worker_new(struct worker **ret, struct udev *udev, struct udev_monito
         if (!worker)
                 return -ENOMEM;
 
-        /* worker + event reference */
-        worker->refcount = 2;
+        worker->refcount = 1;
         worker->udev = udev_ref(udev);
         /* close monitor, but keep address around */
         udev_monitor_disconnect(worker_monitor);
@@ -194,6 +193,15 @@ static int worker_new(struct worker **ret, struct udev *udev, struct udev_monito
         return 0;
 }
 
+static void worker_attach_event(struct worker *worker, struct event *event) {
+        worker->state = WORKER_RUNNING;
+        worker->event_start_usec = now(CLOCK_MONOTONIC);
+        worker->event_warned = false;
+        worker->event = event;
+        event->state = EVENT_RUNNING;
+        worker_ref(worker);
+}
+
 static void worker_spawn(struct event *event) {
         struct udev *udev = event->udev;
         _cleanup_udev_monitor_unref_ struct udev_monitor *worker_monitor = NULL;
@@ -418,11 +426,8 @@ out:
                 if (r < 0)
                         return;
 
-                worker->state = WORKER_RUNNING;
-                worker->event_start_usec = now(CLOCK_MONOTONIC);
-                worker->event_warned = false;
-                worker->event = event;
-                event->state = EVENT_RUNNING;
+                worker_attach_event(worker, event);
+
                 log_debug("seq %llu forked new worker ["PID_FMT"]", udev_device_get_seqnum(event->dev), pid);
                 break;
         }
@@ -447,12 +452,7 @@ static void event_run(struct event *event) {
                         worker->state = WORKER_KILLED;
                         continue;
                 }
-                worker_ref(worker);
-                worker->event = event;
-                worker->state = WORKER_RUNNING;
-                worker->event_start_usec = now(CLOCK_MONOTONIC);
-                worker->event_warned = false;
-                event->state = EVENT_RUNNING;
+                worker_attach_event(worker, event);
                 return;
         }
 

commit 3a19b32a673f133f8b953869dd11a2a17344856c
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Apr 27 11:26:47 2015 +0200

    udevd: worker - make refcounting clearer
    
    Take and drop explicit references where it makes sense.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 4ab3216..f8dbe26 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -144,6 +144,7 @@ static struct worker *worker_ref(struct worker *worker) {
 static void worker_cleanup(struct worker *worker) {
         udev_list_node_remove(&worker->node);
         udev_monitor_unref(worker->monitor);
+        udev_unref(worker->udev);
         children--;
         free(worker);
 }
@@ -166,9 +167,36 @@ static void worker_list_cleanup(struct udev *udev) {
         }
 }
 
+static int worker_new(struct worker **ret, struct udev *udev, struct udev_monitor *worker_monitor, pid_t pid) {
+        struct worker *worker;
+
+        assert(ret);
+        assert(udev);
+        assert(worker_monitor);
+        assert(pid > 1);
+
+        worker = new0(struct worker, 1);
+        if (!worker)
+                return -ENOMEM;
+
+        /* worker + event reference */
+        worker->refcount = 2;
+        worker->udev = udev_ref(udev);
+        /* close monitor, but keep address around */
+        udev_monitor_disconnect(worker_monitor);
+        worker->monitor = udev_monitor_ref(worker_monitor);
+        worker->pid = pid;
+        udev_list_node_append(&worker->node, &worker_list);
+        children++;
+
+        *ret = worker;
+
+        return 0;
+}
+
 static void worker_spawn(struct event *event) {
         struct udev *udev = event->udev;
-        struct udev_monitor *worker_monitor;
+        _cleanup_udev_monitor_unref_ struct udev_monitor *worker_monitor = NULL;
         pid_t pid;
 
         /* listen for new events */
@@ -373,40 +401,28 @@ out:
                 close(worker_watch[WRITE_END]);
                 udev_rules_unref(rules);
                 udev_builtin_exit(udev);
-                udev_monitor_unref(worker_monitor);
                 udev_unref(udev);
                 log_close();
                 exit(rc);
         }
         case -1:
-                udev_monitor_unref(worker_monitor);
                 event->state = EVENT_QUEUED;
                 log_error_errno(errno, "fork of child failed: %m");
                 break;
         default:
         {
                 struct worker *worker;
+                int r;
 
-                worker = new0(struct worker, 1);
-                if (!worker) {
-                        udev_monitor_unref(worker_monitor);
+                r = worker_new(&worker, udev, worker_monitor, pid);
+                if (r < 0)
                         return;
-                }
 
-                /* worker + event reference */
-                worker->refcount = 2;
-                worker->udev = udev;
-                /* close monitor, but keep address around */
-                udev_monitor_disconnect(worker_monitor);
-                worker->monitor = worker_monitor;
-                worker->pid = pid;
                 worker->state = WORKER_RUNNING;
                 worker->event_start_usec = now(CLOCK_MONOTONIC);
                 worker->event_warned = false;
                 worker->event = event;
                 event->state = EVENT_RUNNING;
-                udev_list_node_append(&worker->node, &worker_list);
-                children++;
                 log_debug("seq %llu forked new worker ["PID_FMT"]", udev_device_get_seqnum(event->dev), pid);
                 break;
         }

commit e03c7cc26ed731245827ada3aa24f6c937ff80a1
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Apr 27 11:11:58 2015 +0200

    udevd: worker - only allocate the worker struct in the main process
    
    This is not used in the worker, so avoid having to free it there.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 1eb7b77..4ab3216 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -168,7 +168,6 @@ static void worker_list_cleanup(struct udev *udev) {
 
 static void worker_spawn(struct event *event) {
         struct udev *udev = event->udev;
-        struct worker *worker;
         struct udev_monitor *worker_monitor;
         pid_t pid;
 
@@ -180,15 +179,6 @@ static void worker_spawn(struct event *event) {
         udev_monitor_allow_unicast_sender(worker_monitor, monitor);
         udev_monitor_enable_receiving(worker_monitor);
 
-        worker = new0(struct worker, 1);
-        if (worker == NULL) {
-                udev_monitor_unref(worker_monitor);
-                return;
-        }
-        /* worker + event reference */
-        worker->refcount = 2;
-        worker->udev = udev;
-
         pid = fork();
         switch (pid) {
         case 0: {
@@ -203,7 +193,6 @@ static void worker_spawn(struct event *event) {
                 dev = event->dev;
                 event->dev = NULL;
 
-                free(worker);
                 worker_list_cleanup(udev);
                 event_queue_cleanup(udev, EVENT_UNDEF);
                 udev_monitor_unref(monitor);
@@ -392,10 +381,21 @@ out:
         case -1:
                 udev_monitor_unref(worker_monitor);
                 event->state = EVENT_QUEUED;
-                free(worker);
                 log_error_errno(errno, "fork of child failed: %m");
                 break;
         default:
+        {
+                struct worker *worker;
+
+                worker = new0(struct worker, 1);
+                if (!worker) {
+                        udev_monitor_unref(worker_monitor);
+                        return;
+                }
+
+                /* worker + event reference */
+                worker->refcount = 2;
+                worker->udev = udev;
                 /* close monitor, but keep address around */
                 udev_monitor_disconnect(worker_monitor);
                 worker->monitor = worker_monitor;
@@ -410,6 +410,7 @@ out:
                 log_debug("seq %llu forked new worker ["PID_FMT"]", udev_device_get_seqnum(event->dev), pid);
                 break;
         }
+        }
 }
 
 static void event_run(struct event *event) {

commit f96a5160c48574015cdd87d5991f65f8e03f0c72
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Apr 27 11:08:38 2015 +0200

    udevd: rename worker_new() to worker_spawn()

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 13f5bf1..1eb7b77 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -166,7 +166,7 @@ static void worker_list_cleanup(struct udev *udev) {
         }
 }
 
-static void worker_new(struct event *event) {
+static void worker_spawn(struct event *event) {
         struct udev *udev = event->udev;
         struct worker *worker;
         struct udev_monitor *worker_monitor;
@@ -446,7 +446,7 @@ static void event_run(struct event *event) {
         }
 
         /* start new worker and pass initial device */
-        worker_new(event);
+        worker_spawn(event);
 }
 
 static int event_queue_insert(struct udev_device *dev) {

commit 4914cb2ddc0aa2f71bddb5bb9a4bed7e6948d561
Author: Tom Gundersen <teg at jklm.no>
Date:   Sun Apr 26 01:40:12 2015 +0200

    udevd: don't track worker exitcode
    
    We used to use this to track failed events so they could be retriggered,
    but that is no longer done, so the code can be dropped.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 365bb00..13f5bf1 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -86,7 +86,6 @@ struct event {
         struct udev_device *dev;
         struct udev_device *dev_kernel;
         enum event_state state;
-        int exitcode;
         unsigned long long int delaying_seqnum;
         unsigned long long int seqnum;
         const char *devpath;
@@ -124,7 +123,6 @@ struct worker {
 
 /* passed from worker to main process */
 struct worker_message {
-        int exitcode;
 };
 
 static inline struct worker *node_to_worker(struct udev_list_node *node) {
@@ -254,8 +252,7 @@ static void worker_new(struct event *event) {
                 for (;;) {
                         struct udev_event *udev_event;
                         struct worker_message msg;
-                        int fd_lock = -1;
-                        int err = 0, r;
+                        int fd_lock = -1, r;
 
                         log_debug("seq %llu running", udev_device_get_seqnum(dev));
                         udev_event = udev_event_new(dev);
@@ -291,7 +288,6 @@ static void worker_new(struct event *event) {
                                         fd_lock = open(udev_device_get_devnode(d), O_RDONLY|O_CLOEXEC|O_NOFOLLOW|O_NONBLOCK);
                                         if (fd_lock >= 0 && flock(fd_lock, LOCK_SH|LOCK_NB) < 0) {
                                                 log_debug_errno(errno, "Unable to flock(%s), skipping event handling: %m", udev_device_get_devnode(d));
-                                                err = -EAGAIN;
                                                 fd_lock = safe_close(fd_lock);
                                                 goto skip;
                                         }
@@ -328,11 +324,10 @@ static void worker_new(struct event *event) {
                         udev_monitor_send_device(worker_monitor, NULL, dev);
 
 skip:
-                        log_debug("seq %llu processed with %i", udev_device_get_seqnum(dev), err);
+                        log_debug("seq %llu processed", udev_device_get_seqnum(dev));
 
                         /* send udevd the result of the event execution */
                         memzero(&msg, sizeof(struct worker_message));
-                        msg.exitcode = err;
                         r = send(worker_watch[WRITE_END], &msg, sizeof(struct worker_message), 0);
                         if (r < 0)
                                 log_error_errno(errno, "failed to send result of seq %llu to main daemon: %m",
@@ -655,7 +650,6 @@ static void worker_returned(int fd_worker) {
 
                         /* worker returned */
                         if (worker->event) {
-                                worker->event->exitcode = msg.exitcode;
                                 event_queue_delete(worker->event);
                                 worker->event = NULL;
                         }
@@ -938,7 +932,6 @@ static void handle_signal(struct udev *udev, int signo) {
                                         if (worker->event) {
                                                 log_error("worker ["PID_FMT"] failed while handling '%s'",
                                                           pid, worker->event->devpath);
-                                                worker->event->exitcode = -32;
                                                 /* delete state from disk */
                                                 udev_device_delete_db(worker->event->dev);
                                                 udev_device_tag_index(worker->event->dev, NULL, false);
@@ -1480,7 +1473,6 @@ int main(int argc, char *argv[]) {
                                                 worker->state = WORKER_KILLED;
 
                                                 log_error("seq %llu '%s' killed", udev_device_get_seqnum(worker->event->dev), worker->event->devpath);
-                                                worker->event->exitcode = -64;
                                         } else if (!worker->event_warned) {
                                                 log_warning("worker ["PID_FMT"] %s is taking a long time", worker->pid, worker->event->devpath);
                                                 worker->event_warned = true;



More information about the systemd-commits mailing list