[PATCH v3 0/4] optimize kdump service start time in case of no initramfs rebuild

[Xunlei Pang]

kdump service takes a few seconds to start even no need to rebuild initramfs,
which is a little longer compared to other systemd services, it may affects 
the system boot time a little, we better optimize it as possible as we can.

The main root cause of this is the use of "lsinitrd" which is time-consuming.

This series is mainly reducing the "lsinitrd" uses.

The time data in this series is on Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz.

After this series, kdump service start time in case of no initramfs rebuild
is 1s~2s, it should be good enough considering the service is indeed running
parallelly with others.

Before this patchset:
    $ time kdumpctl start
    kexec: loaded kdump kernel
    Starting kdump: [OK]

    real    0m5.156s
    user    0m5.169s
    sys     0m1.269s

After this patchset:
    $ time kdumpctl start
    kexec: loaded kdump kernel
    Starting kdump: [OK]
    real    0m1.151s
    user    0m1.028s
    sys     0m0.300s

There are still two lsinitrd operations(each consumes around 0.35s) remaining,
we can reconsider it if there are still complaints in the future.

Xunlei Pang (4):
  kdumpctl: remove is_mode_switched()
  kdumpctl: improve check_wdt_modified()
  kdumpctl: update check_dump_fs_modified() to use "lsinitrd -f"
  kdumpctl: improve "while read" time for /etc/kdump.conf

 kdumpctl | 115 +++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 57 insertions(+), 58 deletions(-)

-- 
1.8.3.1

[PATCH v3 1/4] kdumpctl: remove is_mode_switched()

[Xunlei Pang]

handle_mode_switch() can ensure the correct logic, so remove
the needless is_mode_switched(). This helps to save one slow
lsinitrd operation for each boot.

Improved backup_default_initrd() to judge DEFAULT_INITRD.

Signed-off-by: Xunlei Pang <xlpang(a)redhat.com&gt;
---
 kdumpctl | 37 +++++++------------------------------
 1 file changed, 7 insertions(+), 30 deletions(-)

diff --git a/kdumpctl b/kdumpctl
index 129cb6a..2a87bc2 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -311,6 +311,10 @@ check_executable()
 
 backup_default_initrd()
 {
+	if [ ! -f "$DEFAULT_INITRD" ]; then
+		return
+	fi
+
 	if [ ! -e $DEFAULT_INITRD_BAK ]; then
 		echo "Backing up $DEFAULT_INITRD before rebuild."
 		# save checksum to verify before restoring
@@ -690,10 +694,9 @@ check_rebuild()
 		system_modified="1"
 	fi
 
-	if is_mode_switched; then
-		echo -n "Dump mode changed from last boot."; echo
-		handle_mode_switch
-	elif [ $image_time -eq 0 ]; then
+	handle_mode_switch
+
+	if [ $image_time -eq 0 ]; then
 		echo  -n "No kdump initial ramdisk found."; echo
 	elif [ "$force_rebuild" != "0" ]; then
 		echo -n "Force rebuild $TARGET_INITRD"; echo
@@ -853,32 +856,6 @@ propagate_ssh_key()
 	fi
 }
 
-is_mode_switched()
-{
-	local _mod_included
-
-	if [ ! -f $DEFAULT_INITRD ]; then
-		return 1
-	fi
-
-	# find if default initrd has kdumpbase module included.
-	_mod_included=`lsinitrd -m $DEFAULT_INITRD | grep ^kdumpbase$ | wc -l`
-
-	if [ "$DEFAULT_DUMP_MODE" != "fadump" ]; then
-		# If kdumpbase module is included in default initrd,
-		# we must have just switched from fadump mode.
-		if [ "$_mod_included" -eq "1" ]; then
-			return 0
-		fi
-	elif [ "$_mod_included" -eq "0" ]; then
-		# If kdumpbase module is missing in default initrd,
-		# we must have just switched from kdump mode.
-		return 0
-	fi
-
-	return 1
-}
-
 handle_mode_switch()
 {
 	if [ "$DEFAULT_DUMP_MODE" == "fadump" ]; then
-- 
1.8.3.1

[PATCH v3 2/4] kdumpctl: improve check_wdt_modified()

[Xunlei Pang]

Use the logic of dracut 04watchdog/module-setup.sh to check,
then we only need to compare the content of 00-watchdog.conf,
so we can save one operation of lsinitrd.

Signed-off-by: Xunlei Pang <xlpang(a)redhat.com&gt;
---
 kdumpctl | 66 ++++++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 44 insertions(+), 22 deletions(-)

diff --git a/kdumpctl b/kdumpctl
index 2a87bc2..b24e164 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -565,36 +565,58 @@ check_dump_fs_modified()
 
 check_wdt_modified()
 {
+	local -A _drivers
+	local _alldrivers _active _wdtdrv _wdtppath _dir
+	local wd_old wd_new
+
 	is_wdt_mod_omitted
 	[[ $? -eq 0 ]] && return 0
 	[[ -d /sys/class/watchdog/ ]] || return 0
 
-	for dir in /sys/class/watchdog/*; do
-		[[ -d "$dir" ]] || continue
-		[[ -f "$dir/state" ]] || continue
-		wdtdrv=$(< "$dir/device/modalias")
-		wdtdrv=$(modinfo $wdtdrv | grep filename | awk -F"kernel/" '{print
$2}')
-		active=$(< "$dir/state")
-		# rebuild when:
-		# module for this watchdog is not found and watchdog is active
-		# module for this watchdog is found and watchdog is inactive
-		lsinitrd $TARGET_INITRD | grep $wdtdrv &> /dev/null
-		if [ $? -ne 0 ]; then
-			[[ "$active" =  "active" ]] && return 1
-		else
-			[[ "$active" =  "inactive" ]] && return 1
+	# Copied logic from dracut 04watchdog/module-setup.sh::installkernel()
+	for _dir in /sys/class/watchdog/*; do
+		[[ -d "$_dir" ]] || continue
+		[[ -f "$_dir/state" ]] || continue
+		_active=$(< "$_dir/state")
+		[[ "$_active" =  "active" ]] || continue
+		# device/modalias will return driver of this device
+		_wdtdrv=$(< "$_dir/device/modalias")
+		# There can be more than one module represented by same
+		# modalias. Currently load all of them.
+		# TODO: Need to find a way to avoid any unwanted module
+		# represented by modalias
+		_wdtdrv=$(modprobe --set-version "$kdump_kver" -R $_wdtdrv 2>/dev/null)
+		if [[ $_wdtdrv ]]; then
+			for i in $_wdtdrv; do
+				_drivers[$i]=1
+			done
 		fi
+		# however in some cases, we also need to check that if there is
+		# a specific driver for the parent bus/device.  In such cases
+		# we also need to enable driver for parent bus/device.
+		_wdtppath=$(readlink -f "$_dir/device")
+		while [[ -d "$_wdtppath" ]] && [[ "$_wdtppath" !=
"/sys" ]]; do
+			_wdtppath=$(readlink -f "$_wdtppath/..")
+			[[ -f "$_wdtppath/modalias" ]] || continue
+
+			_wdtdrv=$(< "$_wdtppath/modalias")
+			_wdtdrv=$(modprobe --set-version "$kdump_kver" -R $_wdtdrv 2>/dev/null)
+			if [[ $_wdtdrv ]]; then
+				for i in $_wdtdrv; do
+					_drivers[$i]=1
+				done
+			fi
+		done
 	done
 
-	# check if watchdog kernel module unloaded.
-	loaded_mods=$(lsinitrd $TARGET_INITRD -f etc/cmdline.d/00-watchdog.conf)
-	[[ -n $loaded_mods ]] && loaded_mods=$(echo $loaded_mods | awk
-F"rd.driver.pre=" '{print $2}' | sed "s/,/ /g")
-	for mod in $loaded_mods ; do
-	       lsmod | grep $mod &> /dev/null
-	       [[ $? != 0 ]] && return 1
-        done
+	# ensure that watchdog module is loaded as early as possible
+	_alldrivers="${!_drivers[*]}"
+	[[ $_alldrivers ]] && wd_new="rd.driver.pre=${_alldrivers// /,}"
+	wd_old=$(lsinitrd $TARGET_INITRD -f etc/cmdline.d/00-watchdog.conf)
 
-	return 0
+	[[ "$wd_old" = "$wd_new" ]] && return 0
+
+	return 1
 }
 
 # returns 0 if system is not modified
-- 
1.8.3.1

[PATCH v3 3/4] kdumpctl: update check_dump_fs_modified() to use "lsinitrd -f"

[Xunlei Pang]

We use faster "lsinitrd XXX -f usr/lib/dracut/build-parameter.txt"
instead of "lsinitrd XXX | grep "^Arguments:" | head -1", this can
save us around one second.

Signed-off-by: Xunlei Pang <xlpang(a)redhat.com&gt;
---
 kdumpctl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kdumpctl b/kdumpctl
index b24e164..b9d5fe6 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -539,7 +539,7 @@ check_dump_fs_modified()
 		_new_mntpoint="/kdumproot/$(get_mntpoint_from_target $_target)"
 	fi
 
-	_dracut_args=$(lsinitrd $TARGET_INITRD | grep "^Arguments:" | head -1)
+	_dracut_args=$(lsinitrd $TARGET_INITRD -f usr/lib/dracut/build-parameter.txt)
 	if [[ -z "$_dracut_args" ]];then
 		echo "Warning: No dracut arguments found in initrd"
 		return 0
-- 
1.8.3.1

[PATCH v3 4/4] kdumpctl: improve "while read" time for /etc/kdump.conf

[Xunlei Pang]

I found using "cat $KDUMP_CONFIG_FILE|grep -v "^#"|while read ..."
instead of "while read ... do ...; done < $KDUMP_CONFIG_FILE" will
make the script run faster, it saves us nearly half second.

Signed-off-by: Xunlei Pang <xlpang(a)redhat.com&gt;
---
 kdumpctl | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kdumpctl b/kdumpctl
index b9d5fe6..4d6b3e8 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -365,11 +365,11 @@ check_config()
 		return 1
 	}
 
-	while read config_opt config_val; do
+	cat $KDUMP_CONFIG_FILE | grep -v "^#" | while read config_opt config_val; do
 		# remove inline comments after the end of a directive.
 		config_val=$(strip_comments $config_val)
 		case "$config_opt" in
-		\#* | "")
+		"")
 			;;

		raw|ext2|ext3|ext4|minix|btrfs|xfs|nfs|ssh|sshkey|path|core_collector|kdump_post|kdump_pre|extra_bins|extra_modules|default|force_rebuild|force_no_rebuild|dracut_args|fence_kdump_args|fence_kdump_nodes)
 			[ -z "$config_val" ] && {
@@ -386,7 +386,7 @@ check_config()
 			return 1;
 			;;
 		esac
-	done < $KDUMP_CONFIG_FILE
+	done
 
 	check_default_config || return 1
 
@@ -799,7 +799,7 @@ load_kdump()
 
 check_ssh_config()
 {
-	while read config_opt config_val; do
+	cat $KDUMP_CONFIG_FILE | grep -v "^#" | while read config_opt config_val; do
 		# remove inline comments after the end of a directive.
 		config_val=$(strip_comments $config_val)
 		case "$config_opt" in
@@ -820,7 +820,7 @@ check_ssh_config()
 		*)
 			;;
 		esac
-	done < $KDUMP_CONFIG_FILE
+	done
 
 	#make sure they've configured kdump.conf for ssh dumps
 	local SSH_TARGET=`echo -n $DUMP_TARGET | sed -n '/.*@/p'`
-- 
1.8.3.1

[Dave Young]

Hi Xunlei,

On 05/04/17 at 02:56pm, Xunlei Pang wrote:
>  kdump service takes a few seconds to start even no need to rebuild
> initramfs,
>  which is a little longer compared to other systemd services, it may affects 
>  the system boot time a little, we better optimize it as possible as we can.
>  
>  The main root cause of this is the use of "lsinitrd" which is time-consuming.
>  
>  This series is mainly reducing the "lsinitrd" uses.
>  
>  The time data in this series is on Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz.
>  
>  After this series, kdump service start time in case of no initramfs rebuild
>  is 1s~2s, it should be good enough considering the service is indeed running
>  parallelly with others.
>  
>  Before this patchset:
>      $ time kdumpctl start
>      kexec: loaded kdump kernel
>      Starting kdump: [OK]
>  
>      real    0m5.156s
>      user    0m5.169s
>      sys     0m1.269s
>  
>  After this patchset:
>      $ time kdumpctl start
>      kexec: loaded kdump kernel
>      Starting kdump: [OK]
>      real    0m1.151s
>      user    0m1.028s
>      sys     0m0.300s
>  
>  There are still two lsinitrd operations(each consumes around 0.35s) remaining,
>  we can reconsider it if there are still complaints in the future.
>  
>  Xunlei Pang (4):
>    kdumpctl: remove is_mode_switched()
>    kdumpctl: improve check_wdt_modified()
>    kdumpctl: update check_dump_fs_modified() to use "lsinitrd -f"
>    kdumpctl: improve "while read" time for /etc/kdump.conf
>  
>   kdumpctl | 115 +++++++++++++++++++++++++++++++--------------------------------
>   1 file changed, 57 insertions(+), 58 deletions(-)
>  
>  -- 
>  1.8.3.1
>   
Looks good, nice cleanup. Will leave Praryush to review the wdt
functionality changes. otherwise:

Acked-by: Dave Young <dyoung(a)redhat.com&gt;

Thanks
Dave

[Pratyush Anand]

On Friday 05 May 2017 12:38 PM, Dave Young wrote:
>  Hi Xunlei,
>
>  On 05/04/17 at 02:56pm, Xunlei Pang wrote:
> > kdump service takes a few seconds to start even no need to rebuild initramfs,
> > which is a little longer compared to other systemd services, it may affects
> > the system boot time a little, we better optimize it as possible as we can.
> >
> > The main root cause of this is the use of "lsinitrd" which is
> time-consuming.
> >
> > This series is mainly reducing the "lsinitrd" uses.
> >
> > The time data in this series is on Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz.
> >
> > After this series, kdump service start time in case of no initramfs rebuild
> > is 1s~2s, it should be good enough considering the service is indeed running
> > parallelly with others.
> >
> > Before this patchset:
> >     $ time kdumpctl start
> >     kexec: loaded kdump kernel
> >     Starting kdump: [OK]
> >
> >     real    0m5.156s
> >     user    0m5.169s
> >     sys     0m1.269s
> >
> > After this patchset:
> >     $ time kdumpctl start
> >     kexec: loaded kdump kernel
> >     Starting kdump: [OK]
> >     real    0m1.151s
> >     user    0m1.028s
> >     sys     0m0.300s
> >
> > There are still two lsinitrd operations(each consumes around 0.35s) remaining,
> > we can reconsider it if there are still complaints in the future.
> >
> > Xunlei Pang (4):
> >   kdumpctl: remove is_mode_switched()
> >   kdumpctl: improve check_wdt_modified()
> >   kdumpctl: update check_dump_fs_modified() to use "lsinitrd -f"
> >   kdumpctl: improve "while read" time for /etc/kdump.conf
> >
> >  kdumpctl | 115 +++++++++++++++++++++++++++++++--------------------------------
> >  1 file changed, 57 insertions(+), 58 deletions(-)
> >
> > --
> > 1.8.3.1
> >
>
>  Looks good, nice cleanup. Will leave Praryush to review the wdt
>  functionality changes. otherwise:
>
>  Acked-by: Dave Young <dyoung(a)redhat.com&gt; 
Looks fine to me as well. While cleaning upit fixed few corner bugs as well.

Acked-by: Pratyush Anand <panand(a)redhat.com&gt;

[Dave Young]

On 05/05/17 at 01:37pm, Pratyush Anand wrote:
>  
>  
>  On Friday 05 May 2017 12:38 PM, Dave Young wrote:
>  > Hi Xunlei,
>  > 
>  > On 05/04/17 at 02:56pm, Xunlei Pang wrote:
>  > > kdump service takes a few seconds to start even no need to rebuild initramfs,
>  > > which is a little longer compared to other systemd services, it may affects
>  > > the system boot time a little, we better optimize it as possible as we can.
>  > > 
>  > > The main root cause of this is the use of "lsinitrd" which is
> time-consuming.
>  > > 
>  > > This series is mainly reducing the "lsinitrd" uses.
>  > > 
>  > > The time data in this series is on Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz.
>  > > 
>  > > After this series, kdump service start time in case of no initramfs rebuild
>  > > is 1s~2s, it should be good enough considering the service is indeed running
>  > > parallelly with others.
>  > > 
>  > > Before this patchset:
>  > >     $ time kdumpctl start
>  > >     kexec: loaded kdump kernel
>  > >     Starting kdump: [OK]
>  > > 
>  > >     real    0m5.156s
>  > >     user    0m5.169s
>  > >     sys     0m1.269s
>  > > 
>  > > After this patchset:
>  > >     $ time kdumpctl start
>  > >     kexec: loaded kdump kernel
>  > >     Starting kdump: [OK]
>  > >     real    0m1.151s
>  > >     user    0m1.028s
>  > >     sys     0m0.300s
>  > > 
>  > > There are still two lsinitrd operations(each consumes around 0.35s) remaining,
>  > > we can reconsider it if there are still complaints in the future.
>  > > 
>  > > Xunlei Pang (4):
>  > >   kdumpctl: remove is_mode_switched()
>  > >   kdumpctl: improve check_wdt_modified()
>  > >   kdumpctl: update check_dump_fs_modified() to use "lsinitrd -f"
>  > >   kdumpctl: improve "while read" time for /etc/kdump.conf
>  > > 
>  > >  kdumpctl | 115
> +++++++++++++++++++++++++++++++--------------------------------
>  > >  1 file changed, 57 insertions(+), 58 deletions(-)
>  > > 
>  > > --
>  > > 1.8.3.1
>  > > 
>  > 
>  > Looks good, nice cleanup. Will leave Praryush to review the wdt
>  > functionality changes. otherwise:
>  > 
>  > Acked-by: Dave Young <dyoung(a)redhat.com&gt;
>  
>  Looks fine to me as well. While cleaning upit fixed few corner bugs as well.
>  
>  Acked-by: Pratyush Anand <panand(a)redhat.com&gt; 
Applied, here is a new build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=19407344

On my laptop, no-rebuild time test result: 
old:
real	0m10.122s
user	0m7.750s
sys	0m5.955s

new:
real	0m2.388s
user	0m1.789s
sys	0m1.246s

Thanks
Dave