Re: [PATCH 3/4] x86, kdump: Retore crashkernel= to allocate low

Vivek Goyal
2013-04-04T13:42:19Z

On Wed, Apr 03, 2013 at 05:56:00PM -0700, Yinghai Lu wrote:
> On Wed, Apr 3, 2013 at 2:00 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Wed, Apr 03, 2013 at 01:38:56PM -0700, Yinghai Lu wrote:
> >> On Wed, Apr 3, 2013 at 10:47 AM, Vivek Goyal <vgoyal@redhat.com> wrote:
> >> > So what I am saying that all our code is written assuming there is one
> >> > single reserved range. Now if we need to reserve two ranges, then let
> >> > us make it generic to suppoprt multiple ranges instead of hardcoding
> >> > things and assume there can be 2 ranges. That will be a more generic
> >> > solution.
> >>
> >> I don't think we have case that we need to support more than two ranges.
> >>
> >
> > never say never.
>
> One big rang under 4G is working well till second kernel need more than 512M
> on bigger system with more memory.
>
> Now we make the 64bit kernel/initrd etc could stay above 4G, we can have
> enough range above 4G.
>
> The only problem is during that second kernel running it need some ram
> under 4G when iommu can not be used.
>
> So have one big range above 4G, and a small range below 4g like 72M
> is rational in a long run.

That's one use case. You have not addressed the issue of reserving memory
after boot and not finding one contiguous range and finding smaller
contiguous ranges.

>
> >
> > One of the biggest problems is that we are trying to reserve all the
> > memory at system bootup time using kernel command line.
> >
> > Why not reserve memory using some kernel interface after system has booted.
> > That will make life much simpler.
> >
> > Reason being that currently we depend on one big single chunk being
> > available in low memory area. And after boot there is no guarantee
> > we will have that memory.
> >
> > But what if we just reserve enough memory for kernel and initramfs
> > during boot and rest of the memory is reserved from user space. If
> > system has more memory, there are high chances that after boot we will
> > get memory immediately.
>
> Allocate 72M continuous below 4G after system is booted up?
>
> >
> > What if we don't get a single range of memory, but multiple ranges,
> > we should still be able to make use of all those ranges. And I think
> > that is one possible area where multiple ranges could be useful.
>
> swiotlb does not need continuous area?

One chunk could be below 4G during boot. Say 128M in low memory area
which is sufficient to load kernel, initrd and swiotlb. Rest of the
memory reservation could come after boot on need basis and that does
not have to be contiguous.

>
> >
> > So yes, we don't have an immediate use case, but one can always pop
> > up in future as we try to extend the functionality.
>
> I don't think we need to extend it anymore with high/low two range support.
>
> >
> >
> >> We only need to have one big range above 4G, and put second kernel and
> >> initrd in it.
> >> and another low one is only for switotlb or others that will be used by
> >> second kernel.
> >>
> >> >
> >> > So how about this.
> >> >
> >> > - In 3.9, just implement crashkernel=X;high. Don't auto reserve any low
> >> > memory. Support reservation of single range only. It could be either
> >> > high or low.
> >> >
> >> > - Those who are using iommu, they can use crashkernel=X;high. Old code
> >> > can continue to use crashkernel=X and get memory reserved in low
> >> > memory areas.
> >>
> >> That will not handle the case: big system that crashkernel=X;high
> >> and kdump does not work with iommu.
> >
> > If we start supporting multiple ranges properly in 3.10, then it will.
> > And we have never supported it so it is not a regression. Delaying a
> > feature by a realease should be worth it in an attempt to do it
> > properly.
> >
> >>
> >> >
> >> > - In 3.10 add a feature to support multiple crash reserved ranges.
> >>
> >> Again, we only need one high and one low range.
> >> We don't need to support more than two ranges for crash kernel.
> >
> > For your current use case, you need one high and one low currently. But
> > there can always be scenarios where multiple crash ranges are required,
> > like reserving memory from user space.
> >
> > Anyway, if you are adamant on hardcoding this stuff, please don't
> > use crashkernel= space. Continue to use crashkernel_high and/or
> > crashkernel_low options. That way it allows us to extend crashkernel=
> > in the form of crashkernel=X;<range-options>;<option2>;...
> > and support multiple range feature properly down the line and not be
> > bogged down with this new strange semantics.
> >
> > Also docuemnt very clearly that specifying crashkernel_high ignores
> > any of the crashkernel= options.
>
> will change back to crashkernel= override crashkernel_high.
>

[..]
> >
> > Also what happens to reserve memory release interface. IIUC, there
> > is no way to release crashkernel_low memory from user space and
> > only memory reserved by crashkernel_high will be released?
>
> can not decode.

How do we release low memory from user space. Look at
crash_shrink_memory().

>
>
> >
> > Seriously, why are you rushing with this high/low hardcoding. Why
> > not wait for one more release and support multiple ranges properly
> > both in kernel and kexec-tools.
>
> As more then 2 range is not needed.
> These four patches are complete solution.
>
> Thanks
>
> Yinghai