src/udev/udev-manager.c

@@ -53,14 +53,9 @@ typedef enum EventState {
         EVENT_QUEUED,
         EVENT_RUNNING,
         EVENT_LOCKED,
-        EVENT_PROCESSED,
 } EventState;
 
 typedef struct Event {
-        /* All events that have not been processed (state != EVENT_PROCESSED) are referenced by the Manager.
-         * Additionally, an event may be referenced by events blocked by this event. See event_find_blocker(). */
-        unsigned n_ref;
-
         Manager *manager;
         Worker *worker;
         EventState state;
@@ -81,8 +76,9 @@ typedef struct Event {
         char *whole_disk;
         LIST_FIELDS(Event, same_disk);
 
-        /* The last blocker for this event. This event must not be processed before the blocker is processed. */
+        bool dependencies_built;
-        Event *blocker;
+        Set *blocker_events;
+        Set *blocking_events;
 
         LIST_FIELDS(Event, event);
 } Event;
@@ -105,6 +101,21 @@ typedef struct Worker {
         Event *event;
 } Worker;
 
+static void event_clear_dependencies(Event *event) {
+        assert(event);
+
+        Event *e;
+        while ((e = set_steal_first(event->blocker_events)))
+                assert_se(set_remove(e->blocking_events, event) == event);
+        event->blocker_events = set_free(event->blocker_events);
+
+        while ((e = set_steal_first(event->blocking_events)))
+                assert_se(set_remove(e->blocker_events, event) == event);
+        event->blocking_events = set_free(event->blocking_events);
+
+        event->dependencies_built = false;
+}
+
 static void event_unset_whole_disk(Event *event) {
         Manager *manager = ASSERT_PTR(ASSERT_PTR(event)->manager);
 
@@ -129,10 +140,6 @@ static void event_unset_whole_disk(Event *event) {
         event->whole_disk = mfree(event->whole_disk);
 }
 
-static Event* event_free(Event *event);
-DEFINE_PRIVATE_TRIVIAL_REF_UNREF_FUNC(Event, event, event_free);
-DEFINE_TRIVIAL_CLEANUP_FUNC(Event*, event_unref);
-
 static Event* event_free(Event *event) {
         if (!event)
                 return NULL;
@@ -149,25 +156,14 @@ static Event* event_free(Event *event) {
         if (event->worker)
                 event->worker->event = NULL;
 
-        event_unref(event->blocker);
+        event_clear_dependencies(event);
 
         sd_device_unref(event->dev);
 
         return mfree(event);
 }
 
-static Event* event_enter_processed(Event *event) {
+DEFINE_TRIVIAL_CLEANUP_FUNC(Event*, event_free);
-        if (!event)
-                return NULL;
-
-        if (event->state == EVENT_PROCESSED)
-                return NULL;
-
-        event->state = EVENT_PROCESSED;
-        return event_unref(event);
-}
-
-DEFINE_TRIVIAL_CLEANUP_FUNC(Event*, event_enter_processed);
 
 static Worker* worker_free(Worker *worker) {
         if (!worker)
@@ -180,6 +176,7 @@ static Worker* worker_free(Worker *worker) {
         sd_event_source_unref(worker->timeout_warning_event_source);
         sd_event_source_unref(worker->timeout_kill_event_source);
         pidref_done(&worker->pidref);
+        event_free(worker->event);
 
         return mfree(worker);
 }
@@ -203,8 +200,8 @@ Manager* manager_free(Manager *manager) {
         udev_rules_free(manager->rules);
 
         hashmap_free(manager->workers);
-        LIST_FOREACH(event, event, manager->events)
+        while (manager->events)
-                event_enter_processed(event);
+                event_free(manager->events);
 
         prioq_free(manager->locked_events_by_time);
         hashmap_free(manager->locked_events_by_disk);
@@ -395,85 +392,9 @@ void manager_revert(Manager *manager) {
         manager_kill_workers(manager, SIGTERM);
 }
 
-static int on_worker_timeout_kill(sd_event_source *s, uint64_t usec, void *userdata) {
-        Worker *worker = ASSERT_PTR(userdata);
-        Manager *manager = ASSERT_PTR(worker->manager);
-        Event *event = ASSERT_PTR(worker->event);
-
-        (void) pidref_kill_and_sigcont(&worker->pidref, manager->config.timeout_signal);
-        worker->state = WORKER_KILLED;
-
-        log_device_error(event->dev, "Worker ["PID_FMT"] processing SEQNUM=%"PRIu64" killed.", worker->pidref.pid, event->seqnum);
-        return 0;
-}
-
-static int on_worker_timeout_warning(sd_event_source *s, uint64_t usec, void *userdata) {
-        Worker *worker = ASSERT_PTR(userdata);
-        Event *event = ASSERT_PTR(worker->event);
-
-        log_device_warning(event->dev, "Worker ["PID_FMT"] processing SEQNUM=%"PRIu64" is taking a long time.", worker->pidref.pid, event->seqnum);
-        return 0;
-}
-
-static void worker_attach_event(Worker *worker, Event *event) {
-        Manager *manager = ASSERT_PTR(ASSERT_PTR(worker)->manager);
-
-        assert(event);
-        assert(event->state == EVENT_QUEUED);
-        assert(!event->worker);
-        assert(IN_SET(worker->state, WORKER_UNDEF, WORKER_IDLE));
-        assert(!worker->event);
-
-        worker->state = WORKER_RUNNING;
-        worker->event = event;
-        event->state = EVENT_RUNNING;
-        event->worker = worker;
-
-        (void) event_reset_time_relative(
-                        manager->event,
-                        &worker->timeout_warning_event_source,
-                        CLOCK_MONOTONIC,
-                        udev_warn_timeout(manager->config.timeout_usec),
-                        USEC_PER_SEC,
-                        on_worker_timeout_warning,
-                        worker,
-                        EVENT_PRIORITY_WORKER_TIMER,
-                        "worker-timeout-warn",
-                        /* force_reset= */ true);
-
-        (void) event_reset_time_relative(
-                        manager->event,
-                        &worker->timeout_kill_event_source,
-                        CLOCK_MONOTONIC,
-                        manager_kill_worker_timeout(manager),
-                        USEC_PER_SEC,
-                        on_worker_timeout_kill,
-                        worker,
-                        EVENT_PRIORITY_WORKER_TIMER,
-                        "worker-timeout-kill",
-                        /* force_reset= */ true);
-}
-
-static Event* worker_detach_event(Worker *worker) {
-        assert(worker);
-
-        Event *event = TAKE_PTR(worker->event);
-        if (event)
-                assert_se(TAKE_PTR(event->worker) == worker);
-
-        if (worker->state != WORKER_KILLED)
-                worker->state = WORKER_IDLE;
-
-        (void) event_source_disable(worker->timeout_warning_event_source);
-        (void) event_source_disable(worker->timeout_kill_event_source);
-
-        return event;
-}
-
 static int on_sigchld(sd_event_source *s, const siginfo_t *si, void *userdata) {
         _cleanup_(worker_freep) Worker *worker = ASSERT_PTR(userdata);
-        _cleanup_(event_enter_processedp) Event *event = worker_detach_event(worker);
+        sd_device *dev = worker->event ? ASSERT_PTR(worker->event->dev) : NULL;
-        sd_device *dev = event ? ASSERT_PTR(event->dev) : NULL;
 
         assert(si);
 
@@ -551,6 +472,81 @@ static int worker_new(Worker **ret, Manager *manager, sd_device_monitor *worker_
         return 0;
 }
 
+static int on_worker_timeout_kill(sd_event_source *s, uint64_t usec, void *userdata) {
+        Worker *worker = ASSERT_PTR(userdata);
+        Manager *manager = ASSERT_PTR(worker->manager);
+        Event *event = ASSERT_PTR(worker->event);
+
+        (void) pidref_kill_and_sigcont(&worker->pidref, manager->config.timeout_signal);
+        worker->state = WORKER_KILLED;
+
+        log_device_error(event->dev, "Worker ["PID_FMT"] processing SEQNUM=%"PRIu64" killed.", worker->pidref.pid, event->seqnum);
+        return 0;
+}
+
+static int on_worker_timeout_warning(sd_event_source *s, uint64_t usec, void *userdata) {
+        Worker *worker = ASSERT_PTR(userdata);
+        Event *event = ASSERT_PTR(worker->event);
+
+        log_device_warning(event->dev, "Worker ["PID_FMT"] processing SEQNUM=%"PRIu64" is taking a long time.", worker->pidref.pid, event->seqnum);
+        return 0;
+}
+
+static void worker_attach_event(Worker *worker, Event *event) {
+        Manager *manager = ASSERT_PTR(ASSERT_PTR(worker)->manager);
+
+        assert(event);
+        assert(event->state == EVENT_QUEUED);
+        assert(!event->worker);
+        assert(IN_SET(worker->state, WORKER_UNDEF, WORKER_IDLE));
+        assert(!worker->event);
+
+        worker->state = WORKER_RUNNING;
+        worker->event = event;
+        event->state = EVENT_RUNNING;
+        event->worker = worker;
+
+        (void) event_reset_time_relative(
+                        manager->event,
+                        &worker->timeout_warning_event_source,
+                        CLOCK_MONOTONIC,
+                        udev_warn_timeout(manager->config.timeout_usec),
+                        USEC_PER_SEC,
+                        on_worker_timeout_warning,
+                        worker,
+                        EVENT_PRIORITY_WORKER_TIMER,
+                        "worker-timeout-warn",
+                        /* force_reset= */ true);
+
+        (void) event_reset_time_relative(
+                        manager->event,
+                        &worker->timeout_kill_event_source,
+                        CLOCK_MONOTONIC,
+                        manager_kill_worker_timeout(manager),
+                        USEC_PER_SEC,
+                        on_worker_timeout_kill,
+                        worker,
+                        EVENT_PRIORITY_WORKER_TIMER,
+                        "worker-timeout-kill",
+                        /* force_reset= */ true);
+}
+
+static Event* worker_detach_event(Worker *worker) {
+        assert(worker);
+
+        Event *event = TAKE_PTR(worker->event);
+        if (event)
+                assert_se(TAKE_PTR(event->worker) == worker);
+
+        if (worker->state != WORKER_KILLED)
+                worker->state = WORKER_IDLE;
+
+        (void) event_source_disable(worker->timeout_warning_event_source);
+        (void) event_source_disable(worker->timeout_kill_event_source);
+
+        return event;
+}
+
 static int worker_spawn(Manager *manager, Event *event) {
         int r;
 
@@ -653,21 +649,17 @@ bool devpath_conflict(const char *a, const char *b) {
         return *a == '/' || *b == '/' || *a == *b;
 }
 
-static void event_find_blocker(Event *event) {
+static int event_build_dependencies(Event *event) {
+        int r;
+
         assert(event);
 
         /* lookup event for identical, parent, child device */
 
-        if (event->blocker && event->blocker->state != EVENT_PROCESSED)
+        if (event->dependencies_built)
-                /* Previously found blocker is not processed yet. */
+                return 0;
-                return;
-
-        /* If we have not found blocker yet, or the previously found blocker has been processed, let's find
-         * (another) blocker for this event. */
-        LIST_FOREACH_BACKWARDS(event, e, (event->blocker ?: event)->event_prev) {
-                if (e->state == EVENT_PROCESSED)
-                        continue;
 
+        LIST_FOREACH_BACKWARDS(event, e, event->event_prev) {
                 if (!streq_ptr(event->id, e->id) &&
                     !devpath_conflict(event->devpath, e->devpath) &&
                     !devpath_conflict(event->devpath, e->devpath_old) &&
@@ -675,16 +667,22 @@ static void event_find_blocker(Event *event) {
                     !(event->devnode && streq_ptr(event->devnode, e->devnode)))
                         continue;
 
+                r = set_ensure_put(&event->blocker_events, NULL, e);
+                if (r < 0)
+                        return r;
+
+                r = set_ensure_put(&e->blocking_events, NULL, event);
+                if (r < 0) {
+                        assert_se(set_remove(event->blocker_events, e) == e);
+                        return r;
+                }
+
                 log_device_debug(event->dev, "SEQNUM=%" PRIu64 " blocked by SEQNUM=%" PRIu64,
                                  event->seqnum, e->seqnum);
-
-                unref_and_replace_full(event->blocker, e, event_ref, event_unref);
-                return;
         }
 
-        /* No new blocker is found, and if set, previously found blocker has been processed. Clear the
+        event->dependencies_built = true;
-         * previous blocker if set. */
+        return 0;
-        event->blocker = event_unref(event->blocker);
 }
 
 static bool manager_can_process_event(Manager *manager) {
@@ -741,10 +739,14 @@ static int event_queue_start(Manager *manager) {
                 if (event->state != EVENT_QUEUED)
                         continue;
 
-                event_find_blocker(event);
+                r = event_build_dependencies(event);
+                if (r < 0)
+                        log_device_warning_errno(event->dev, r,
+                                                 "Failed to check dependencies for event (SEQNUM=%"PRIu64", ACTION=%s), ignoring: %m",
+                                                 event->seqnum, strna(device_action_to_string(event->action)));
 
                 /* do not start event if parent or child event is still running or queued */
-                if (event->blocker)
+                if (!set_isempty(event->blocker_events))
                         continue;
 
                 r = event_run(event);
@@ -925,12 +927,11 @@ static int event_queue_insert(Manager *manager, sd_device *dev) {
         if (r < 0 && r != -ENOENT)
                 return r;
 
-        _cleanup_(event_unrefp) Event *event = new(Event, 1);
+        _cleanup_(event_freep) Event *event = new(Event, 1);
         if (!event)
                 return -ENOMEM;
 
         *event = (Event) {
-                .n_ref = 1,
                 .dev = sd_device_ref(dev),
                 .seqnum = seqnum,
                 .action = action,
@@ -942,9 +943,6 @@ static int event_queue_insert(Manager *manager, sd_device *dev) {
                 .locked_event_prioq_index = PRIOQ_IDX_NULL,
         };
 
-        /* The kernel sometimes sends events in a wrong order, and we may receive an event with smaller
-         * SEQNUM after one with larger SEQNUM. To workaround the issue, let's reorder events if necessary. */
-
         Event *prev = NULL;
         LIST_FOREACH_BACKWARDS(event, e, manager->last_event) {
                 if (e->seqnum < event->seqnum) {
@@ -956,9 +954,8 @@ static int event_queue_insert(Manager *manager, sd_device *dev) {
                                                         "The event (SEQNUM=%"PRIu64") has been already queued.",
                                                         event->seqnum);
 
-                /* The inserted event may be a blocker of an already queued event, hence the already found
+                /* Inserting an event in an earlier place may change dependency tree. Let's rebuild it later. */
-                 * blocker may not be the last one. Let's find the last blocker again later. */
+                event_clear_dependencies(e);
-                e->blocker = event_unref(e->blocker);
         }
 
         LIST_INSERT_AFTER(event, manager->events, prev, event);
@@ -1214,7 +1211,7 @@ static int on_worker_notify(sd_event_source *s, int fd, uint32_t revents, void *
                 return 0;
         }
 
-        _cleanup_(event_enter_processedp) Event *event = worker_detach_event(worker);
+        _cleanup_(event_freep) Event *event = worker_detach_event(worker);
 
         if (strv_contains(l, "TRY_AGAIN=1")) {
                 /* Worker cannot lock the device. */