lkml.org 
[lkml]   [2006]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86_64: setup saved_max_pfn correctly (kdump)
On Fri, Nov 03, 2006 at 02:40:26AM +0900, Magnus Damm wrote:
> On 11/2/06, Vivek Goyal <vgoyal@in.ibm.com> wrote:
> >On Thu, Nov 02, 2006 at 10:19:34PM +0900, Magnus Damm wrote:
> >> x86_64: setup saved_max_pfn correctly
> >>
> >> 2.6.19-rc4 has broken CONFIG_CRASH_DUMP support on x86_64. It is
> >impossible
> >> to read out the kernel contents from /proc/vmcore because saved_max_pfn
> >is set
> >> to zero instead of the max_pfn value before the user map is setup.
> >>
> >> This happens because saved_max_pfn is initialized at parse_early_param()
> >time,
> >> and at this time no active regions have been registered. save_max_pfn is
> >setup
> >> from e820_end_of_ram(), more exact find_max_pfn_with_active_regions()
> >which
> >> returns 0 because no regions exist.
> >>
> >> This patch fixes this by registering before and removing after the call
> >> to e820_end_of_ram().
> >>
> >> Signed-off-by: Magnus Damm <magnus@valinux.co.jp>
> >> ---
> >>
> >> Applies to 2.6.19-rc4.
> >>
> >> arch/x86_64/kernel/e820.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> --- 0002/arch/x86_64/kernel/e820.c
> >> +++ work/arch/x86_64/kernel/e820.c 2006-11-02 21:37:19.000000000 +0900
> >> @@ -594,7 +594,9 @@ static int __init parse_memmap_opt(char
> >> * size before original memory map is
> >> * reset.
> >> */
> >> + e820_register_active_regions(0, 0, -1UL);
> >> saved_max_pfn = e820_end_of_ram();
> >> + remove_all_active_ranges();
> >> #endif
> >> end_pfn_map = 0;
> >> e820.nr_map = 0;
> >
> >This looks fine to me for the time being.
>
> Great, thanks.
>

I looked at Mel's suggestion of shifting the call to
e820_register_active_regions() above parse_early_param() and that should
be a better solution.

> >Down the line I am thinking that how about passing saved_max_pfn as
> >command line parameter. I think that way we don't have to pass all the
> >memmap= options to second kernel and kexec-tools can pass the memory map
> >through parameter segment. This memory map can be modified to represent
> >only the memory which can be used by second kernel and not the whole of
> >the memory.
>
> Hm, I'm not sure how that will improve things. Isn't memmap= just used
> to inform the secondary kernel which space it can use? You need to
> tell it somehow regardless - I think the current implementation seems
> to work pretty well. But I guess you mean that we should pass an
> modified e820 map that only includes valid areas for the second
> kernel. That may simplify things. But changing the interface is
> painful.
>

Actually kexec already passes a memory map to second kernel through
parameter segment. We can just modify that memory map instead of
passing it through memmap= command line options. Currently memory map
passed by kexec is used to calculate the saved_max_pfn and it can be
calculated using a command line parameter. So effectively I am replacing
multiple memmap= command line parameter with one.

> Right now I feel that so many things are happening in the kdump world
> that it's difficult just to keep the current code working as is.
>

I agree. Probably we should do it later.

> >I think this will simplify the logic and also save us precious comand
> >line in second kernel for kdump purposes.
>
> I remember seeing some email regarding extending the amount of command
> line space, that's an alternative approach if we are running out of
> space.

I had also seen those patches. Does not seem to be upstream. Don't know
what happened to those.

Thanks
Vivek
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-11-02 19:11    [W:0.039 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site