On 05/10/21 at 01:56pm, David Hildenbrand wrote: > On 10.05.21 13:44, Dave Young wrote: > > Hi David, > > Hi Dave, > > > On 05/10/21 at 01:01pm, David Hildenbrand wrote: > > [snip] > > > It also bugged me for quite a bit that we don't have a sane way to achieve > > > what we're doing here upstream. It somewhat feels like "this doesn't belong > > > in the kernel and is user policy" but then, the existing kernel support is > > > suboptimal. > > > > > > Maybe reserving some "maybe too big but okayish to boot the system in a sane > > > environment -- e.g., X% of system RAM and at least Y" size first and > > > shrinking it later as triggered by user space early (where we do seem to > > > have a way to pre-calculate things now) might actually be a good direction > > > to look into. > > > > Hmm, that is also an option we considered before. Even for your > > suggestion we still need a kernel option to set the default ratio/value. > > and the ratio/value should be another patch which expands crashkernel > > syntax. > > Right. > > > > > Actually the kconfig help text in this patch is indeed misleading, it is > > not introducing crashkernel=a:b... and no need to explain about the > > crashkernel syntax, the config option is actually just some interface we > > can add any valid crashkernel settings to be used by default. So current > > patch help text describes the default value of crash auto str, instead > > of describes what crash auto str is. > > Right. And I would much rather prefer either > > a) handling "auto" completely in the kernel, not just setting some > questionable default at compile time Thanks for the suggestions. 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? > b) passing it explicitly in via the cmdline > > > > > And crashkernel=auto makes this more flexibly. We can tune the values > > easily when upgrading. But if we pass a fixed value in userspace we > > can not know if the value is set by distribution automatically or by user > > manually thus we can not blindly update it. > > I think there are two different cases: > > > 1. kernel space updates the value later during boot. "crashkernel=auto" > really does the right thing, meaning > > a) allocate something reasonable and safe during early boot > b) update the allocation during late boot when we know what kind of system > we're running on > > Then, we indeed care about "crashkernel=auto" in the kernel and I think it > would be a nice thing to have. The only question is on how to make that a > little configurable, depending on different thingies we might want to run in > the crashkernel (assuming someone doesn't want kdump). > > > 2. user space updates the value later during boot > > IMHO we don't really car who decided on the value as we do the update from > user space. If an admin messes with crashkernel=, the admin can also mess > with kdump not doing any overwrites (e.g., make that configurable, or detect > the overwrite in kdump somehow). > > -- > Thanks, > > David / dhildenb >
