device-manager: use device timeout instead of udev coldplug monitor

https://lists.freedesktop.org/archives/systemd-devel/2015-March/029184.html
As explained there, plymouth watching for coldplug events is not the
behaviour we're looking for.

Replace this with a configurable timeout. We claim DRM devices as soon as
we're aware of them (displaying the splash still subject to ShowDelay),
but legacy framebuffer and text console devices are only claimed after
this new DeviceTimeout.

This avoids an issue where the initramfs finishes (generating a coldplug
event) before udev has informed plymouth of the DRM devices. This was
causing plymouth to activate text mode and ignore the DRM devices appearing
a moment later.

https://bugs.freedesktop.org/show_bug.cgi?id=95356

diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c
index 31a6f62571db833fb3d3e8d249edc69e36db9718..f6da33932ab845b6aea1d278b51bc4a85034fe60 100644 (file)
--- a/src/libply-splash-core/ply-device-manager.c
+++ b/src/libply-splash-core/ply-device-manager.c
@@ -41,6 +41,8 @@
 #define SUBSYSTEM_DRM "drm"
 #define SUBSYSTEM_FRAME_BUFFER "graphics"
 
+static void create_devices_from_udev (ply_device_manager_t *manager);
+
 static void create_devices_for_terminal_and_renderer_type (ply_device_manager_t *manager,
                                                            const char           *device_path,
                                                            ply_terminal_t       *terminal,
@@ -56,9 +58,6 @@ struct _ply_device_manager
         ply_list_t                *text_displays;
         ply_list_t                *pixel_displays;
         struct udev               *udev_context;
-        struct udev_queue         *udev_queue;
-        int                        udev_queue_fd;
-        ply_fd_watch_t            *udev_queue_fd_watch;
         struct udev_monitor       *udev_monitor;
 
         ply_keyboard_added_handler_t         keyboard_added_handler;
@@ -341,18 +340,16 @@ on_udev_event (ply_device_manager_t *manager)
 
         if (strcmp (action, "add") == 0) {
                 const char *subsystem;
-                bool coldplug_complete = manager->udev_queue_fd_watch == NULL;
 
                 subsystem = udev_device_get_subsystem (device);
 
-                if (strcmp (subsystem, SUBSYSTEM_DRM) == 0 ||
-                    coldplug_complete) {
-                        if (coldplug_complete && manager->local_console_managed && manager->local_console_is_text)
+                if (strcmp (subsystem, SUBSYSTEM_DRM) == 0) {
+                        if (manager->local_console_managed && manager->local_console_is_text)
                                 ply_trace ("ignoring since we're already using text splash for local console");
                         else
                                 create_devices_for_udev_device (manager, device);
                 } else {
-                        ply_trace ("ignoring since we only handle subsystem %s devices after coldplug completes", subsystem);
+                        ply_trace ("ignoring since we only handle subsystem %s devices after timeout", subsystem);
                 }
         } else if (strcmp (action, "remove") == 0) {
                 free_devices_for_udev_device (manager, device);
@@ -493,6 +490,10 @@ ply_device_manager_free (ply_device_manager_t *manager)
         if (manager == NULL)
                 return;
 
+        ply_event_loop_stop_watching_for_timeout (manager->loop,
+                                         (ply_event_loop_timeout_handler_t)
+                                         create_devices_from_udev, manager);
+
         ply_event_loop_stop_watching_for_exit (manager->loop,
                                                (ply_event_loop_exit_handler_t)
                                                detach_from_event_loop,
@@ -745,7 +746,7 @@ create_devices_from_udev (ply_device_manager_t *manager)
 {
         bool found_drm_device, found_fb_device;
 
-        ply_trace ("Looking for devices from udev");
+        ply_trace ("Timeout elapsed, looking for devices from udev");
 
         found_drm_device = create_devices_for_subsystem (manager, SUBSYSTEM_DRM);
         found_fb_device = create_devices_for_subsystem (manager, SUBSYSTEM_FRAME_BUFFER);
@@ -769,63 +770,9 @@ create_fallback_devices (ply_device_manager_t *manager)
                                                        PLY_RENDERER_TYPE_AUTO);
 }
 
-static void
-on_udev_queue_changed (ply_device_manager_t *manager)
-{
-        if (!udev_queue_get_queue_is_empty (manager->udev_queue))
-                return;
-
-        ply_trace ("udev coldplug complete");
-        ply_event_loop_stop_watching_fd (manager->loop, manager->udev_queue_fd_watch);
-        manager->udev_queue_fd_watch = NULL;
-        udev_queue_unref (manager->udev_queue);
-
-        close (manager->udev_queue_fd);
-        manager->udev_queue_fd = -1;
-
-        manager->udev_queue = NULL;
-
-        create_devices_from_udev (manager);
-}
-
-static void
-watch_for_coldplug_completion (ply_device_manager_t *manager)
-{
-        int fd;
-        int result;
-
-        manager->udev_queue = udev_queue_new (manager->udev_context);
-
-        if (udev_queue_get_queue_is_empty (manager->udev_queue)) {
-                ply_trace ("udev coldplug completed already ");
-                create_devices_from_udev (manager);
-                return;
-        }
-
-        fd = inotify_init1 (IN_CLOEXEC);
-        result = inotify_add_watch (fd, "/run/udev", IN_MOVED_TO| IN_DELETE);
-
-        if (result < 0) {
-                ply_trace ("could not watch for udev to show up: %m");
-                close (fd);
-
-                create_fallback_devices (manager);
-                return;
-        }
-
-        manager->udev_queue_fd = fd;
-
-        manager->udev_queue_fd_watch = ply_event_loop_watch_fd (manager->loop,
-                                                                fd,
-                                                                PLY_EVENT_LOOP_FD_STATUS_HAS_DATA,
-                                                                (ply_event_handler_t)
-                                                                on_udev_queue_changed,
-                                                                NULL,
-                                                                manager);
-}
-
 void
 ply_device_manager_watch_devices (ply_device_manager_t                *manager,
+                                  double                               device_timeout,
                                   ply_keyboard_added_handler_t         keyboard_added_handler,
                                   ply_keyboard_removed_handler_t       keyboard_removed_handler,
                                   ply_pixel_display_added_handler_t    pixel_display_added_handler,
@@ -858,7 +805,10 @@ ply_device_manager_watch_devices (ply_device_manager_t                *manager,
         }
 
         watch_for_udev_events (manager);
-        watch_for_coldplug_completion (manager);
+        ply_event_loop_watch_for_timeout (manager->loop,
+                                         device_timeout,
+                                         (ply_event_loop_timeout_handler_t)
+                                         create_devices_from_udev, manager);
 }
 
 bool

diff --git a/src/libply-splash-core/ply-device-manager.h b/src/libply-splash-core/ply-device-manager.h
index ace517cd6f8e66bf2dc38bb2a99c6e82525fee0f..058f6e86f333beb60cc59ebea65ae48e160b31ef 100644 (file)
--- a/src/libply-splash-core/ply-device-manager.h
+++ b/src/libply-splash-core/ply-device-manager.h
@@ -46,6 +46,7 @@ typedef void (* ply_text_display_removed_handler_t) (void *, ply_text_display_t
 ply_device_manager_t *ply_device_manager_new (const char                *default_tty,
                                               ply_device_manager_flags_t flags);
 void ply_device_manager_watch_devices (ply_device_manager_t                *manager,
+                                       double                               device_timeout,
                                        ply_keyboard_added_handler_t         keyboard_added_handler,
                                        ply_keyboard_removed_handler_t       keyboard_removed_handler,
                                        ply_pixel_display_added_handler_t    pixel_display_added_handler,

diff --git a/src/main.c b/src/main.c
index 36fc157037153e791d52846255fe0c08dec0c715..899c1230b316ac59a83f71e0df2e60da8244769f 100644 (file)
--- a/src/main.c
+++ b/src/main.c
@@ -107,6 +107,7 @@ typedef struct
 
         double                  start_time;
         double                  splash_delay;
+        double                  device_timeout;
 
         char                    kernel_command_line[PLY_MAX_COMMAND_LINE_SIZE];
         uint32_t                kernel_command_line_is_set : 1;
@@ -293,7 +294,6 @@ load_settings (state_t    *state,
                char      **theme_path)
 {
         ply_key_file_t *key_file = NULL;
-        const char *delay_string;
         bool settings_loaded = false;
         const char *splash_string;
 
@@ -313,6 +313,8 @@ load_settings (state_t    *state,
                   splash_string, splash_string);
 
         if (isnan (state->splash_delay)) {
+                const char *delay_string;
+
                 delay_string = ply_key_file_get_value (key_file, "Daemon", "ShowDelay");
 
                 if (delay_string != NULL) {
@@ -321,6 +323,17 @@ load_settings (state_t    *state,
                 }
         }
 
+        if (isnan (state->device_timeout)) {
+                const char *timeout_string;
+
+                timeout_string = ply_key_file_get_value (key_file, "Daemon", "DeviceTimeout");
+
+                if (timeout_string != NULL) {
+                        state->device_timeout = atof (timeout_string);
+                        ply_trace ("Device timeout is set to %lf", state->device_timeout);
+                }
+        }
+
         settings_loaded = true;
 out:
         ply_key_file_free (key_file);
@@ -1071,6 +1084,7 @@ load_devices (state_t                   *state,
         state->local_console_terminal = ply_device_manager_get_default_terminal (state->device_manager);
 
         ply_device_manager_watch_devices (state->device_manager,
+                                          state->device_timeout,
                                           (ply_keyboard_added_handler_t)
                                           on_keyboard_added,
                                           (ply_keyboard_removed_handler_t)
@@ -2270,6 +2284,7 @@ main (int    argc,
 
         state.progress = ply_progress_new ();
         state.splash_delay = NAN;
+        state.device_timeout = NAN;
 
         ply_progress_load_cache (state.progress,
                                  get_cache_file_for_mode (state.mode));

diff --git a/src/plymouthd.defaults b/src/plymouthd.defaults
index fc48b156c323ad0abcebf9d4e4b780f286493cb8..4d3b6d48c19e1cafdad7912080fb8dfbba69aba8 100644 (file)
--- a/src/plymouthd.defaults
+++ b/src/plymouthd.defaults
@@ -3,3 +3,4 @@
 [Daemon]
 Theme=spinner
 ShowDelay=5
+DeviceTimeout=5