Messages in this thread | | | Date | Wed, 3 Apr 2013 09:18:43 -0400 | From | Vivek Goyal <> | Subject | Re: [PATCH 3/4] x86, kdump: Retore crashkernel= to allocate low |
| |
On Tue, Apr 02, 2013 at 01:36:02PM -0700, Yinghai Lu wrote:
[..] > > You are just describing what your code does. There is no theme or > > justification behind this behavior. There is no uniformity. A user can > > question that so far you used to honor last crashkernel= parameter and > > suddenly in 3.9 that's no more the case. Out of blue crashkernel=X,high is > > overriding crashkernel=X and it is not obivious why. > > Let me repeat again: > we keep crashkernel=X old behavior with old kexec-tools. > crashkernel=X;high is for new kexec-tools that support loading high. > > If the user want to use ,high but still with old kexec-tools, that is > not going to work. > > Can we just keep it separated?
Kernel does not know about old kexec-tools or new kexec-tools. Neither kernel can enforce what command line options are passed by user. So kernel needs to define a clean interface here which is easily understood and is extensible also in future.
[..] > > > > If user wants 128M in low memory, they will just specify > > crashkernel=128M;low > > in the kernel-parameter.txt, already says ;low is need to used with ;high.
But why are we tying ;low to ;high. One should be easily extend crashkernel=X to be able to reserve memory above 4G if specified amount is not available below 4G. In that case also one might want to reserve some low memory?
For that matter crashkernel=range1:size,range2:size syntax should be extendible too to reserve memory above 4G if desired size of memory is not available in low memory.
Now in those cases too, one would like to have 72M of low memory reserved. So ;low shoud not be tied to ;high necessarily.
In fact current code does not care whetehr ;high was specified or not. If memory is reserved above 4G, ;low code will kick in.
> > > > > If they want to control multiple ranges of memory, then that's the feature > > we currently don't support. Currently we support only reserving one range > > of memory. > > > > If you want to support multiple ranges of memory,then do it properly. > > Parse all crashkernel= options, prepare a list of memory to be reserved > > and unreserved, resolve all the conflicts between various options and > > then reserve the memory. But that does not seem to be a requirement at > > this point of time. > > No we does not support multiple ranges, as it will need more changes > in kexec-tools. > > Can we stop here with those four patches? > > Later, we can extend it if it is really needed.
crashkernel= options are already confusing. I think we with this patchset we will just make them even more confusing and future extensions difficult.
We really need to stick to the notion of only one crashkernel= option is accepted and that is last one on command line. And if need be, we need to work on multi range reservation feature where we process and reserve ranges as specified by all crashkernel= parameters on command line.
Creating new combinations where some crashkernel= are preferred over others and some crashkernel= options work with only selected crashkernel= options, is asking for trouble, especially keeping in mind future extensions.
I prefer following for 3.9.
- process only right most crashkernel= option. - implement crashkernel_no_auto_low option to opt out of auto reserved low memory - implement crashkernel=X;high to support high memory reservations.
And now old kexec-tools user can use crashkernel=X while users needing high memory reservation can use crashkernel=X;high.
If you really want to support user defined crashkernel=X;low along with crashkernel=Y;high, that is really a multi range reservation feature and need to be implemented properly instead of coming up with short cuts.
Thanks Vivek
| |