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