udev: readjust priorities of event sources

Follow-up for 5116190.

Notable changes are
- SIGTERM is the highest among others, to make not udevd queue too
  many events, as we need to serialize them anyway.
- device monitor has the second highest priority, to make 'remove'
  uevents received earlier than IN_IGNORED inotify events. Otherwise,
  after IN_IGNORED is received, if there is no queued event,
  /run/udev/queue file will be removed by the post-event source of the
  inotify event, and 'udevadm settle' or friends may wrongly finish,
  even we will soon queue 'remove' uevents for the device.
  This change should fix the recent instability of TEST-64-UDEV-STORAGE.

For other changes, see the comments in the code.

Author: yuwata

src/udev/udev-manager-ctrl.c

@@ -125,7 +125,7 @@ int manager_start_ctrl(Manager *manager) {
         /* This needs to be after the inotify and uevent handling, to make sure that the ping is send back
          * after fully processing the pending uevents (including the synthetic ones we may create due to
          * inotify events). */
-        r = sd_event_source_set_priority(udev_ctrl_get_event_source(manager->ctrl), SD_EVENT_PRIORITY_IDLE);
+        r = sd_event_source_set_priority(udev_ctrl_get_event_source(manager->ctrl), EVENT_PRIORITY_CONTROL);
         if (r < 0)
                 return log_error_errno(r, "Failed to set IDLE event priority for udev control event source: %m");
 

src/udev/udev-manager.c

@@ -219,7 +219,7 @@ static int manager_reset_kill_workers_timer(Manager *manager) {
                                 USEC_PER_SEC,
                                 on_kill_workers_event,
                                 manager,
-                                SD_EVENT_PRIORITY_NORMAL,
+                                EVENT_PRIORITY_WORKER_TIMER,
                                 "kill-workers-event",
                                 /* force_reset = */ false);
                 if (r < 0)
@@ -450,13 +450,28 @@ static void worker_attach_event(Worker *worker, Event *event) {
         event->state = EVENT_RUNNING;
         event->worker = worker;
 
-        (void) sd_event_add_time_relative(e, &event->timeout_warning_event, CLOCK_MONOTONIC,
-                                          udev_warn_timeout(manager->config.timeout_usec), USEC_PER_SEC,
-                                          on_event_timeout_warning, event);
-
-        (void) sd_event_add_time_relative(e, &event->timeout_event, CLOCK_MONOTONIC,
-                                          manager_kill_worker_timeout(manager), USEC_PER_SEC,
-                                          on_event_timeout, event);
+        (void) event_reset_time_relative(
+                        e,
+                        &event->timeout_warning_event,
+                        CLOCK_MONOTONIC,
+                        udev_warn_timeout(manager->config.timeout_usec),
+                        USEC_PER_SEC,
+                        on_event_timeout_warning,
+                        event,
+                        EVENT_PRIORITY_WORKER_TIMER,
+                        "event-timeout-warn",
+                        /* force_reset = */ true);
+
+        (void) event_reset_time_relative(
+                        e,
+                        &event->timeout_event, CLOCK_MONOTONIC,
+                        manager_kill_worker_timeout(manager),
+                        USEC_PER_SEC,
+                        on_event_timeout,
+                        event,
+                        EVENT_PRIORITY_WORKER_TIMER,
+                        "event-timeout-kill",
+                        /* force_reset = */ true);
 }
 
 static int worker_spawn(Manager *manager, Event *event) {
@@ -740,10 +755,17 @@ static void event_requeue(Event *event) {
         if (event->retry_again_timeout_usec == 0)
                 event->retry_again_timeout_usec = usec_add(now_usec, EVENT_RETRY_TIMEOUT_USEC);
 
-        r = event_reset_time_relative(event->manager->event, &event->retry_event_source,
-                                      CLOCK_MONOTONIC, EVENT_RETRY_INTERVAL_USEC, 0,
-                                      on_event_retry, NULL,
-                                      0, "retry-event", true);
+        r = event_reset_time_relative(
+                        event->manager->event,
+                        &event->retry_event_source,
+                        CLOCK_MONOTONIC,
+                        EVENT_RETRY_INTERVAL_USEC,
+                        /* accuracy = */ 0,
+                        on_event_retry,
+                        /* userdata = */ NULL,
+                        EVENT_PRIORITY_RETRY_EVENT,
+                        "retry-event",
+                        /* force_reset = */ true);
         if (r < 0) {
                 log_device_warning_errno(
                                 dev, r,
@@ -1216,6 +1238,37 @@ static int on_post(sd_event_source *s, void *userdata) {
         return 0;
 }
 
+static int manager_setup_signal(
+                Manager *manager,
+                sd_event *event,
+                int signal,
+                sd_event_signal_handler_t handler,
+                int64_t priority,
+                const char *description) {
+
+        _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL;
+        int r;
+
+        assert(manager);
+        assert(event);
+
+        r = sd_event_add_signal(event, &s, signal | SD_EVENT_SIGNAL_PROCMASK, handler, manager);
+        if (r < 0)
+                return r;
+
+        r = sd_event_source_set_priority(s, priority);
+        if (r < 0)
+                return r;
+
+        (void) sd_event_source_set_description(s, description);
+
+        r = sd_event_source_set_floating(s, true);
+        if (r < 0)
+                return r;
+
+        return 0;
+}
+
 static int manager_setup_event(Manager *manager) {
         _cleanup_(sd_event_unrefp) sd_event *e = NULL;
         int r;
@@ -1229,15 +1282,15 @@ static int manager_setup_event(Manager *manager) {
         if (r < 0)
                 return log_error_errno(r, "Failed to allocate event loop: %m");
 
-        r = sd_event_add_signal(e, /* ret_event_source = */ NULL, SIGINT | SD_EVENT_SIGNAL_PROCMASK, on_sigterm, manager);
+        r = manager_setup_signal(manager, e, SIGINT, on_sigterm, EVENT_PRIORITY_SIGTERM, "sigint-event-source");
         if (r < 0)
                 return log_error_errno(r, "Failed to create SIGINT event source: %m");
 
-        r = sd_event_add_signal(e, /* ret_event_source = */ NULL, SIGTERM | SD_EVENT_SIGNAL_PROCMASK, on_sigterm, manager);
+        r = manager_setup_signal(manager, e, SIGTERM, on_sigterm, EVENT_PRIORITY_SIGTERM, "sigterm-event-source");
         if (r < 0)
                 return log_error_errno(r, "Failed to create SIGTERM event source: %m");
 
-        r = sd_event_add_signal(e, /* ret_event_source = */ NULL, SIGHUP | SD_EVENT_SIGNAL_PROCMASK, on_sighup, manager);
+        r = manager_setup_signal(manager, e, SIGHUP, on_sighup, EVENT_PRIORITY_SIGHUP, "sighup-event-source");
         if (r < 0)
                 return log_error_errno(r, "Failed to create SIGHUP event source: %m");
 

src/udev/udev-manager.h

@@ -15,17 +15,32 @@
 #include "udev-ctrl.h"
 #include "udev-def.h"
 
+/* This should have a higher priority than the device monitor and inotify watch, to make device monitor and
+ * inotify event source stopped as soon as possible when the signal is received. Otherwise, we may continue
+ * receive events that needs to be serialized anyway. */
+#define EVENT_PRIORITY_SIGTERM        (SD_EVENT_PRIORITY_NORMAL - 6)
+/* This must have a higher priority than the inotify event source, to make 'remove' uevent received earlier
+ * than IN_IGNORED inotify event. */
+#define EVENT_PRIORITY_DEVICE_MONITOR (SD_EVENT_PRIORITY_NORMAL - 5)
 /* This must have a higher priority than the worker notification, to make IN_IGNORED event received earlier
  * than notifications about requests of adding/removing inotify watches. */
-#define EVENT_PRIORITY_INOTIFY_WATCH  (SD_EVENT_PRIORITY_NORMAL - 30)
+#define EVENT_PRIORITY_INOTIFY_WATCH  (SD_EVENT_PRIORITY_NORMAL - 4)
 /* This must have a higher priority than the worker SIGCHLD event, to make notifications about completions of
  * processing events received before SIGCHLD. */
-#define EVENT_PRIORITY_WORKER_NOTIFY  (SD_EVENT_PRIORITY_NORMAL - 20)
-/* This should have a higher priority than other events, especially timer events about killing long running
- * worker processes or idle worker processes. */
-#define EVENT_PRIORITY_WORKER_SIGCHLD (SD_EVENT_PRIORITY_NORMAL - 10)
-/* This should have a lower priority to make signal and timer event sources processed earlier. */
-#define EVENT_PRIORITY_DEVICE_MONITOR (SD_EVENT_PRIORITY_NORMAL + 10)
+#define EVENT_PRIORITY_WORKER_NOTIFY  (SD_EVENT_PRIORITY_NORMAL - 3)
+/* This should have a higher priority than timer events about killing long running worker processes or idle
+ * worker processes. */
+#define EVENT_PRIORITY_WORKER_SIGCHLD (SD_EVENT_PRIORITY_NORMAL - 2)
+/* As said in the above, this should have a lower proority than the SIGCHLD event source. */
+#define EVENT_PRIORITY_WORKER_TIMER   (SD_EVENT_PRIORITY_NORMAL - 1)
+/* This should have a lower priority than most event sources, but let's process earlier than varlink and the
+ * legacy control socket. */
+#define EVENT_PRIORITY_SIGHUP         (SD_EVENT_PRIORITY_NORMAL + 1)
+/* Let's not interrupt the service by any user process, even that requires privileges. */
+#define EVENT_PRIORITY_VARLINK        (SD_EVENT_PRIORITY_NORMAL + 2)
+#define EVENT_PRIORITY_CONTROL        (SD_EVENT_PRIORITY_NORMAL + 2)
+/* The event is intended to trigger the post-event source, hence can be the lowest priority. */
+#define EVENT_PRIORITY_RETRY_EVENT    (SD_EVENT_PRIORITY_NORMAL + 3)
 
 typedef struct Event Event;
 typedef struct UdevRules UdevRules;

src/udev/udev-varlink.c

@@ -170,7 +170,7 @@ int manager_start_varlink_server(Manager *manager) {
         /* This needs to be after the inotify and uevent handling, to make sure that the ping is send back
          * after fully processing the pending uevents (including the synthetic ones we may create due to
          * inotify events). */
-        r = sd_varlink_server_attach_event(v, manager->event, SD_EVENT_PRIORITY_IDLE);
+        r = sd_varlink_server_attach_event(v, manager->event, EVENT_PRIORITY_VARLINK);
         if (r < 0)
                 return log_error_errno(r, "Failed to attach Varlink connection to event loop: %m");