From e4c7b5f5173d92cfc5378c75fa3ab7ef0252f32b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 26 Jan 2021 20:23:12 +0100 Subject: [PATCH] core: split system/user job timeouts and make them configurable Config options are -Ddefault-timeout-sec= and -Ddefault-user-timeout-sec=. Existing -Dupdate-helper-user-timeout= is renamed to -Dupdate-helper-user-timeout-sec= for consistency. All three options take an integer value in seconds. The renaming and type-change of the option is a small compat break, but it's just at compile time and result in a clear error message. I also doubt that anyone was actually using the option. This commit separates the user manager timeouts, but keeps them unchanged at 90 s. The timeout for the user manager is set to 4/3*user-timeout, which means that it is still 120 s. Fedora wants to experiment with lower timeouts, but doing this via a patch would be annoying and more work than necessary. Let's make this easy to configure. --- man/custom-entities.ent.in | 2 ++ man/systemd-system.conf.xml | 34 ++++++++++++++------------------ man/systemd.service.xml | 11 ++++++----- meson.build | 5 ++++- meson_options.txt | 8 ++++++-- src/basic/constants.h | 7 ++++++- src/core/main.c | 8 ++++---- src/core/manager.c | 6 +++--- src/core/manager.h | 4 ++++ src/core/system.conf.in | 6 +++--- src/core/user.conf.in | 5 +++-- src/rpm/systemd-update-helper.in | 10 +++++----- units/user@.service.in | 2 +- 13 files changed, 62 insertions(+), 46 deletions(-) diff --git a/man/custom-entities.ent.in b/man/custom-entities.ent.in index 6d662a380a..0376e0feb9 100644 --- a/man/custom-entities.ent.in +++ b/man/custom-entities.ent.in @@ -15,5 +15,7 @@ + + diff --git a/man/systemd-system.conf.xml b/man/systemd-system.conf.xml index 481fa31e01..82a52e2150 100644 --- a/man/systemd-system.conf.xml +++ b/man/systemd-system.conf.xml @@ -349,25 +349,20 @@ DefaultTimeoutAbortSec= DefaultRestartSec= - Configures the default timeouts for starting, - stopping and aborting of units, as well as the default time to sleep - between automatic restarts of units, as configured per-unit in - TimeoutStartSec=, - TimeoutStopSec=, - TimeoutAbortSec= and - RestartSec= (for services, see + Configures the default timeouts for starting, stopping and aborting of units, as well + as the default time to sleep between automatic restarts of units, as configured per-unit in + TimeoutStartSec=, TimeoutStopSec=, + TimeoutAbortSec= and RestartSec= (for services, see systemd.service5 - for details on the per-unit settings). Disabled by default, when - service with Type=oneshot is used. - For non-service units, - DefaultTimeoutStartSec= sets the default - TimeoutSec= - value. DefaultTimeoutStartSec= and - DefaultTimeoutStopSec= default to - 90s. DefaultTimeoutAbortSec= is not set by default - so that all units fall back to TimeoutStopSec=. - DefaultRestartSec= defaults to - 100ms. + for details on the per-unit settings). For non-service units, + DefaultTimeoutStartSec= sets the default TimeoutSec= value. + + + DefaultTimeoutStartSec= and DefaultTimeoutStopSec= + default to &DEFAULT_TIMEOUT; in the system manager and &DEFAULT_USER_TIMEOUT; in the user manager. + DefaultTimeoutAbortSec= is not set by default so that all units fall back to + TimeoutStopSec=. DefaultRestartSec= defaults to 100 ms. + @@ -378,7 +373,8 @@ and /etc/crypttab (see systemd.mount5, crypttab5). - Defaults to 90s. + Defaults to &DEFAULT_TIMEOUT; in the system manager and &DEFAULT_USER_TIMEOUT; in the user manager. + diff --git a/man/systemd.service.xml b/man/systemd.service.xml index d44c48c7b1..a1a32e8cd9 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -612,11 +612,12 @@ TimeoutStartSec= - Configures the time to wait for start-up. If a daemon service does not signal start-up - completion within the configured time, the service will be considered failed and will be shut down again. The - precise action depends on the TimeoutStartFailureMode= option. Takes a unit-less value in - seconds, or a time span value such as "5min 20s". Pass infinity to disable the timeout logic. - Defaults to DefaultTimeoutStartSec= from the manager configuration file, except when + Configures the time to wait for start-up. If a daemon service does not signal + start-up completion within the configured time, the service will be considered failed and will be + shut down again. The precise action depends on the TimeoutStartFailureMode= + option. Takes a unit-less value in seconds, or a time span value such as "5min 20s". Pass + infinity to disable the timeout logic. Defaults to + DefaultTimeoutStartSec= set in the manager, except when Type=oneshot is used, in which case the timeout is disabled by default (see systemd-system.conf5). diff --git a/meson.build b/meson.build index a856bb2f84..97246a3c6f 100644 --- a/meson.build +++ b/meson.build @@ -294,7 +294,6 @@ conf.set_quoted('USER_TMPFILES_DIR', usertmpfilesdir) conf.set_quoted('UDEVLIBEXECDIR', udevlibexecdir) conf.set_quoted('UDEV_HWDB_DIR', udevhwdbdir) conf.set_quoted('UDEV_RULES_DIR', udevrulesdir) -conf.set_quoted('UPDATE_HELPER_USER_TIMEOUT', get_option('update-helper-user-timeout')) conf.set_quoted('USER_CONFIG_UNIT_DIR', pkgsysconfdir / 'user') conf.set_quoted('USER_DATA_UNIT_DIR', userunitdir) conf.set_quoted('USER_ENV_GENERATOR_DIR', userenvgeneratordir) @@ -310,6 +309,10 @@ conf.set10('MEMORY_ACCOUNTING_DEFAULT', memory_accounting_ conf.set('STATUS_UNIT_FORMAT_DEFAULT', 'STATUS_UNIT_FORMAT_' + status_unit_format_default.to_upper()) conf.set_quoted('STATUS_UNIT_FORMAT_DEFAULT_STR', status_unit_format_default) +conf.set('DEFAULT_TIMEOUT_SEC', get_option('default-timeout-sec')) +conf.set('DEFAULT_USER_TIMEOUT_SEC', get_option('default-user-timeout-sec')) +conf.set('UPDATE_HELPER_USER_TIMEOUT_SEC', get_option('update-helper-user-timeout-sec')) + conf.set10('FIRST_BOOT_FULL_PRESET', get_option('first-boot-full-preset')) ##################################################################### diff --git a/meson_options.txt b/meson_options.txt index 8ce245dbb9..17c88e23a1 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -187,6 +187,10 @@ option('dbussystemservicedir', type : 'string', description : 'D-Bus system service directory') option('dbus-interfaces-dir', type : 'string', description : 'export D-Bus introspection XML as standalone files') +option('default-timeout-sec', type : 'integer', value : 90, + description : 'default timeout for system unit start/stop') +option('default-user-timeout-sec', type : 'integer', value : 90, + description : 'default timeout for user unit start/stop') option('pkgconfigdatadir', type : 'string', value : '', description : 'directory for arch-independent pkg-config files') option('pkgconfiglibdir', type : 'string', value : '', @@ -195,8 +199,8 @@ option('xinitrcdir', type : 'string', value : '', description : 'directory for xinitrc files') option('rpmmacrosdir', type : 'string', value : 'lib/rpm/macros.d', description : 'directory for rpm macros ["no" disables]') -option('update-helper-user-timeout', type : 'string', value : '15s', - description : 'how long to wait for user manager operations') +option('update-helper-user-timeout-sec', type : 'integer', value : 15, + description : 'timeout for user manager package operations') option('pamlibdir', type : 'string', description : 'directory for PAM modules') option('pamconfdir', type : 'string', diff --git a/src/basic/constants.h b/src/basic/constants.h index 4a24ba9c8c..5d68cc6332 100644 --- a/src/basic/constants.h +++ b/src/basic/constants.h @@ -25,8 +25,13 @@ # endif #endif -#define DEFAULT_TIMEOUT_USEC (90*USEC_PER_SEC) #define DEFAULT_RESTART_USEC (100*USEC_PER_MSEC) + +/* Many different things, but also system unit start/stop */ +#define DEFAULT_TIMEOUT_USEC (DEFAULT_TIMEOUT_SEC*USEC_PER_SEC) +/* User unit start/stop */ +#define DEFAULT_USER_TIMEOUT_USEC (DEFAULT_USER_TIMEOUT_SEC*USEC_PER_SEC) +/* Timeout for user confirmation on the console */ #define DEFAULT_CONFIRM_USEC (30*USEC_PER_SEC) /* We use an extra-long timeout for the reload. This is because a reload or reexec means generators are rerun diff --git a/src/core/main.c b/src/core/main.c index a7cdc8d631..867f3c7adc 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2436,11 +2436,11 @@ static void reset_arguments(void) { arg_default_std_output = EXEC_OUTPUT_JOURNAL; arg_default_std_error = EXEC_OUTPUT_INHERIT; arg_default_restart_usec = DEFAULT_RESTART_USEC; - arg_default_timeout_start_usec = DEFAULT_TIMEOUT_USEC; - arg_default_timeout_stop_usec = DEFAULT_TIMEOUT_USEC; - arg_default_timeout_abort_usec = DEFAULT_TIMEOUT_USEC; + arg_default_timeout_start_usec = manager_default_timeout(arg_system); + arg_default_timeout_stop_usec = manager_default_timeout(arg_system); + arg_default_timeout_abort_usec = manager_default_timeout(arg_system); arg_default_timeout_abort_set = false; - arg_default_device_timeout_usec = DEFAULT_TIMEOUT_USEC; + arg_default_device_timeout_usec = manager_default_timeout(arg_system); arg_default_start_limit_interval = DEFAULT_START_LIMIT_INTERVAL; arg_default_start_limit_burst = DEFAULT_START_LIMIT_BURST; arg_runtime_watchdog = 0; diff --git a/src/core/manager.c b/src/core/manager.c index d39e917134..67bab54b9b 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -835,10 +835,10 @@ int manager_new(LookupScope scope, ManagerTestRunFlags test_run_flags, Manager * .default_memory_accounting = MEMORY_ACCOUNTING_DEFAULT, .default_tasks_accounting = true, .default_tasks_max = TASKS_MAX_UNSET, - .default_timeout_start_usec = DEFAULT_TIMEOUT_USEC, - .default_timeout_stop_usec = DEFAULT_TIMEOUT_USEC, + .default_timeout_start_usec = manager_default_timeout(scope == LOOKUP_SCOPE_SYSTEM), + .default_timeout_stop_usec = manager_default_timeout(scope == LOOKUP_SCOPE_SYSTEM), .default_restart_usec = DEFAULT_RESTART_USEC, - .default_device_timeout_usec = DEFAULT_TIMEOUT_USEC, + .default_device_timeout_usec = manager_default_timeout(scope == LOOKUP_SCOPE_SYSTEM), .original_log_level = -1, .original_log_target = _LOG_TARGET_INVALID, diff --git a/src/core/manager.h b/src/core/manager.h index c13de8fe78..e7b594fdd3 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -485,6 +485,10 @@ static inline usec_t manager_default_timeout_abort_usec(Manager *m) { #define MANAGER_IS_TEST_RUN(m) ((m)->test_run_flags != 0) +static inline usec_t manager_default_timeout(bool is_system) { + return is_system ? DEFAULT_TIMEOUT_USEC : DEFAULT_USER_TIMEOUT_USEC; +} + int manager_new(LookupScope scope, ManagerTestRunFlags test_run_flags, Manager **m); Manager* manager_free(Manager *m); DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); diff --git a/src/core/system.conf.in b/src/core/system.conf.in index 531a7694d9..0c27586c46 100644 --- a/src/core/system.conf.in +++ b/src/core/system.conf.in @@ -43,10 +43,10 @@ #DefaultTimerAccuracySec=1min #DefaultStandardOutput=journal #DefaultStandardError=inherit -#DefaultTimeoutStartSec=90s -#DefaultTimeoutStopSec=90s +#DefaultTimeoutStartSec={{DEFAULT_TIMEOUT_SEC}}s +#DefaultTimeoutStopSec={{DEFAULT_TIMEOUT_SEC}}s #DefaultTimeoutAbortSec= -#DefaultDeviceTimeoutSec=90s +#DefaultDeviceTimeoutSec={{DEFAULT_TIMEOUT_SEC}}s #DefaultRestartSec=100ms #DefaultStartLimitIntervalSec=10s #DefaultStartLimitBurst=5 diff --git a/src/core/user.conf.in b/src/core/user.conf.in index b4938942d1..d67650b858 100644 --- a/src/core/user.conf.in +++ b/src/core/user.conf.in @@ -24,9 +24,10 @@ #DefaultTimerAccuracySec=1min #DefaultStandardOutput=inherit #DefaultStandardError=inherit -#DefaultTimeoutStartSec=90s -#DefaultTimeoutStopSec=90s +#DefaultTimeoutStartSec={{DEFAULT_USER_TIMEOUT_SEC}}s +#DefaultTimeoutStopSec={{DEFAULT_USER_TIMEOUT_SEC}}s #DefaultTimeoutAbortSec= +#DefaultDeviceTimeoutSec={{DEFAULT_USER_TIMEOUT_SEC}}s #DefaultRestartSec=100ms #DefaultStartLimitIntervalSec=10s #DefaultStartLimitBurst=5 diff --git a/src/rpm/systemd-update-helper.in b/src/rpm/systemd-update-helper.in index b35d952fab..c623a5ea17 100755 --- a/src/rpm/systemd-update-helper.in +++ b/src/rpm/systemd-update-helper.in @@ -32,7 +32,7 @@ case "$command" in users=$(systemctl list-units 'user@*' --legend=no | sed -n -r 's/.*user@([0-9]+).service.*/\1/p') for user in $users; do - SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT}} \ + SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT_SEC}}s \ systemctl --user -M "$user@" disable --now --no-warn "$@" & done wait @@ -53,7 +53,7 @@ case "$command" in users=$(systemctl list-units 'user@*' --legend=no | sed -n -r 's/.*user@([0-9]+).service.*/\1/p') for user in $users; do for unit in "$@"; do - SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT}} \ + SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT_SEC}}s \ systemctl --user -M "$user@" set-property "$unit" Markers=+needs-restart & done done @@ -89,7 +89,7 @@ case "$command" in if [[ "$command" =~ reexec ]]; then for user in $users; do - SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT}} \ + SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT_SEC}}s \ systemctl --user -M "$user@" daemon-reexec & done wait @@ -97,7 +97,7 @@ case "$command" in if [[ "$command" =~ reload ]]; then for user in $users; do - SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT}} \ + SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT_SEC}}s \ systemctl --user -M "$user@" daemon-reload & done wait @@ -105,7 +105,7 @@ case "$command" in if [[ "$command" =~ restart ]]; then for user in $users; do - SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT}} \ + SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT_SEC}}s \ systemctl --user -M "$user@" reload-or-restart --marked & done wait diff --git a/units/user@.service.in b/units/user@.service.in index 1558764909..5e95efe1d1 100644 --- a/units/user@.service.in +++ b/units/user@.service.in @@ -23,6 +23,6 @@ Slice=user-%i.slice KillMode=mixed Delegate=pids memory cpu TasksMax=infinity -TimeoutStopSec=120s +TimeoutStopSec={{ DEFAULT_USER_TIMEOUT_SEC*4//3 }}s KeyringMode=inherit OOMScoreAdjust=100