On Thu, Jul 26, 2018 at 7:49 AM, Dave Young <dyoung@redhat.com> wrote:
> On 07/26/18 at 01:51am, Bhupesh Sharma wrote:
>> Hello Kairui,
>>
>> Thanks for the patch.
>>
>> Can you please list the changes in v2 (as compared to v1) in the commit message.
>>
>> On Wed, Jul 25, 2018 at 4:25 PM, Kairui Song <kasong@redhat.com> wrote:
>> > Currently, we only rebuilt kdump initramfs on config file change,
>> > fs change, or watchdog related change. This will not cover the case
>> > that hardware changed but fs layout and other configurations still
>> > stays the same, and kdump may fail.
>>
>> Hmm. This is a chicken and egg issue. We can have modules being
>> unloaded (waiting for dependencies or timeouts to be met) or loaded
>> while we executing 'lsmod' via "kdumpctl restart" , then we will miss
>> those modules being unloaded/loaded.
>
> We only handle the current loaded ones when service is starting. setup
> can change after service started, it will be only detected when service
> restarts eg. next reboot.
>
> One way is restart the kdump service for every module load uevents and
> handle it in udev rules, but the side effect is we will have a lot of
> restart and rebuild, it is not acceptable. So we depend on service
> restart to detect the new changes.
Right. So I am not sure we get desired results with the current
approach as well - if we have to wait for the next reboot.
May be we can think of a more comprehensive approach for handling this.
Thanks,
Bhupesh
>> Can we look at ways to determine the same as well or at least warn the
>> user that this is probably not the final/correct representation of the
>> modules being unloaded/loaded when 'kdumpctl restart' is being
>> executed.
>>
>> Thanks,
>> Bhupesh
>>
>> > To cover such case, we can detect and compare loaded kernel modules,
>> > if a hardware change requires the image to be rebuilt, loaded kernel
>> > modules must have changed.
>> >
>> > Starting from commit 7047294 dracut will record loaded kernel modules
>> > when the image is built if hostonly mode is enabled. With this patch,
>> > kdumpctl will compare the recorded value with currently loaded kernel
>> > modules, and rebuild the image on change.
>> >
>> > "kdumpctl start" will be a bit slower, as we have to call lsinitrd one
>> > more time to get the loaded kernel modules list. I measure the time
>> > consumption and we have an overall 0.2s increased loading time.
>> >
>> > Time consumption of command "kdumpctl restart":
>> >
>> > Before:
>> > real 0m0.587s
>> > user 0m0.481s
>> > sys 0m0.102s
>> >
>> > After:
>> > real 0m0.731s
>> > user 0m0.591s
>> > sys 0m0.133s
>> >
>> > Time comsumption of command "kdumpctl restart" with image rebuild:
>> >
>> > Before (force rebuild):
>> > real 0m10.972s
>> > user 0m8.966s
>> > sys 0m1.318s
>> >
>> > After (inserted ~100 new modules):
>> > real 0m11.220s
>> > user 0m9.387s
>> > sys 0m1.337s
>> >
>> > Signed-off-by: Kairui Song <kasong@redhat.com>
>> > ---
>> > kdumpctl | 30 ++++++++++++++++++++++++++++++
>> > 1 file changed, 30 insertions(+)
>> >
>> > diff --git a/kdumpctl b/kdumpctl
>> > index 13b8209..6a01c13 100755
>> > --- a/kdumpctl
>> > +++ b/kdumpctl
>> > @@ -458,6 +458,31 @@ check_wdt_modified()
>> > return 1
>> > }
>> >
>> > +check_kmodules_modified()
>> > +{
>> > + # always sort again to avoid LANG/LC inconsistent problem
>> > + local _old_modules="$(lsinitrd $TARGET_INITRD -f /usr/lib/dracut/loaded-kernel-modules.txt | sort)"
>> > + local _new_modules="$(get_loaded_kernel_modules | sort)"
>> > +
>> > + [[ -z $_old_modules ]] && echo "Warning: Previous loaded kernel module list is absent or empty"
>> > +
>> > + local _added_modules=$(comm -13 <(echo "$_old_modules") <(echo "$_new_modules"))
>> > + local _dropped_modules=$(comm -23 <(echo "$_old_modules") <(echo "$_new_modules"))
>> > +
>> > + if [ "$_old_modules" != "$_new_modules" ]; then
>> > + echo "Detected change(s) of loaded kernel modules list:"
>> > + [[ -n $_added_modules ]] && for _module in $_added_modules; do
>> > + echo " +$_module"
>> > + done
>> > + [[ -n $_dropped_modules ]] && for _module in $_dropped_modules; do
>> > + echo " -$_module"
>> > + done
>> > + return 1
>> > + fi
>> > +
>> > + return 0
>> > +}
>> > +
>> > # returns 0 if system is not modified
>> > # returns 1 if system is modified
>> > # returns 2 if system modification is invalid
>> > @@ -485,6 +510,11 @@ check_system_modified()
>> > return 1
>> > fi
>> >
>> > + check_kmodules_modified
>> > + if [ $? -ne 0 ]; then
>> > + return 1
>> > + fi
>> > +
>> > return 0
>> > }
>> >
>> > --
>> > 2.17.1
>> > _______________________________________________
>> > kexec mailing list -- kexec@lists.fedoraproject.org
>> > To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
>> > Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>> > List Archives: https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org/message/57RUGXBMGD6QWUPC4EVRFUZKLZWYEEBV/
>> _______________________________________________
>> kexec mailing list -- kexec@lists.fedoraproject.org
>> To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
>> Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>> List Archives: https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org/message/7ABIME3QD5EGCBYC572GS6LOKDAL4GR5/
>
> Thanks
> Dave