Re: [PATCH 1/2] Watchdog: Load module for only active watchdog in kdump kernel

[Dave Young]

Hi, Pratyush

Thanks for the effort. Let's cc people who may be interested in the problem.

On 02/02/16 at 10:58am, Pratyush Anand wrote:
> Currently we take care to load only iTCO_wdt if this module was loaded in
> primary kernel. This new approach is capitalizing on recent changes
> proposed in the kernel which are following:
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=65...
>  http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=90...
>  http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=33...
>
> Above kernel patches fixes/adds two major aspects:
> It fixes parent of watchdog_device so that
>
> /sys/class/watchdog/watchdogn/device is populated.
> 2) Adds some sysfs device attributes so that we can read different watchdog
> status.
>
> With the above support, now we can find out whether a watchdog is active or
> not. If it is active, we can also find out the driver/module responsible
> for that watchdog device.
>
> Proposed patch uses above kernel support and then loads relevant wdt
> modules in kdump initramfs only for the active device.
>
> Testing:
> It has been tested with kernel-4.5.0-0.rc1.git2.1.fc24 and fc23 user space.
> Added RuntimeWatchdogSec=40s in /etc/systemd/system.conf
> Modified cmdline for crashkernel=256M
> systemctl enable kdump
> [temporarily changed /etc/kdump.conf to stop at dracut shell]
> restart
> echo c > /proc/sysrq-trigger
>
> On dracut shell I can see
> kdump:/# cat /sys/class/watchdog/watchdog0/identity
> iTCO_wdt
> kdump:/# cat /sys/class/watchdog/watchdog0/state
> active
>
> Tested also with correct /etc/kdump.conf and it was able to save the
> vmcore.
>
> Assumption: Both watchdog and kdump daemon are managed by systemd.systemd
> starts watchdog daemon before kdump. If an user changes the watchdog status
> afterwards then he/she will have to execute `kdumpctrl restart`.
>
> Limitations: This patch will be able to recognize an active wdt, only if
> its driver has been written in watchdog-core framework and registered with
> watchdog_class.

I have several thing to make clear:

* Previously we tried to add it as a dracut module in upstream dracut.
https://www.mail-archive.com/[e-mail]/msg03299.html
In dracut there is a simple watchdog module which is used for testing purpose
only.
We have a reliable way to get hostonly live watchdog module based on your
kernel patches, so it might be better to retry to enhance the dracut watchdog
module. Kdump use dracut --hostonly, we can add using wdt module for hostonly
mode. In this way we can add another dracut kernel cmdline like rd.nowdt so
that one can disable wdt functionality in initramfs if he/she want the original
behavior.

* Previously we planned to only enabling wdt when driver initialization can not
stop wdt eg. in 2nd kernel the wdt status is active after insmod. But if we
copy systemd.conf from 1st kernel to initrd, it will by default enable the
wdt. Do you think it is a better way? Maybe it can address the concern from
Prarit?

* What if in 1st kernel one do not use systemd as wdt daemon? Then in 2nd
kernel the behavior will be just like what we planned, insmod, driver stop the
wdt, but still need kick it when driver failed to stop the wdt?
> Signed-off-by: Pratyush Anand
>
> dracut-module-setup.sh | 19 +++++++++++++------
>  kdump-lib.sh           | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+), 6 deletions(-)
>
> diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
> index 4cd7107c4a35..32032304476e 100755
> --- a/dracut-module-setup.sh
> +++ b/dracut-module-setup.sh
> @@ -706,10 +706,17 @@ install() {
>  }
>
> installkernel() {
> wdt=$(lsmod|cut -f1 -d' '|grep "wdt$")
> if [ -n "$wdt" ]; then
>    [ "$wdt" = "iTCO_wdt" ] && instmods lpc_ich &&
>        echo "rd.driver.pre=lpc_ich,iTCO_wdt " >> ${initdir}/etc/cmdline.d/00-wdt.conf
>    instmods $wdt
> fi
> wdtcmdline=$(get_wdt_cmdline)
> if [ "$?" = "0" ]; then
> echo $wdtcmdline >> ${initdir}/etc/cmdline.d/00-wdt.conf
> IFS='=,' read -ra modarr <<< "$wdtcmdline"
> count='1'
> while [ "$count" -lt ${#modarr[@]} ]
> do
> 	instmods ${modarr[$count]}
> 	count=`expr $count + 1`
> done
> fi
> return 0
>
> }
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index 4d3420652b2f..f60909bca047 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -230,3 +230,42 @@ is_hostname()
>      fi
>      echo $1 | grep -q "[a-zA-Z]"
>  }
>
> +get_wdt_cmdline()
> +{
> local wdtcmdline=""
> wdtcls=/sys/class/watchdog
> cd $wdtcls
> for dir in */; do
> cd $dir
> active=`[ -f state ] && cat state`
> if [ "$active" =  "active" ]; then
> 	# device/modalias will return driver of this device
> 	wdtdrv=`cat device/modalias`
> 	# There can be more than one module represnted by same
> 	# modalias. Currently load all of them.
> 	# TODO: Need to find a way to avoid any unwanted module
> 	# represented by modalias
> 	wdtdrv=`modprobe -R $wdtdrv | tr "\n" "," | sed 's/.$//'`
> 	wdtcmdline="rd.driver.pre=$wdtdrv"
> 	# 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="device/..";
> 	while [ -f "$wdtppath/modalias" ]
> 	do
> 		wdtpdrv=`cat $wdtppath/modalias`
> 		wdtpdrv=`modprobe -R $wdtpdrv | tr "\n" "," | sed 's/.$//'`
> 		wdtcmdline="$wdtcmdline,$wdtpdrv"
> 		wdtppath="$wdtppath/.."
> 	done
> 	echo "$wdtcmdline"
> 	return 0
> fi
> cd ..
> done
> echo "$wdtcmdline"
> return 1
> +}
>
> 2.5.0
> _______________________________________________
> kexec mailing list
> [e-mail]
> http://lists.fedoraproject.org/admin/lists/[e-mail]

Thanks
Dave