From 630ca2e18fc63c3e144f11b54faf70228d4d2e92 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 30 Mar 2021 20:07:53 +0200 Subject: ply-device-manager: Don't deactivate renderers from ply_device_manager_free () Don't deactivate renderers from ply_device_manager_free (), ply_device_manager_free () is only ever called with the renderers still active on a "plymouth quit --retain-splash". Since the splash is being retained in this case the renderers should not be deactivated. Normally this does not matter because plymouthd exits almost immediately afterwards and the kernels will close the renderers fds on exit. But with the upcoming plymouthd-drm-escrow binary which keeps the renderers fds open, not deactivate renderers; and thus not dropping DRM master rights does make a difference. Signed-off-by: Hans de Goede --- src/libply-splash-core/ply-device-manager.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c index db843fc7..aed7bac2 100644 --- a/src/libply-splash-core/ply-device-manager.c +++ b/src/libply-splash-core/ply-device-manager.c @@ -189,11 +189,16 @@ free_devices_from_device_path (ply_device_manager_t *manager, ply_hashtable_remove (manager->renderers, (void *) device_path); free (key); - if (manager->renderers_activated) - ply_renderer_deactivate (renderer); + /* + * Close is false when called from ply_device_manager_free (), in this + * case we don't deactivate / close for retain-splash purposes. + */ + if (close) { + if (manager->renderers_activated) + ply_renderer_deactivate (renderer); - if (close) ply_renderer_close (renderer); + } ply_renderer_free (renderer); } -- cgit v1.2.3 From 156ae7437efaf32825d92943577f24914cf56cec Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 29 Mar 2021 11:05:01 +0200 Subject: main: Only mark plymouthd as unkillable when running from the initrd Before this commit plymouthd would always mark itself as "unkillable" by setting "argv[0][0] = '@';" as documented here: https://www.freedesktop.org/wiki/Software/systemd/RootStorageDaemons/ There are 2 problems with this: 1. This causes filesystems to fail to remount read-only in some case, plymouthd may be holding open a deleted file (say an upgraded library). If that happens, then the filesystem won't allow the disk to be remounted read-only, because when plymouth dies, the filesystem will need to do I/O to clean up the removed file from disk. 2. This causes the "gracefully shutdown" of displays which the kernel's i915 driver recently introduced in commit fe0f1e3bfdfe ("drm/i915: Shut down displays gracefully on reboot") to get undone. Because of being "unkillable" plymouthd keeps running and showing the spinner animation to the very end, this results in a drmModeSetCrtc () call after the i915 display driver has turned off the displays. This causes 2 issues: 2.1 This causes the screen to go black for 1-2 seconds and then show the plymouth screen again for 1-2 seconds on poweroff/reboot which looks ugly: https://bugzilla.redhat.com/show_bug.cgi?id=1941329 2.2 This may cause issues with the attached monitors on reboot, since it undoes the gracefull shutdown which the i915 does. Change the code to only set "argv[0][0] = '@';" when run from the initrd at bootup, this solves the 2 mentioned issues and brings the code inline with the above specification which says this should only ever be used for daemons started from the initrd. Note this will cause plymouth to get killed on shutdown, leading to the last couple of text messages of shutdown being shown on shutdown. This will be fixed by the next couple of patches. Related: https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/118 Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1941329 Signed-off-by: Hans de Goede --- src/main.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main.c b/src/main.c index 122cb734..dd4843d5 100644 --- a/src/main.c +++ b/src/main.c @@ -2214,11 +2214,14 @@ main (int argc, } /* Make the first byte in argv be '@' so that we can survive systemd's killing - * spree when going from initrd to /, and so we stay alive all the way until - * the power is killed at shutdown. + * spree when going from initrd to / * http://www.freedesktop.org/wiki/Software/systemd/RootStorageDaemons + * Note ply_file_exists () does not work here because /etc/initrd-release + * is a symlink when using a dracut generated initrd. */ - argv[0][0] = '@'; + if (state.mode == PLY_BOOT_SPLASH_MODE_BOOT_UP && + access ("/etc/initrd-release", F_OK) >= 0) + argv[0][0] = '@'; state.boot_server = start_boot_server (&state); -- cgit v1.2.3 From eb5227b47485183174f98807e15d224ecde25217 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 30 Mar 2021 13:35:04 +0200 Subject: main: Cleanly quit on SIGTERM Now that we are no longer unconditionally opting out of getting terminated, we get send a SIGTERM when transitioning from the rootfs back to the initrd on poweroff/reboot. Catch this SIGTERM and then exit cleanly by calling the on_quit handler, besides exiting cleanly being the right thing to do, this will also allow the boot-splash to go idle, so that it can cleanly finish the end-animation. Signed-off-by: Hans de Goede --- src/main.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/main.c b/src/main.c index dd4843d5..0cee985e 100644 --- a/src/main.c +++ b/src/main.c @@ -2055,6 +2055,16 @@ on_crash (int signum) raise (signum); } +static void +on_term_signal (state_t *state) +{ + bool retain_splash = false; + + ply_trace ("received SIGTERM"); + + on_quit (state, retain_splash, ply_trigger_new (NULL)); +} + static void write_pid_file (const char *filename) { @@ -2223,6 +2233,10 @@ main (int argc, access ("/etc/initrd-release", F_OK) >= 0) argv[0][0] = '@'; + /* Catch SIGTERM for clean shutdown on poweroff/reboot */ + ply_event_loop_watch_signal (state.loop, SIGTERM, + (ply_event_handler_t) on_term_signal, &state); + state.boot_server = start_boot_server (&state); if (state.boot_server == NULL) { -- cgit v1.2.3 From a4bb4c146b8461d872136684575ba15c8ab4c8c8 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 31 Mar 2021 14:58:11 +0200 Subject: main: Dump debug log to plymouth-shutdown-debug.log on shutdown/reboot When working on plymouth I always have "plymouth.debug=stream:/dev/null" on the kernel commandline. This enables tracing without logging anything to the console and causes the entire trace to be logged to /var/log/plymouth-debug.log when plymouth quits. This is very useful for debugging (non crash) issues with plymouth at boot. With the recent "main: Cleanly quit on SIGTERM" change plymouth will now also write a trace log to /var/log/plymouth-debug.log on shutdown/reboot, but this will be overwritten again on boot by the boot log. This commit changes the default debug_buffer_path value from: /var/log/plymouth-debug.log to /var/log/plymouth-shutdown-debug.log when in shutdown or reboot mode so that it does not get overwritten by the debug-log written at boot. Signed-off-by: Hans de Goede --- src/main.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main.c b/src/main.c index 0cee985e..1c5faa6c 100644 --- a/src/main.c +++ b/src/main.c @@ -1875,8 +1875,13 @@ check_verbosity (state_t *state) } if (debug_buffer != NULL) { - if (debug_buffer_path == NULL) - debug_buffer_path = strdup (PLYMOUTH_LOG_DIRECTORY "/plymouth-debug.log"); + if (debug_buffer_path == NULL) { + if (state->mode == PLY_BOOT_SPLASH_MODE_SHUTDOWN || + state->mode == PLY_BOOT_SPLASH_MODE_REBOOT) + debug_buffer_path = strdup (PLYMOUTH_LOG_DIRECTORY "/plymouth-shutdown-debug.log"); + else + debug_buffer_path = strdup (PLYMOUTH_LOG_DIRECTORY "/plymouth-debug.log"); + } ply_logger_add_filter (ply_logger_get_error_default (), (ply_logger_filter_handler_t) -- cgit v1.2.3 From a0c743c76a3436b8db088398b8fee10d902bf9fd Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Mon, 29 Mar 2021 22:22:40 +0200 Subject: main: Add a plymouthd-fd-escrow helper When plymouth receives SIGTERM during shutdown or reboot, we must exit cleanly to avoid keeping files open on the rootfs and to avoid making drmModeSetCrtc () calls after the kms driver's shutdown method has ran. But at the same time we also want the boot-splash to stay up (in its idle form) until the system actually reboots or powers off. So we want to avoid the boot-splash getting replaced by e.g. the text-console. Add a plymouthd-fd-escrow helper which will get forked off when we receive a SIGTERM in reboot/shutdown mode with pixel-displays active. This helper will keep the fds for the pixel-displays open, so that the boot-splash stays up until the end. Changes by Hans de Goede: - Start the escrow helper from main.c instead of from the drm plugin - Rename the helper from plymouthd-drm-escrow to plymouthd-fd-escrow, since it will be used to escrow fbdev fd-s too now - In the child of the fork, continue with quiting normally (letting the bootsplash become idle) instead of exiting directly - Make plymouthd-fd-escrow a normal dynamic binary instead of a static binary, the initrd already contains dynamic binaries so it does not have to be static - Split the changes adding plymouth-switch-root-initramfs.service into a separate patch - Rewrite commit message Signed-off-by: Hans de Goede --- src/Makefile.am | 6 ++++++ src/main.c | 27 +++++++++++++++++++++++++++ src/plymouthd-fd-escrow.c | 18 ++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 src/plymouthd-fd-escrow.c diff --git a/src/Makefile.am b/src/Makefile.am index cbba2be4..ad3655d5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -7,6 +7,7 @@ AM_CPPFLAGS = -I$(top_srcdir) \ -I$(srcdir)/libply-splash-core \ -I$(srcdir) \ -DPLYMOUTH_LOCALE_DIRECTORY=\"$(localedir)\" \ + -DPLYMOUTH_DRM_ESCROW_DIRECTORY=\"$(libexecdir)/plymouth\" \ -DPLYMOUTH_LOG_DIRECTORY=\"$(localstatedir)/log\" \ -DPLYMOUTH_SPOOL_DIRECTORY=\"$(localstatedir)/spool/plymouth\" \ -DPLYMOUTH_TIME_DIRECTORY=\"$(localstatedir)/lib/plymouth/\" \ @@ -31,6 +32,11 @@ plymouthd_SOURCES = \ plugins/splash/details/plugin.c \ main.c +escrowdir = $(libexecdir)/plymouth +escrow_PROGRAMS = plymouthd-fd-escrow + +plymouthd_fd_escrow_SOURCES = plymouthd-fd-escrow.c + plymouthdrundir = $(localstatedir)/run/plymouth plymouthdspooldir = $(localstatedir)/spool/plymouth plymouthdtimedir = $(localstatedir)/lib/plymouth diff --git a/src/main.c b/src/main.c index 1c5faa6c..23319a1c 100644 --- a/src/main.c +++ b/src/main.c @@ -2060,6 +2060,21 @@ on_crash (int signum) raise (signum); } +static void +start_plymouthd_fd_escrow (void) +{ + pid_t pid; + + pid = fork (); + if (pid == 0) { + const char *argv[] = { PLYMOUTH_DRM_ESCROW_DIRECTORY "/plymouthd-fd-escrow", NULL }; + + execve (argv[0], (char * const *) argv, NULL); + ply_trace ("could not launch fd escrow process: %m"); + _exit (1); + } +} + static void on_term_signal (state_t *state) { @@ -2067,6 +2082,18 @@ on_term_signal (state_t *state) ply_trace ("received SIGTERM"); + /* + * On shutdown/reboot with pixel-displays active, start the plymouthd-fd-escrow + * helper to hold on to the pixel-displays fds until the end. + */ + if ((state->mode == PLY_BOOT_SPLASH_MODE_SHUTDOWN || + state->mode == PLY_BOOT_SPLASH_MODE_REBOOT) && + !state->is_inactive && state->boot_splash && + ply_boot_splash_uses_pixel_displays (state->boot_splash)) { + start_plymouthd_fd_escrow (); + retain_splash = true; + } + on_quit (state, retain_splash, ply_trigger_new (NULL)); } diff --git a/src/plymouthd-fd-escrow.c b/src/plymouthd-fd-escrow.c new file mode 100644 index 00000000..89ec5b68 --- /dev/null +++ b/src/plymouthd-fd-escrow.c @@ -0,0 +1,18 @@ +#include +#include + +int +main(int argc, char **argv) +{ + signal (SIGTERM, SIG_IGN); + + /* Make the first byte in argv be '@' so that we can survive systemd's killing + * spree until the power is killed at shutdown. + * http://www.freedesktop.org/wiki/Software/systemd/RootStorageDaemons + */ + argv[0][0] = '@'; + + while (pause()); + + return 0; +} -- cgit v1.2.3 From 2a7755febb2daa92f785abb710aee00d48cb7e3a Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Fri, 17 Jul 2020 16:06:44 -0400 Subject: systemd: Add plymouth-switch-root-initramfs.service to switch back to initramfs on shutdown Add a plymouth-switch-root-initramfs.service, which will call "plymouth update-root-fs --new-root-dir=/run/initramfs" to switch back to the initramfs (when applicable). Systemd will run this service before plymouthd receives the SIGTERM on shutdown, so this will cause the plymouthd-fd-escrow helper to run from the initramfs. This avoids the plymouthd-fd-escrow helper keeping the rootfs busy. Changes by Hans de Goede: - Fix a couple of typos - Add Conflicts=dracut-shutdown.service to plymouth-switch-root-initramfs.service dracut-shutdown.service restores the initramfs when it is _stopped_ use Conflicts to make sure its ExecStop has run before we do - Add a check for switching back to the initramfs to on_newroot () and dump the debug-buffer before the switch (while we still have access to /var/log). - Also add plymouth-switch-root-initramfs.service to kexec.target.wants. kexec.target.wants uses --mode=shutdown, so the plymouthd-fd-escrow helper will run, so we need to switch to the initramfs. Signed-off-by: Hans de Goede --- configure.ac | 1 + scripts/plymouth-populate-initrd.in | 2 ++ src/main.c | 7 ++++ systemd-units/Makefile.am | 37 +++++++++++++++------- .../plymouth-switch-root-initramfs.service.in | 15 +++++++++ 5 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 systemd-units/plymouth-switch-root-initramfs.service.in diff --git a/configure.ac b/configure.ac index a7b92994..9c9a3e13 100644 --- a/configure.ac +++ b/configure.ac @@ -342,6 +342,7 @@ AC_CONFIG_FILES([Makefile po/Makefile.in systemd-units/plymouth-reboot.service systemd-units/plymouth-start.service systemd-units/plymouth-switch-root.service + systemd-units/plymouth-switch-root-initramfs.service systemd-units/systemd-ask-password-plymouth.path systemd-units/systemd-ask-password-plymouth.service systemd-units/Makefile diff --git a/scripts/plymouth-populate-initrd.in b/scripts/plymouth-populate-initrd.in index 616ecc4e..0ff1f124 100755 --- a/scripts/plymouth-populate-initrd.in +++ b/scripts/plymouth-populate-initrd.in @@ -22,6 +22,7 @@ [ -z "$PLYMOUTH_POLICYDIR" ] && PLYMOUTH_POLICYDIR="@PLYMOUTH_POLICY_DIR@" [ -z "$PLYMOUTH_DAEMON_PATH" ] && PLYMOUTH_DAEMON_PATH="@PLYMOUTH_DAEMON_DIR@/plymouthd" [ -z "$PLYMOUTH_CLIENT_PATH" ] && PLYMOUTH_CLIENT_PATH="@PLYMOUTH_CLIENT_DIR@/plymouth" +[ -z "$PLYMOUTH_DRM_ESCROW_PATH" ] && PLYMOUTH_DRM_ESCROW_PATH="@PLYMOUTH_LIBEXECDIR@/plymouth/plymouthd-fd-escrow" [ -z "$SYSTEMD_UNIT_DIR" ] && SYSTEMD_UNIT_DIR="@SYSTEMD_UNIT_DIR@" # Generic substring function. If $2 is in $1, return 0. @@ -416,6 +417,7 @@ ddebug "Running with PLYMOUTH_LDD_PATH=$PLYMOUTH_LDD_PATH" mkdir -p ${INITRDDIR}${PLYMOUTH_DATADIR}/plymouth/themes inst ${PLYMOUTH_DAEMON_PATH} $INITRDDIR inst ${PLYMOUTH_CLIENT_PATH} $INITRDDIR +inst ${PLYMOUTH_DRM_ESCROW_PATH} $INITRDDIR inst ${PLYMOUTH_DATADIR}/plymouth/themes/text/text.plymouth $INITRDDIR inst ${PLYMOUTH_PLUGIN_PATH}/text.so $INITRDDIR inst ${PLYMOUTH_DATADIR}/plymouth/themes/details/details.plymouth $INITRDDIR diff --git a/src/main.c b/src/main.c index 23319a1c..aa6f90b8 100644 --- a/src/main.c +++ b/src/main.c @@ -158,6 +158,7 @@ static void on_quit (state_t *state, static bool sh_is_init (state_t *state); static void cancel_pending_delayed_show (state_t *state); static void prepare_logging (state_t *state); +static void dump_debug_buffer_to_file (void); static void on_session_output (state_t *state, @@ -653,6 +654,12 @@ on_newroot (state_t *state, } ply_trace ("new root mounted at \"%s\", switching to it", root_dir); + + if (!strcmp (root_dir, "/run/initramfs") && debug_buffer != NULL) { + ply_trace ("switching back to initramfs, dumping debug-buffer now"); + dump_debug_buffer_to_file (); + } + chdir (root_dir); chroot ("."); chdir ("/"); diff --git a/systemd-units/Makefile.am b/systemd-units/Makefile.am index b1d843b6..cd2d7be5 100644 --- a/systemd-units/Makefile.am +++ b/systemd-units/Makefile.am @@ -1,5 +1,6 @@ systemd_unit_templates = \ plymouth-switch-root.service.in \ + plymouth-switch-root-initramfs.service.in \ plymouth-start.service.in \ plymouth-read-write.service.in \ plymouth-quit.service.in \ @@ -37,17 +38,25 @@ install-data-hook: $(LN_S) ../plymouth-quit.service && \ $(LN_S) ../plymouth-quit-wait.service) (cd $(DESTDIR)$(SYSTEMD_UNIT_DIR)/reboot.target.wants && \ - rm -f plymouth-reboot.service && \ - $(LN_S) ../plymouth-reboot.service) + rm -f plymouth-reboot.service \ + plymouth-switch-root-initramfs.service && \ + $(LN_S) ../plymouth-reboot.service && \ + $(LN_S) ../plymouth-switch-root-initramfs.service) (cd $(DESTDIR)$(SYSTEMD_UNIT_DIR)/kexec.target.wants && \ - rm -f plymouth-kexec.service && \ - $(LN_S) ../plymouth-kexec.service) + rm -f plymouth-kexec.service \ + plymouth-switch-root-initramfs.service && \ + $(LN_S) ../plymouth-kexec.service && \ + $(LN_S) ../plymouth-switch-root-initramfs.service) (cd $(DESTDIR)$(SYSTEMD_UNIT_DIR)/poweroff.target.wants && \ - rm -f plymouth-poweroff.service && \ - $(LN_S) ../plymouth-poweroff.service) + rm -f plymouth-poweroff.service \ + plymouth-switch-root-initramfs.service && \ + $(LN_S) ../plymouth-poweroff.service && \ + $(LN_S) ../plymouth-switch-root-initramfs.service) (cd $(DESTDIR)$(SYSTEMD_UNIT_DIR)/halt.target.wants && \ - rm -f plymouth-halt.service && \ - $(LN_S) ../plymouth-halt.service) + rm -f plymouth-halt.service \ + plymouth-switch-root-initramfs.service && \ + $(LN_S) ../plymouth-halt.service && \ + $(LN_S) ../plymouth-switch-root-initramfs.service) uninstall-hook: (cd $(DESTDIR)$(SYSTEMD_UNIT_DIR)/initrd-switch-root.target.wants && \ @@ -57,13 +66,17 @@ uninstall-hook: (cd $(DESTDIR)$(SYSTEMD_UNIT_DIR)/multi-user.target.wants && \ rm -f plymouth-quit.service plymouth-quit-wait.service) (cd $(DESTDIR)$(SYSTEMD_UNIT_DIR)/reboot.target.wants && \ - rm -f plymouth-reboot.service) + rm -f plymouth-reboot.service \ + plymouth-switch-root-initramfs.service) (cd $(DESTDIR)$(SYSTEMD_UNIT_DIR)/kexec.target.wants && \ - rm -f plymouth-kexec.service) + rm -f plymouth-kexec.service \ + plymouth-switch-root-initramfs.service) (cd $(DESTDIR)$(SYSTEMD_UNIT_DIR)/poweroff.target.wants && \ - rm -f plymouth-poweroff.service) + rm -f plymouth-poweroff.service \ + plymouth-switch-root-initramfs.service) (cd $(DESTDIR)$(SYSTEMD_UNIT_DIR)/halt.target.wants && \ - rm -f plymouth-halt.service) + rm -f plymouth-halt.service \ + plymouth-switch-root-initramfs.service) rmdir --ignore-fail-on-non-empty \ $(DESTDIR)$(SYSTEMD_UNIT_DIR)/sysinit.target.wants \ $(DESTDIR)$(SYSTEMD_UNIT_DIR)/multi-user.target.wants \ diff --git a/systemd-units/plymouth-switch-root-initramfs.service.in b/systemd-units/plymouth-switch-root-initramfs.service.in new file mode 100644 index 00000000..0610803c --- /dev/null +++ b/systemd-units/plymouth-switch-root-initramfs.service.in @@ -0,0 +1,15 @@ +[Unit] +Description=Tell Plymouth To Jump To initramfs +DefaultDependencies=no +# dracut-shutdown.service restores the initramfs when it is _stopped_ +# use Conflicts to make sure its ExecStop has run before we do +Conflicts=dracut-shutdown.service +After=plymouth-halt.service plymouth-reboot.service plymouth-poweroff.service plymouth-kexec.service dracut-shutdown.service +ConditionPathExists=/run/initramfs/bin/sh + +[Service] +Type=oneshot +RemainAfterExit=yes +ExecStart=-@PLYMOUTH_CLIENT_DIR@/plymouth update-root-fs --new-root-dir=/run/initramfs +Type=oneshot +RemainAfterExit=yes -- cgit v1.2.3