Re: [PATCH] IA64: better calculate PT_LOAD segment size

[Jay Lan <jlan@sgi.com>]

Simon Horman wrote:
> 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@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? 

I think there is still a bug in the kernel.

> If these
> multiple PT_LOAD segments are really one segment,
> why aren't they being fed in as one segment?

I searched a little... My guess is that it was done so that
the program headers provided more information for kexec
and crash? And thus put it back to simulate a boot loader?
I really do not know why it was done. Maybe Vivek or Eric
would know?

The current comparison part of code is basically a no-op.
I think we can simplify the code here.

Regards,
jay

> 
>> ---
>>  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++;
>>  	}
>>  }
> 
> 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec