lkml.org 
[lkml]   [2018]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] kdump: add default crashkernel reserve kernel config options
    Hi Eric,
    On 05/23/18 at 10:53am, Eric W. Biederman wrote:
    > Dave Young <dyoung@redhat.com> writes:
    >
    > > [snip]
    > >
    > >> >
    > >> > +config CRASHKERNEL_DEFAULT_THRESHOLD_MB
    > >> > + int "System memory size threshold for kdump memory default reserving"
    > >> > + depends on CRASH_CORE
    > >> > + default 0
    > >> > + help
    > >> > + CRASHKERNEL_DEFAULT_MB is used as default crashkernel value if
    > >> > + the system memory size is equal or bigger than the threshold.
    > >>
    > >> "the threshold" is rather vague. Can it be clarified?
    > >>
    > >> In fact I'm really struggling to understand the logic here....
    > >>
    > >>
    > >> > +config CRASHKERNEL_DEFAULT_MB
    > >> > + int "Default crashkernel memory size reserved for kdump"
    > >> > + depends on CRASH_CORE
    > >> > + default 0
    > >> > + help
    > >> > + This is used as the default kdump reserved memory size in MB.
    > >> > + crashkernel=X kernel cmdline can overwrite this value.
    > >> > +
    > >> > config HAVE_IMA_KEXEC
    > >> > bool
    > >> >
    > >> > @@ -143,6 +144,24 @@ static int __init parse_crashkernel_simp
    > >> > return 0;
    > >> > }
    > >> >
    > >> > +static int __init get_crashkernel_default(unsigned long long system_ram,
    > >> > + unsigned long long *size)
    > >> > +{
    > >> > + unsigned long long sz = CONFIG_CRASHKERNEL_DEFAULT_MB;
    > >> > + unsigned long long thres = CONFIG_CRASHKERNEL_DEFAULT_THRESHOLD_MB;
    > >> > +
    > >> > + thres *= SZ_1M;
    > >> > + sz *= SZ_1M;
    > >> > +
    > >> > + if (sz >= system_ram || system_ram < thres) {
    > >> > + pr_debug("crashkernel default size can not be used.\n");
    > >> > + return -EINVAL;
    > >>
    > >> In other words,
    > >>
    > >> if (system_ram <= CONFIG_CRASHKERNEL_DEFAULT_MB ||
    > >> system_ram < CONFIG_CRASHKERNEL_DEFAULT_THRESHOLD_MB)
    > >> fail;
    > >>
    > >> yes?
    > >>
    > >> How come? What's happening here? Perhaps a (good) explanatory comment
    > >> is needed. And clearer Kconfig text.
    > >>
    > >> All confused :(
    > >
    > > Andrew, I tuned it a bit, removed the check of sz >= system_ram, so if
    > > the size is too large and kernel can not find enough memory it will
    > > still fail in latter code.
    > >
    > > Is below version looks clearer?
    >
    > What is the advantage of providing this in a kconfig option rather
    > than on the kernel command line as we can now?

    It is not a replacement of the cmdline, this can be a supplement to
    the crashkernel command line. For a lot of common use cases if we have
    the auto reservation user just do not need to manually set the cmdline
    for example on a virtual machine and usual setup (except of the
    comlicate storage and very large machines). The crashkernel=auto
    has been used for long time, Red Hat QE tested it on a lot of different
    lab machines and proved it works well. Kdump usually just works so admin
    do little work to enable kdump.

    But the crashkernel=auto implementation has some drawbacks that is it
    is more like embed policy in the code and it is not flexible like a
    config option.

    Thanks
    Dave

    \
     
     \ /
      Last update: 2018-05-24 03:43    [W:3.805 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site