[systemd-commits] 6 commits - src/shared src/udev

Tom Gundersen tomegun at kemper.freedesktop.org
Sat May 16 01:33:07 PDT 2015


 src/shared/util.c          |    6 +--
 src/udev/net/link-config.c |    6 +--
 src/udev/udev-ctrl.c       |   50 ++++++++++++----------------
 src/udev/udevd.c           |   80 +++++++++++++++++++++++++--------------------
 4 files changed, 75 insertions(+), 67 deletions(-)

New commits:
commit cb49a4f2dd53184ab787bcad69bcc9b8177d2e6e
Author: Tom Gundersen <teg at jklm.no>
Date:   Sat May 16 10:14:20 2015 +0200

    udevd: queue - update queue state when events are queued/freed
    
    This way it is more obvious that the queue flag file is always
    up-to-date. Moreover, we only have to touch/unlink it when the
    first/last event is allocated/freed.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index a7f14c0..37cf0de 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -69,6 +69,7 @@ typedef struct Manager {
         Hashmap *workers;
         struct udev_list_node events;
         char *cgroup;
+        pid_t pid; /* the process that originally allocated the manager object */
 
         struct udev_rules *rules;
         struct udev_list properties;
@@ -90,6 +91,7 @@ enum event_state {
 
 struct event {
         struct udev_list_node node;
+        Manager *manager;
         struct udev *udev;
         struct udev_device *dev;
         struct udev_device *dev_kernel;
@@ -135,6 +137,8 @@ struct worker_message {
 };
 
 static void event_free(struct event *event) {
+        int r;
+
         if (!event)
                 return;
 
@@ -145,6 +149,17 @@ static void event_free(struct event *event) {
         if (event->worker)
                 event->worker->event = NULL;
 
+        assert(event->manager);
+
+        if (udev_list_node_is_empty(&event->manager->events)) {
+                /* only clean up the queue from the process that created it */
+                if (event->manager->pid == getpid()) {
+                        r = unlink("/run/udev/queue");
+                        if (r < 0)
+                                log_warning_errno(errno, "could not unlink /run/udev/queue: %m");
+                }
+        }
+
         free(event);
 }
 
@@ -492,15 +507,20 @@ static void event_run(Manager *manager, struct event *event) {
 
 static int event_queue_insert(Manager *manager, struct udev_device *dev) {
         struct event *event;
+        int r;
 
         assert(manager);
         assert(dev);
 
+        /* only the main process can add events to the queue */
+        assert(manager->pid == getpid());
+
         event = new0(struct event, 1);
-        if (event == NULL)
-                return -1;
+        if (!event)
+                return -ENOMEM;
 
         event->udev = udev_device_get_udev(dev);
+        event->manager = manager;
         event->dev = dev;
         event->dev_kernel = udev_device_shallow_clone(dev);
         udev_device_copy_properties(event->dev_kernel, dev);
@@ -516,7 +536,15 @@ static int event_queue_insert(Manager *manager, struct udev_device *dev) {
              udev_device_get_action(dev), udev_device_get_subsystem(dev));
 
         event->state = EVENT_QUEUED;
+
+        if (udev_list_node_is_empty(&manager->events)) {
+                r = touch("/run/udev/queue");
+                if (r < 0)
+                        log_warning_errno(r, "could not touch /run/udev/queue: %m");
+        }
+
         udev_list_node_append(&event->node, &manager->events);
+
         return 0;
 }
 
@@ -726,22 +754,6 @@ static int on_uevent(sd_event_source *s, int fd, uint32_t revents, void *userdat
         return 1;
 }
 
-static void event_queue_update(Manager *manager) {
-        int r;
-
-        assert(manager);
-
-        if (!udev_list_node_is_empty(&manager->events)) {
-                r = touch("/run/udev/queue");
-                if (r < 0)
-                        log_warning_errno(r, "could not touch /run/udev/queue: %m");
-        } else {
-                r = unlink("/run/udev/queue");
-                if (r < 0 && errno != ENOENT)
-                        log_warning("could not unlink /run/udev/queue: %m");
-        }
-}
-
 /* receive the udevd message from userspace */
 static int on_ctrl_msg(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
         Manager *manager = userdata;
@@ -813,13 +825,8 @@ static int on_ctrl_msg(sd_event_source *s, int fd, uint32_t revents, void *userd
                 arg_children_max = i;
         }
 
-        if (udev_ctrl_get_ping(ctrl_msg) > 0) {
+        if (udev_ctrl_get_ping(ctrl_msg) > 0)
                 log_debug("udevd message (SYNC) received");
-                /* tell settle that we are busy or idle, this needs to be before the
-                 * PING handling
-                 */
-                event_queue_update(manager);
-        }
 
         if (udev_ctrl_get_exit(ctrl_msg) > 0) {
                 log_debug("udevd message (EXIT) received");
@@ -1255,6 +1262,8 @@ static int manager_new(Manager **ret) {
         if (!manager)
                 return log_oom();
 
+        manager->pid = getpid();
+
         manager->udev = udev_new();
         if (!manager->udev)
                 return log_error_errno(errno, "could not allocate udev context: %m");
@@ -1528,9 +1537,6 @@ int main(int argc, char *argv[]) {
                         timeout = 3 * MSEC_PER_SEC;
                 }
 
-                /* tell settle that we are busy or idle */
-                event_queue_update(manager);
-
                 fdcount = epoll_wait(fd_ep, ev, ELEMENTSOF(ev), timeout);
                 if (fdcount < 0)
                         continue;
@@ -1668,7 +1674,6 @@ int main(int argc, char *argv[]) {
 
 exit:
         udev_ctrl_cleanup(manager->ctrl);
-        unlink("/run/udev/queue");
 exit_daemonize:
         if (fd_ep >= 0)
                 close(fd_ep);

commit 738a790778608a8590d93c790f11b484f1dd8da4
Author: Tom Gundersen <teg at jklm.no>
Date:   Sat May 16 01:12:21 2015 +0200

    udevd: on_worker - distinguish between EINTR and EAGAIN
    
    EAGAIN means there are no more messages to read, so give up. EINTR means we got interrupted
    reading a message, so try again.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 8142ec6..a7f14c0 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -667,8 +667,11 @@ static int on_worker(sd_event_source *s, int fd, uint32_t revents, void *userdat
 
                 size = recvmsg(fd, &msghdr, MSG_DONTWAIT);
                 if (size < 0) {
-                        if (errno == EAGAIN || errno == EINTR)
-                                return 1;
+                        if (errno == EINTR)
+                                continue;
+                        else if (errno == EAGAIN)
+                                /* nothing more to read */
+                                break;
 
                         return log_error_errno(errno, "failed to receive message: %m");
                 } else if (size != sizeof(struct worker_message)) {

commit 9a73bd7cab019aa1b9b4342ce2abd51b6e50b085
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri May 15 11:41:36 2015 +0200

    udevd: worker - use loop_write() rather than send()
    
    When notifying the main daemon about event completion, make sure the message is sent
    successfully, and not interrupted.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 318d769..8142ec6 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -221,6 +221,12 @@ static void worker_attach_event(struct worker *worker, struct event *event) {
         event->worker = worker;
 }
 
+static int worker_send_message(int fd) {
+        struct worker_message message = {};
+
+        return loop_write(fd, &message, sizeof(message), false);
+}
+
 static void worker_spawn(Manager *manager, struct event *event) {
         struct udev *udev = event->udev;
         _cleanup_udev_monitor_unref_ struct udev_monitor *worker_monitor = NULL;
@@ -292,7 +298,6 @@ static void worker_spawn(Manager *manager, struct event *event) {
 
                 for (;;) {
                         struct udev_event *udev_event;
-                        struct worker_message msg;
                         int fd_lock = -1;
 
                         log_debug("seq %llu running", udev_device_get_seqnum(dev));
@@ -369,10 +374,9 @@ skip:
                         log_debug("seq %llu processed", udev_device_get_seqnum(dev));
 
                         /* send udevd the result of the event execution */
-                        memzero(&msg, sizeof(struct worker_message));
-                        r = send(worker_watch[WRITE_END], &msg, sizeof(struct worker_message), 0);
+                        r = worker_send_message(worker_watch[WRITE_END]);
                         if (r < 0)
-                                log_error_errno(errno, "failed to send result of seq %llu to main daemon: %m",
+                                log_error_errno(r, "failed to send result of seq %llu to main daemon: %m",
                                                 udev_device_get_seqnum(dev));
 
                         udev_device_unref(dev);

commit 8c7e28a191709f90736054e6f7844cf35b4ab93d
Author: Tom Gundersen <teg at jklm.no>
Date:   Sat May 16 01:07:45 2015 +0200

    util: loop_write - accept 0-length message
    
    write() can send empty messages, so make sure loop_write() can do the same.

diff --git a/src/shared/util.c b/src/shared/util.c
index dda88bd..3060adc 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -1664,7 +1664,7 @@ int loop_write(int fd, const void *buf, size_t nbytes, bool do_poll) {
 
         errno = 0;
 
-        while (nbytes > 0) {
+        do {
                 ssize_t k;
 
                 k = write(fd, p, nbytes);
@@ -1684,12 +1684,12 @@ int loop_write(int fd, const void *buf, size_t nbytes, bool do_poll) {
                         return -errno;
                 }
 
-                if (k == 0) /* Can't really happen */
+                if (nbytes > 0 && k == 0) /* Can't really happen */
                         return -EIO;
 
                 p += k;
                 nbytes -= k;
-        }
+        } while (nbytes > 0);
 
         return 0;
 }

commit 43d60b77a83b3185e37c65c4f2649d24c227c7f0
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri May 15 23:59:28 2015 +0200

    udevd: net - fix leak in .link config
    
    Path, Driver and Type are now strv rather than strings, so free them properly.

diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c
index 00052dd..b3e7d02 100644
--- a/src/udev/net/link-config.c
+++ b/src/udev/net/link-config.c
@@ -67,9 +67,9 @@ static void link_config_free(link_config *link) {
         free(link->filename);
 
         free(link->match_mac);
-        free(link->match_path);
-        free(link->match_driver);
-        free(link->match_type);
+        strv_free(link->match_path);
+        strv_free(link->match_driver);
+        strv_free(link->match_type);
         free(link->match_name);
         free(link->match_host);
         free(link->match_virt);

commit 35927d13dfb8e313bf653f244fdb3fdb6089c982
Author: Tom Gundersen <teg at jklm.no>
Date:   Thu May 14 15:30:52 2015 +0200

    udev-ctrl: make _unref() always return NULL
    
    Bring this in line with the rest of the codebase.

diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c
index 0f357ba..b0ad277 100644
--- a/src/udev/udev-ctrl.c
+++ b/src/udev/udev-ctrl.c
@@ -140,21 +140,19 @@ struct udev *udev_ctrl_get_udev(struct udev_ctrl *uctrl) {
 }
 
 static struct udev_ctrl *udev_ctrl_ref(struct udev_ctrl *uctrl) {
-        if (uctrl == NULL)
-                return NULL;
-        uctrl->refcount++;
+        if (uctrl)
+                uctrl->refcount++;
+
         return uctrl;
 }
 
 struct udev_ctrl *udev_ctrl_unref(struct udev_ctrl *uctrl) {
-        if (uctrl == NULL)
-                return NULL;
-        uctrl->refcount--;
-        if (uctrl->refcount > 0)
-                return uctrl;
-        if (uctrl->sock >= 0)
-                close(uctrl->sock);
-        free(uctrl);
+        if (uctrl && -- uctrl->refcount == 0) {
+                if (uctrl->sock >= 0)
+                        close(uctrl->sock);
+                free(uctrl);
+        }
+
         return NULL;
 }
 
@@ -224,15 +222,15 @@ struct udev_ctrl_connection *udev_ctrl_connection_ref(struct udev_ctrl_connectio
 }
 
 struct udev_ctrl_connection *udev_ctrl_connection_unref(struct udev_ctrl_connection *conn) {
-        if (conn == NULL)
-                return NULL;
-        conn->refcount--;
-        if (conn->refcount > 0)
-                return conn;
-        if (conn->sock >= 0)
-                close(conn->sock);
-        udev_ctrl_unref(conn->uctrl);
-        free(conn);
+        if (conn && -- conn->refcount == 0) {
+                if (conn->sock >= 0)
+                        close(conn->sock);
+
+                udev_ctrl_unref(conn->uctrl);
+
+                free(conn);
+        }
+
         return NULL;
 }
 
@@ -405,13 +403,11 @@ err:
 }
 
 struct udev_ctrl_msg *udev_ctrl_msg_unref(struct udev_ctrl_msg *ctrl_msg) {
-        if (ctrl_msg == NULL)
-                return NULL;
-        ctrl_msg->refcount--;
-        if (ctrl_msg->refcount > 0)
-                return ctrl_msg;
-        udev_ctrl_connection_unref(ctrl_msg->conn);
-        free(ctrl_msg);
+        if (ctrl_msg && -- ctrl_msg->refcount == 0) {
+                udev_ctrl_connection_unref(ctrl_msg->conn);
+                free(ctrl_msg);
+        }
+
         return NULL;
 }
 



More information about the systemd-commits mailing list