Hi all,

I think we only care about the module loaded after the service just started.
kdump server starts after fs, network and remote-fs are ready, I think most of the modules we care about should be
already loaded by that time.

There might be other services try to load new modules which are not guaranteed to finish before kdump service,
maybe we can update kdump's systemd service spec file to start after certain services if we care about the module being loaded.

And if there are modules we don't care being loaded at random order, maybe kdump will rebuild the image unnecessarily, maybe we can
introduce a config option to turn off kernel module checking to avoid such circumstances.

How do you think about this solution?

On Thu, Jul 26, 2018 at 1:39 PM Bhupesh Sharma <bhsharma@redhat.com> wrote:
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


--
Best Regards,
Kairui Song