On Wed, Apr 3, 2013 at 6:18 AM, Vivek Goyal <vgo...@redhat.com> wrote: > 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.
Looks you are chasing wrong direction. Those four patches fixes the regression that Wang and you reported, User don't need to change their kexec-tools and boot command lines kdump still works. We will never can stop user doing crazy thing with their system. > > [..] >> > >> > 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? I want to keep crashkernel=X to the old behavior. If you want to have crashkernel=X to allocate high above 4G, old kexec-tools will not work with new kernel. > > 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. No, that is not right. only when ;high is specified, kernel will try to allocate high above 4G. > >> >> > >> > 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. So keep crashkernel= without high and low to old behavior. > > 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. That is kept. and only last high is honored > > 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 don't think so. old conf that works before still use crashkernel= with high and low. old conf that does not work, could switch to crashkernel=;high/low with new kexec-tools > > I prefer following for 3.9. > > - process only right most crashkernel= option. what is "right most" ? only last crashkernel=X is honored? I restored that already with those four patches. > - implement crashkernel_no_auto_low option to opt out of auto reserved > low memory No, that is ugly. > - 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. The four patches did not do that? > > 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. No it is not. It's *you* want me to change "Crash kernel low" to "Crash kernel". Do we need to drop second patch? So will still keep "Crash kernel low" in /proc/iomem? Thanks Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/