Turns out this is indeed not needed, during implementing this patch serial
I forgot to let "kdumpctl reload" check if kdump is loaded or not at first, and
it always reload kdump resources, and this is-system-running check is
added at that time to make it work, then I forgot to remove it after fixed
the kdumpctl reload cmdline. Will remove it to simplify the code.
On Mon, Oct 29, 2018 at 7:11 PM Dave Young <dyoung(a)redhat.com> wrote:
On 10/25/18 at 02:21pm, Kairui Song wrote:
> Previously, kdump will restart / reload for many times on hotplug
> event, especially memory hotplug events. Hotplugged memory may
> generate many udev event as memory are managed and hotplugged in
> small chunks by the kernel.
>
> This results in unnecessary system workload and an actually longer
> delay of kdump reload and the hotplug event, as udev will either
> get blocked or kdumpctl will be waiting for other triggered operation.
>
> To fix this, introduce a kdump-udev-throttler as an agent which will
> be called by udev and merge concurrent kdump restart requests. Tested
> with a Hyper-V VM which is failing due to udev timeout previously,
> no new issues found.
>
> Signed-off-by: Kairui Song <kasong(a)redhat.com>
> ---
> 98-kexec.rules | 2 +-
> kdump-udev-throttler | 54 ++++++++++++++++++++++++++++++++++++++++++++
> kexec-tools.spec | 3 +++
> 3 files changed, 58 insertions(+), 1 deletion(-)
> create mode 100755 kdump-udev-throttler
>
> diff --git a/98-kexec.rules b/98-kexec.rules
> index 62ddd69..2f88c77 100644
> --- a/98-kexec.rules
> +++ b/98-kexec.rules
> @@ -7,6 +7,6 @@ GOTO="kdump_reload_end"
>
> LABEL="kdump_reload"
>
> -RUN+="/usr/bin/systemctl is-active kdump.service &&
/usr/bin/systemd-run --no-block /sbin/kdumpctl reload"
> +RUN+="/usr/bin/systemd-run --no-block
/usr/lib/udev/kdump-udev-throttler"
>
> LABEL="kdump_reload_end"
> diff --git a/kdump-udev-throttler b/kdump-udev-throttler
> new file mode 100755
> index 0000000..9e6bbbf
> --- /dev/null
> +++ b/kdump-udev-throttler
> @@ -0,0 +1,54 @@
> +#!/bin/bash
> +# This util helps to reduce the workload of kdump service restarting
> +# on udev event. When hotplugging memory / CPU, multiple udev
> +# events may be triggered concurrently, and obviously, we don't want
> +# to restart kdump service for each event.
> +
> +# This script will be called by udev, and make sure kdump service is
> +# restart after all events we are watching are settled.
> +
> +# On each call, this script will update try to aquire the $throttle_lock
> +# The first instance acquired the file lock will keep waiting for events
> +# to settle and then reload kdump. Other instances will just exit
> +# In this way, we can make sure kdump service is restarted immediately
> +# and for exactly once after udev events are settled.
> +
> +
> +throttle_lock="/var/lock/kdump-udev-throttle"
> +interval=2
> +
> +# Don't reload kdump service if kdump service is not started by systemd
> +systemctl is-active kdump.service &>/dev/null || exit 0
> +
> +# Don't restart kdump service if kdump is still starting up in bootup stage
> +# CPU and memories generates udev event in bootup stage, but they are not hot
> +# plugged and may result in unneccessary kdump reload
> +if ! systemctl is-system-running &> /dev/null; then
> + [[ "$(systemctl is-active kdump.service)" ==
"activating" ]] && exit 0
when will the state be activating? Only boot phase?
And when will system state be is-system-running?
Maybe it is not necessary since we do not have this check before and it
works well?
> +fi
> +
> +exec 9>$throttle_lock
> +if [ $? -ne 0 ]; then
> + echo "Failed to create the lock file! Fallback to non-throttled kdump
service restart"
> + /bin/kdumpctl reload
> + exit 1
> +fi
> +
> +flock -n 9
> +if [ $? -ne 0 ]; then
> + echo "Throttling kdump restart for concurrent udev event"
> + exit 0
> +fi
> +
> +# Wait for at least 1 second, at most 4 seconds for udev to settle
> +# Idealy we will have a less than 1 second lag between udev events settle
> +# and kdump reload
> +sleep 1 && udevadm settle --timeout 3
> +
> +# Release the lock, /bin/kdumpctl will block and make the process
> +# holding two locks at the same time and we might miss some events
> +exec 9>&-
> +
> +/bin/kdumpctl reload
> +
> +exit 0
> diff --git a/kexec-tools.spec b/kexec-tools.spec
> index 6330534..91d322f 100644
> --- a/kexec-tools.spec
> +++ b/kexec-tools.spec
> @@ -29,6 +29,7 @@ Source24: kdump.sysconfig.ppc64le
> Source25: kdumpctl.8
> Source26: live-image-kdump-howto.txt
> Source27: early-kdump-howto.txt
> +Source28: kdump-udev-throttler
>
> #######################################
> # These are sources for mkdumpramfs
> @@ -171,6 +172,7 @@ install -m 755 %{SOURCE23}
$RPM_BUILD_ROOT%{_prefix}/lib/kdump/kdump-lib-initram
> # For s390x the ELF header is created in the kdump kernel and therefore kexec
> # udev rules are not required
> install -m 644 %{SOURCE14} $RPM_BUILD_ROOT%{_udevrulesdir}/98-kexec.rules
> +install -m 755 %{SOURCE28} $RPM_BUILD_ROOT%{_udevrulesdir}/../kdump-udev-throttler
> %endif
> install -m 644 %{SOURCE15} $RPM_BUILD_ROOT%{_mandir}/man5/kdump.conf.5
> install -m 644 %{SOURCE16} $RPM_BUILD_ROOT%{_unitdir}/kdump.service
> @@ -298,6 +300,7 @@ done
> %config(noreplace,missingok) %{_sysconfdir}/kdump.conf
> %ifnarch s390x
> %config %{_udevrulesdir}
> +%{_udevrulesdir}/../kdump-udev-throttler
> %endif
> %{dracutlibdir}/modules.d/*
> %dir %{_localstatedir}/crash
> --
> 2.17.1
>
Thanks
Dave
--
Best Regards,
Kairui Song