Hi Eric, On 05/23/18 at 10:53am, Eric W. Biederman wrote: > Dave Young <dyoung@xxxxxxxxxx> 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 _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec