Re: [patch 48/91] kernel/crash_core: add crashkernel=auto for vmcore creation

[David Hildenbrand <david@redhat.com>]

On 18.05.21 10:49, Baoquan He wrote:
> On 05/17/21 at 10:22am, David Hildenbrand wrote:
>> On 12.05.21 16:51, Baoquan He wrote:
>>> On 05/11/21 at 07:07pm, David Hildenbrand wrote:
>>>>>> If the way adding default value into kernel config is disliked,
>>>>>> this a) option looks good. We can get value with x% of system RAM, but
>>>>>> clamp it with CRASH_KERNEL_MIN/MAX. The CRASH_KERNEL_MIN/MAX may need be
>>>>>> defined with a default value for different ARCHes. It's very close to
>>>>>> our current implementation, and handling 'auto' in kernel.
>>>>>>
>>>>>> And kernel config provided so that people can tune the MIN/MAX value,
>>>>>> but no need to post patch to do the tuning each time if have to?
>>>>> Maybe I'm missing something, but the whole point is to avoid kernel
>>>>> configuration option at all. If the crashkernel=auto works good for 99% of
>>>>> the cases, there is no need to provide build time configuration along with
>>>>> it. There are plenty of ways users can control crashkernel reservations
>>>>> with the existing 2-4 (depending on architecture) command line options.
>>>>>
>>>>> Simply hard coding a reasonable defaults (e.g.
>>>>> "1G-64G:128M,64G-1T:256M,1T-:512M"), and using these defaults when
>>>>> crashkernel=auto is set would cover the same 99% of users you referred to.
>>>>
>>>> Right, and we can easily allocate a bit more as a safety net temporarily
>>>> when we can actually shrink the area later.
>>>>
>>>>>
>>>>> If we can resize the reservation later during boot this will also address
>>>>> David's concern about the wasted memory.
>>>>>
>>>>
>>>> Yes.
>>>>
>>>>> You mentioned that amount of memory that is required for crash kernel
>>>>> reservation depends on the devices present on the system. Is is possible to
>>>>> detect how much memory is required at late stages of boot?
>>>>
>>>> Here is my thinking:
>>>>
>>>> There seems to be some kind of formula we can roughly use to come up with
>>>> the final crashkernel size. Baoquan for sure knows all the dirty details, I
>>>> assume it's roughly "core kernel + drivers + user space".
>>>>
>>>> In the kernel, we can only come up with "core kernel + drivers" expecting
>>>> that we will run
>>>>
>>>> a) roughly the same kernel
>>>> b) with roughly the same drivers
>>>
>>> As replied to Mike, kernel size is undecided for different kernel with
>>> different configs. We can define a default minimal size to cover kernel
>>> and driver on systems with not many devices, but hardcoding the size
>>> into upstream is not helpful. If the size is big, users will be asked to
>>> check and shrink always. If the size is too small, a new value need be
>>> got and added to cmdline and reboot.
>>>
>>
>> Hi Baoquan, Kairui, Dave,
>>
>> so IIUC now, our "old" kernel cannot actually tell us any reliable
>> "crashkernel area size" because
>>
>> a) it has no idea with which cmdline parameters the crashkernel will be
>>     started with, and these can have a big impact.
>> b) it has no idea which driver will be loaded in the crashkernel.
>> c) It has no idea what will be running in the crashkernel user space.
>>
>>
>> AFAIKS, best we can do without further information is, therefore, use some
>> heuristic to a) allocate some memory early during boot in the kernel and b)
>> later refine our allocation, triggered by user space (-> shrink the
>> crashkernel area).
>>
>> I dislike calling a) "auto". It provides a default based on some heuristic
>> (boot memory size), and that default might be very unfortunate in some
>> scenarios (-> waste memory).
>>
>> While we could discuss calling the current approach ( a)
>> )"crashkernel=default", whereby the default is encoded at compile time as
>> determined by a distributor, I still still quite don't like it because it
>> feels like this is not necessary. We have a way to pass something like that
>> via the cmdline, so it's just a matter of properly using that feature from
>> user space.
>>
>>
>> AFAIKS, all you want is most probably a more dynamic way to construct a
>> kernel cmdline, with some properties specific to a kernel.
>>
>> Let's assume the following:
>>
>> a) When a distributor ships a kernel, he also ships some kind of defaults
>> file. Let's assume for simplicity
>>
>> /lib/modules/5.11.19-200.fc33.x86_64/defaults.conf
>>
>> The file might contain
>>
>> CRASHKERNEL_DEFAULT=WHATEVER
>>
>>
>> b) When generating the cmdline for e.g.,
>> /boot/loader/entries/XXX-5.11.19-200.fc33.x86_64.conf we run some script
>> that consult that file in addition to /etc/default/grub. For example, if the
>> kdump service was installed and /etc/default/grub does not contain
>> "crashkernel=" (except when we encounter "crashkernel=auto" for compat
>> handling), we add "crashkernel=WHATEVER". Of course, we might do more
>> involved stuff based on the current setup, user config, etc.
>>
>>
>> c) When we install the kdump service, all we have to do is re-generate the
>> boot entries AFAIKS. Just like we would when adding "crashkernel=auto" right
>> now.
>>
>>
>> The end result would also allow for having per-kernel defaults and change
>> them on kernel updates. Would require some thought on how to make it fly in
>> user space, how to "ship" the defaults etc.
> 
> Thanks for looking into this, and really appreciate your insight,
> comments and patience.

Thanks for being patient with me :)

> 
> We had a sync in team about various viable solutions the other day,
> and also talked about the similar one as you suggested here since
> it seems to be able to resolve the concerns we have for a replacement
> of crashkernel=auto. We will try these in userspace in our side, hope it
> won't introduce risk and can replace crashkernel=auto perfectly.

Sure, and as I said, if we want to look into shrinking of the 
crashkernel area triggered by user space, I'm happy to help.

-- 
Thanks,

David / dhildenb