[PATCH] IA64: better calculate PT_LOAD segment size

[Simon Horman]

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