Re: [PATCHv7] x86/kdump: bugfix, make the behavior of crashkernel=X consistent with kaslr

[Dave Young <dyoung@redhat.com>]

> > 
> > Dave Young sent the original post, and I just re-post it with commit log
> 
> If he sent it, he should be the author I guess.

Just drop the line, but can change the credit to Chao Wang since he did
it initially.

But Chao does not work on kexec/kdump any more, and the email address is
outdated as well.

> 
> > improvement as his requirement.
> > http://lists.infradead.org/pipermail/kexec/2017-October/019571.html
> > There was an old discussion below (previously posted by Chao Wang):
> > https://lkml.org/lkml/2013/10/15/601
> 
> All that changelog info doesn't belong in the commit message ...
> 
> > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > Cc: Dave Young <dyoung@redhat.com>
> > Cc: Baoquan He <bhe@redhat.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
> > Cc: yinghai@kernel.org,
> > Cc: vgoyal@redhat.com
> > Cc: Randy Dunlap <rdunlap@infradead.org>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: x86@kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> 
> .... but here.
> 
> > v6 -> v7: commit log improvement
> >  arch/x86/kernel/setup.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index 3d872a5..fa62c81 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -551,6 +551,22 @@ static void __init reserve_crashkernel(void)
> >  						    high ? CRASH_ADDR_HIGH_MAX
> >  							 : CRASH_ADDR_LOW_MAX,
> >  						    crash_size, CRASH_ALIGN);
> > +#ifdef CONFIG_X86_64
> > +		/*
> > +		 * crashkernel=X reserve below 896M fails? Try below 4G
> > +		 */
> > +		if (!high && !crash_base)
> > +			crash_base = memblock_find_in_range(CRASH_ALIGN,
> > +						(1ULL << 32),
> > +						crash_size, CRASH_ALIGN);
> > +		/*
> > +		 * crashkernel=X reserve below 4G fails? Try MAXMEM
> > +		 */
> > +		if (!high && !crash_base)
> > +			crash_base = memblock_find_in_range(CRASH_ALIGN,
> > +						CRASH_ADDR_HIGH_MAX,
> > +						crash_size, CRASH_ALIGN);
> > +#endif
> 
> Ok, so this is silly: we know at which physical address KASLR allocated
> the kernel so why aren't we querying that and seeing if there's enough
> room before it or after it to call memblock_find_in_range() on the
> bigger range?

Baoquan may have comments?

> 
> Also, why is "high" dealt with separately and why isn't the code
> enforcing "high" if the normal reservation fails?
> 

AFAIK, some people prefer to explictly reserve crash memory at high
region even if it is possible to reserve at low area.  May because
<4G memory is limited on large server, they want to leave this for other
use. 

Yinghai or Vivek should know more about the history, probably they can
recall some initial reason.

> The presence of high is requiring from our users to pay attention what
> to use when the kernel can do all that automatically. Looks like a UI
> fail to me.
> 
> And look at all the flavors of crashkernel= :
> 
>         crashkernel=size[KMG][@offset[KMG]]
>         crashkernel=range1:size1[,range2:size2,...][@offset]
>         crashkernel=size[KMG],high
>         crashkernel=size[KMG],low
> 
> We couldn't do one so we made 4?!?!
> 
> What for?
> 
> Nowhere in that help text does it explain why a user would care about
> high or low or range or offset or the planets alignment...
> 
> So what's up?

Good question, still it may be some historical reason, but it is good to
make them clear and rethink about it after long time.

I also want to understand, need dig the log more.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> Good mailing practices for 400: avoid top-posting and trim the reply.

Thanks
Dave