On Fri, 18 Sep 2015 19:39:02 +0800
Minfei Huang <mhuang at redhat.com> wrote:
> On 09/18/15 at 08:03am, Petr Tesarik wrote:
> > Hello,
> >
> > There may be more than one crash kernel regions on x86. However, the kexec
> > syscall checks that target address is within crashk_res boundaries. Looking
> > at the logic in arch/x86/kernel/setup.c, there are only two possible layouts:
> >
> > 1. crashk_res is below 4G, and there is only one region,
> > 2. crashk_res is above 4G, and crashk_low_res is below 4G
> >
> > In either case, kexec-tools must pick the highest region.
> >
> > Currently, kexec-tools picks the largest region. If high reservation is
> > smaller than low, kexec(2) returns -EADDRNOTAVAIL, and kexec prints out
> > this error message:
> >
> > kexec_load failed: Cannot assign requested address
> >
> > Signed-off-by: Petr Tesarik <ptesarik at suse.com>
> > ---
> > kexec/arch/i386/crashdump-x86.c | 12 ++----------
> > 1 file changed, 2 insertions(+), 10 deletions(-)
> >
> > diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
> > index 63959b7..2710a9e 100644
> > --- a/kexec/arch/i386/crashdump-x86.c
> > +++ b/kexec/arch/i386/crashdump-x86.c
> > @@ -1034,16 +1034,8 @@ int get_max_crash_kernel_limit(uint64_t *start, uint64_t *end)
> > if (!crash_reserved_mem_nr)
> > return -1;
> >
> > - for (i = crash_reserved_mem_nr - 1; i >= 0; i--) {
> > - sz = crash_reserved_mem[i].end - crash_reserved_mem[i].start +1;
> > - if (sz <= sz_max)
> > - continue;
> > - sz_max = sz;
> > - idx = i;
> > - }
> > -
> > - *start = crash_reserved_mem[idx].start;
> > - *end = crash_reserved_mem[idx].end;
> > + *start = crash_reserved_mem[crash_reserved_mem_nr - 1].start;
> > + *end = crash_reserved_mem[crash_reserved_mem_nr - 1].end;
> >
> > return 0;
>
> It is not a proper function name for what it does in your patch. How
True. It should be now called something like get_crashk_res_location().
I'm going to change that.
> about get_reserved_mem_limit()? Also it is appreciative, if you can
> comment it more detail.
I'm not sure what would be more detail. Regarding the motivation, I
have already documented why it fails and what are the symptoms of
failure. Regarding the implementation, the code speaks for itself, IMO.
Can you explain what kind of detail you're missing, please?
Regards,
Petr Tesarik
