[systemd-commits] 3 commits - src/udev
Tom Gundersen
tomegun at kemper.freedesktop.org
Wed May 6 14:54:26 PDT 2015
src/udev/udevd.c | 215 ++++++++++++++++++++++++++-----------------------------
1 file changed, 102 insertions(+), 113 deletions(-)
New commits:
commit 020328e197c1729832d1bb2761d2d2d1e952c41c
Author: Tom Gundersen <teg at jklm.no>
Date: Wed May 6 23:36:36 2015 +0200
udevd: don't explicitly count the number of workers
Simply query the size of the hashmap keeping all the worker contexts instead.
diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 1a2a38c..dd0cba5 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -60,11 +60,10 @@ static int fd_ep = -1;
static int fd_inotify = -1;
static bool stop_exec_queue;
static bool reload;
-static int children;
static bool arg_debug = false;
static int arg_daemonize = false;
static int arg_resolve_names = 1;
-static int arg_children_max;
+static unsigned arg_children_max;
static int arg_exec_delay;
static usec_t arg_event_timeout_usec = 180 * USEC_PER_SEC;
static usec_t arg_event_timeout_warn_usec = 180 * USEC_PER_SEC / 3;
@@ -154,8 +153,6 @@ static void worker_free(struct worker *worker) {
udev_unref(worker->udev);
event_free(worker->event);
- children--;
-
free(worker);
}
@@ -198,8 +195,6 @@ static int worker_new(struct worker **ret, struct udev *udev, struct udev_monito
if (r < 0)
return r;
- children++;
-
*ret = worker;
worker = NULL;
@@ -472,9 +467,9 @@ static void event_run(struct event *event) {
return;
}
- if (children >= arg_children_max) {
+ if (hashmap_size(workers) >= arg_children_max) {
if (arg_children_max > 1)
- log_debug("maximum number (%i) of children reached", children);
+ log_debug("maximum number (%i) of children reached", hashmap_size(workers));
return;
}
@@ -1044,7 +1039,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value) {
prio = util_log_priority(value);
log_set_max_level(prio);
} else if (streq(key, "children-max")) {
- r = safe_atoi(value, &arg_children_max);
+ r = safe_atou(value, &arg_children_max);
if (r < 0)
log_warning("invalid udev.children-max ignored: %s", value);
} else if (streq(key, "exec-delay")) {
@@ -1106,7 +1101,7 @@ static int parse_argv(int argc, char *argv[]) {
arg_daemonize = true;
break;
case 'c':
- r = safe_atoi(optarg, &arg_children_max);
+ r = safe_atou(optarg, &arg_children_max);
if (r < 0)
log_warning("Invalid --children-max ignored: %s", optarg);
break;
@@ -1319,7 +1314,7 @@ int main(int argc, char *argv[]) {
sd_notify(1, "READY=1");
}
- if (arg_children_max <= 0) {
+ if (arg_children_max == 0) {
cpu_set_t cpu_set;
arg_children_max = 8;
@@ -1409,12 +1404,12 @@ int main(int argc, char *argv[]) {
worker_kill(udev);
/* exit after all has cleaned up */
- if (udev_list_node_is_empty(&event_list) && children == 0)
+ if (udev_list_node_is_empty(&event_list) && hashmap_isempty(workers))
break;
/* timeout at exit for workers to finish */
timeout = 30 * MSEC_PER_SEC;
- } else if (udev_list_node_is_empty(&event_list) && children == 0) {
+ } else if (udev_list_node_is_empty(&event_list) && hashmap_isempty(workers)) {
/* we are idle */
timeout = -1;
commit a505965d955a5b90a5199b1be953e9707b19e3b5
Author: Tom Gundersen <teg at jklm.no>
Date: Wed May 6 23:26:25 2015 +0200
udevd: keep workers in a hashmap rather than a list
This makes the code somewhat more readable.
diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 8a97b4a..1a2a38c 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -49,6 +49,7 @@
#include "udev.h"
#include "udev-util.h"
#include "formats-util.h"
+#include "hashmap.h"
static struct udev_rules *rules;
static struct udev_ctrl *udev_ctrl;
@@ -69,7 +70,7 @@ static usec_t arg_event_timeout_usec = 180 * USEC_PER_SEC;
static usec_t arg_event_timeout_warn_usec = 180 * USEC_PER_SEC / 3;
static sigset_t sigmask_orig;
static UDEV_LIST(event_list);
-static UDEV_LIST(worker_list);
+Hashmap *workers;
static char *udev_cgroup;
static struct udev_list properties_list;
static bool udev_exit;
@@ -148,7 +149,7 @@ static void worker_free(struct worker *worker) {
if (!worker)
return;
- udev_list_node_remove(&worker->node);
+ hashmap_remove(workers, UINT_TO_PTR(worker->pid));
udev_monitor_unref(worker->monitor);
udev_unref(worker->udev);
event_free(worker->event);
@@ -158,18 +159,20 @@ static void worker_free(struct worker *worker) {
free(worker);
}
-static void worker_list_cleanup(struct udev *udev) {
- struct udev_list_node *loop, *tmp;
-
- udev_list_node_foreach_safe(loop, tmp, &worker_list) {
- struct worker *worker = node_to_worker(loop);
+static void workers_free(void) {
+ struct worker *worker;
+ Iterator i;
+ HASHMAP_FOREACH(worker, workers, i)
worker_free(worker);
- }
+
+ hashmap_free(workers);
+ workers = NULL;
}
static int worker_new(struct worker **ret, struct udev *udev, struct udev_monitor *worker_monitor, pid_t pid) {
- struct worker *worker;
+ _cleanup_free_ struct worker *worker = NULL;
+ int r;
assert(ret);
assert(udev);
@@ -186,10 +189,19 @@ static int worker_new(struct worker **ret, struct udev *udev, struct udev_monito
udev_monitor_disconnect(worker_monitor);
worker->monitor = udev_monitor_ref(worker_monitor);
worker->pid = pid;
- udev_list_node_append(&worker->node, &worker_list);
+
+ r = hashmap_ensure_allocated(&workers, NULL);
+ if (r < 0)
+ return r;
+
+ r = hashmap_put(workers, UINT_TO_PTR(pid), worker);
+ if (r < 0)
+ return r;
+
children++;
*ret = worker;
+ worker = NULL;
return 0;
}
@@ -235,7 +247,7 @@ static void worker_spawn(struct event *event) {
dev = event->dev;
event->dev = NULL;
- worker_list_cleanup(udev);
+ workers_free();
event_queue_cleanup(udev, EVENT_UNDEF);
udev_monitor_unref(monitor);
udev_ctrl_unref(udev_ctrl);
@@ -439,10 +451,10 @@ out:
}
static void event_run(struct event *event) {
- struct udev_list_node *loop;
+ struct worker *worker;
+ Iterator i;
- udev_list_node_foreach(loop, &worker_list) {
- struct worker *worker = node_to_worker(loop);
+ HASHMAP_FOREACH(worker, workers, i) {
ssize_t count;
if (worker->state != WORKER_IDLE)
@@ -498,11 +510,10 @@ static int event_queue_insert(struct udev_device *dev) {
}
static void worker_kill(struct udev *udev) {
- struct udev_list_node *loop;
-
- udev_list_node_foreach(loop, &worker_list) {
- struct worker *worker = node_to_worker(loop);
+ struct worker *worker;
+ Iterator i;
+ HASHMAP_FOREACH(worker, workers, i) {
if (worker->state == WORKER_KILLED)
continue;
@@ -633,8 +644,7 @@ static void worker_returned(int fd_worker) {
struct cmsghdr *cmsg;
ssize_t size;
struct ucred *ucred = NULL;
- struct udev_list_node *loop;
- bool found = false;
+ struct worker *worker;
size = recvmsg(fd_worker, &msghdr, MSG_DONTWAIT);
if (size < 0) {
@@ -661,25 +671,17 @@ static void worker_returned(int fd_worker) {
}
/* lookup worker who sent the signal */
- udev_list_node_foreach(loop, &worker_list) {
- struct worker *worker = node_to_worker(loop);
-
- if (worker->pid != ucred->pid)
- continue;
- else
- found = true;
-
- if (worker->state != WORKER_KILLED)
- worker->state = WORKER_IDLE;
-
- /* worker returned */
- event_free(worker->event);
-
- break;
+ worker = hashmap_get(workers, UINT_TO_PTR(ucred->pid));
+ if (!worker) {
+ log_debug("worker ["PID_FMT"] returned, but is no longer tracked", ucred->pid);
+ continue;
}
- if (!found)
- log_debug("worker ["PID_FMT"] returned, but is no longer tracked", ucred->pid);
+ if (worker->state != WORKER_KILLED)
+ worker->state = WORKER_IDLE;
+
+ /* worker returned */
+ event_free(worker->event);
}
}
@@ -914,57 +916,48 @@ static void handle_signal(struct udev *udev, int signo) {
for (;;) {
pid_t pid;
int status;
- struct udev_list_node *loop, *tmp;
- bool found = false;
+ struct worker *worker;
pid = waitpid(-1, &status, WNOHANG);
if (pid <= 0)
break;
- udev_list_node_foreach_safe(loop, tmp, &worker_list) {
- struct worker *worker = node_to_worker(loop);
+ worker = hashmap_get(workers, UINT_TO_PTR(pid));
+ if (!worker) {
+ log_warning("worker ["PID_FMT"] is unknown, ignoring", pid);
+ continue;
+ }
- if (worker->pid != pid)
- continue;
+ if (WIFEXITED(status)) {
+ if (WEXITSTATUS(status) == 0)
+ log_debug("worker ["PID_FMT"] exited", pid);
else
- found = true;
-
- if (WIFEXITED(status)) {
- if (WEXITSTATUS(status) == 0)
- log_debug("worker ["PID_FMT"] exited", pid);
- else
- log_warning("worker ["PID_FMT"] exited with return code %i", pid, WEXITSTATUS(status));
- } else if (WIFSIGNALED(status)) {
- log_warning("worker ["PID_FMT"] terminated by signal %i (%s)",
- pid, WTERMSIG(status), strsignal(WTERMSIG(status)));
- } else if (WIFSTOPPED(status)) {
- log_info("worker ["PID_FMT"] stopped", pid);
- break;
- } else if (WIFCONTINUED(status)) {
- log_info("worker ["PID_FMT"] continued", pid);
- break;
- } else {
- log_warning("worker ["PID_FMT"] exit with status 0x%04x", pid, status);
- }
+ log_warning("worker ["PID_FMT"] exited with return code %i", pid, WEXITSTATUS(status));
+ } else if (WIFSIGNALED(status)) {
+ log_warning("worker ["PID_FMT"] terminated by signal %i (%s)",
+ pid, WTERMSIG(status), strsignal(WTERMSIG(status)));
+ } else if (WIFSTOPPED(status)) {
+ log_info("worker ["PID_FMT"] stopped", pid);
+ continue;
+ } else if (WIFCONTINUED(status)) {
+ log_info("worker ["PID_FMT"] continued", pid);
+ continue;
+ } else {
+ log_warning("worker ["PID_FMT"] exit with status 0x%04x", pid, status);
+ }
- if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
- if (worker->event) {
- log_error("worker ["PID_FMT"] failed while handling '%s'", pid, worker->event->devpath);
- /* delete state from disk */
- udev_device_delete_db(worker->event->dev);
- udev_device_tag_index(worker->event->dev, NULL, false);
- /* forward kernel event without amending it */
- udev_monitor_send_device(monitor, NULL, worker->event->dev_kernel);
- }
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+ if (worker->event) {
+ log_error("worker ["PID_FMT"] failed while handling '%s'", pid, worker->event->devpath);
+ /* delete state from disk */
+ udev_device_delete_db(worker->event->dev);
+ udev_device_tag_index(worker->event->dev, NULL, false);
+ /* forward kernel event without amending it */
+ udev_monitor_send_device(monitor, NULL, worker->event->dev_kernel);
}
-
- worker_free(worker);
-
- break;
}
- if (!found)
- log_warning("worker ["PID_FMT"] is unknown, ignoring", pid);
+ worker_free(worker);
}
break;
case SIGHUP:
@@ -1338,7 +1331,6 @@ int main(int argc, char *argv[]) {
log_debug("set children_max to %u", arg_children_max);
udev_list_node_init(&event_list);
- udev_list_node_init(&worker_list);
fd_inotify = udev_watch_init(udev);
if (fd_inotify < 0) {
@@ -1442,7 +1434,8 @@ int main(int argc, char *argv[]) {
continue;
if (fdcount == 0) {
- struct udev_list_node *loop;
+ struct worker *worker;
+ Iterator j;
/* timeout */
if (udev_exit) {
@@ -1457,8 +1450,7 @@ int main(int argc, char *argv[]) {
}
/* check for hanging events */
- udev_list_node_foreach(loop, &worker_list) {
- struct worker *worker = node_to_worker(loop);
+ HASHMAP_FOREACH(worker, workers, j) {
struct event *event = worker->event;
usec_t ts;
@@ -1595,7 +1587,7 @@ exit:
exit_daemonize:
if (fd_ep >= 0)
close(fd_ep);
- worker_list_cleanup(udev);
+ workers_free();
event_queue_cleanup(udev, EVENT_UNDEF);
udev_rules_unref(rules);
udev_builtin_exit(udev);
commit c6aa11f200bcdb910563964d4680807bde372a39
Author: Tom Gundersen <teg at jklm.no>
Date: Mon Apr 27 11:43:31 2015 +0200
udevd: worker - drop reference counting
Make the worker context have the same life-span as the worker process. It is created on fork()
and free'd on SIGCHLD.
The change means that we can get worker_returned() for a worker context that is no longer around,
this is not a problem and we can just drop the message. The only use for worker_returned() is to
know to reschedule events to workers that are still around, so if the worker has already exited
it is not important to keep track of. We still print a debug statement in this case to be on the
safe side.
diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 8c81ecf..8a97b4a 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -85,6 +85,7 @@ struct event {
struct udev *udev;
struct udev_device *dev;
struct udev_device *dev_kernel;
+ struct worker *worker;
enum event_state state;
unsigned long long int delaying_seqnum;
unsigned long long int seqnum;
@@ -129,31 +130,32 @@ static inline struct worker *node_to_worker(struct udev_list_node *node) {
return container_of(node, struct worker, node);
}
-static void event_queue_delete(struct event *event) {
+static void event_free(struct event *event) {
+ if (!event)
+ return;
+
udev_list_node_remove(&event->node);
udev_device_unref(event->dev);
udev_device_unref(event->dev_kernel);
+
+ if (event->worker)
+ event->worker->event = NULL;
+
free(event);
}
-static struct worker *worker_ref(struct worker *worker) {
- worker->refcount++;
- return worker;
-}
+static void worker_free(struct worker *worker) {
+ if (!worker)
+ return;
-static void worker_cleanup(struct worker *worker) {
udev_list_node_remove(&worker->node);
udev_monitor_unref(worker->monitor);
udev_unref(worker->udev);
+ event_free(worker->event);
+
children--;
- free(worker);
-}
-static void worker_unref(struct worker *worker) {
- if (!worker || --worker->refcount > 0)
- return;
- log_debug("worker ["PID_FMT"] cleaned up", worker->pid);
- worker_cleanup(worker);
+ free(worker);
}
static void worker_list_cleanup(struct udev *udev) {
@@ -162,7 +164,7 @@ static void worker_list_cleanup(struct udev *udev) {
udev_list_node_foreach_safe(loop, tmp, &worker_list) {
struct worker *worker = node_to_worker(loop);
- worker_cleanup(worker);
+ worker_free(worker);
}
}
@@ -193,12 +195,17 @@ static int worker_new(struct worker **ret, struct udev *udev, struct udev_monito
}
static void worker_attach_event(struct worker *worker, struct event *event) {
+ assert(worker);
+ assert(event);
+ assert(!event->worker);
+ assert(!worker->event);
+
worker->state = WORKER_RUNNING;
worker->event = event;
event->state = EVENT_RUNNING;
event->start_usec = now(CLOCK_MONOTONIC);
event->warned = false;
- worker_ref(worker);
+ event->worker = worker;
}
static void worker_spawn(struct event *event) {
@@ -602,7 +609,7 @@ static void event_queue_cleanup(struct udev *udev, enum event_state match_type)
if (match_type != EVENT_UNDEF && match_type != event->state)
continue;
- event_queue_delete(event);
+ event_free(event);
}
}
@@ -662,19 +669,17 @@ static void worker_returned(int fd_worker) {
else
found = true;
- /* worker returned */
- if (worker->event) {
- event_queue_delete(worker->event);
- worker->event = NULL;
- }
if (worker->state != WORKER_KILLED)
worker->state = WORKER_IDLE;
- worker_unref(worker);
+
+ /* worker returned */
+ event_free(worker->event);
+
break;
}
if (!found)
- log_warning("unknown worker ["PID_FMT"] returned", ucred->pid);
+ log_debug("worker ["PID_FMT"] returned, but is no longer tracked", ucred->pid);
}
}
@@ -944,20 +949,17 @@ static void handle_signal(struct udev *udev, int signo) {
if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
if (worker->event) {
- log_error("worker ["PID_FMT"] failed while handling '%s'",
- pid, worker->event->devpath);
+ log_error("worker ["PID_FMT"] failed while handling '%s'", pid, worker->event->devpath);
/* delete state from disk */
udev_device_delete_db(worker->event->dev);
udev_device_tag_index(worker->event->dev, NULL, false);
/* forward kernel event without amending it */
udev_monitor_send_device(monitor, NULL, worker->event->dev_kernel);
- event_queue_delete(worker->event);
-
- /* drop reference taken for state 'running' */
- worker_unref(worker);
}
}
- worker_unref(worker);
+
+ worker_free(worker);
+
break;
}
More information about the systemd-commits
mailing list