On Fri, Sep 19, 2008 at 07:17:05PM -0700, Jay Lan wrote:
> This patch combines consecutive PL_LOAD segments into one.
> The end address of the last PL_LOAD segment, calculated by
> adding p_memsz to p_paddr & rounded up to ELF_PAGE_SIZE,
> will be the end address of this loaded_segments[] entry.
>
> This patch fixes the kdump kernel MCA problem caused by under-
> allocation of memory and a "kdump broken on ALtix 350" problem
> reported by Bernhard Walle.
>
> Simon, this patch replaces my previous patch I submitted on the
> underallocation issue.
>
> Signed-off-by: Jay Lan <jlan at sgi.com>
>
The logic-bug portion of this fix I am fine with.
As you pointed out previously & has higher precedence
than += which makes the following bogus:
loaded_segments[loaded_segments_num].end +=
(phdr->p_memsz + ELF_PAGE_SIZE - 1) &
~(ELF_PAGE_SIZE - 1)
The segment merging portion I am less clear about.
Why are there gaps in the first place? If these
multiple PT_LOAD segments are really one segment,
why aren't they being fed in as one segment?
> ---
> kexec/arch/ia64/crashdump-ia64.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> Index: kexec-tools/kexec/arch/ia64/crashdump-ia64.c
> ===================================================================
> --- kexec-tools.orig/kexec/arch/ia64/crashdump-ia64.c 2008-09-19 14:33:07.593344017 -0700
> +++ kexec-tools/kexec/arch/ia64/crashdump-ia64.c 2008-09-19 17:39:03.732928237 -0700
> @@ -86,19 +86,20 @@ static void add_loaded_segments_info(str
> loaded_segments[loaded_segments_num].end =
> loaded_segments[loaded_segments_num].start;
>
> + /* Consolidate consecutive PL_LOAD segments into one.
> + * The end addr of the last PL_LOAD segment, calculated by
> + * adding p_memsz to p_paddr & rounded up to ELF_PAGE_SIZE,
> + * will be the end address of this loaded_segments entry.
> + */
> while (i < ehdr->e_phnum) {
> phdr = &ehdr->e_phdr[i];
> if (phdr->p_type != PT_LOAD)
> break;
> - if (loaded_segments[loaded_segments_num].end !=
> - phdr->p_paddr & ~(ELF_PAGE_SIZE-1))
> - break;
> - loaded_segments[loaded_segments_num].end +=
> - (phdr->p_memsz + ELF_PAGE_SIZE - 1) &
> - ~(ELF_PAGE_SIZE - 1);
> + loaded_segments[loaded_segments_num].end =
> + (phdr->p_paddr + phdr->p_memsz +
> + ELF_PAGE_SIZE - 1) & ~(ELF_PAGE_SIZE - 1);
> i++;
> }
> -
> loaded_segments_num++;
> }
> }
--
Simon Horman
VA Linux Systems Japan K.K., Sydney, Australia Satellite Office
H: www.vergenet.net/~horms/ W: www.valinux.co.jp/en
