2007-09-09 08:40:51

by Bernhard Walle

[permalink] [raw]
Subject: [patch 3/5] Use extended crashkernel command line on x86_64

This patch removes the crashkernel parsing from
arch/x86_64/kernel/machine_kexec.c and calls the generic function, introduced in
the last patch, in setup_bootmem_allocator().

This is necessary because the amount of System RAM must be known in this
function now because of the new syntax.


Signed-off-by: Bernhard Walle <[email protected]>


---
arch/x86_64/kernel/e820.c | 3 ++-
arch/x86_64/kernel/machine_kexec.c | 27 ---------------------------
arch/x86_64/kernel/setup.c | 35 ++++++++++++++++++++++++++++-------
3 files changed, 30 insertions(+), 35 deletions(-)

--- a/arch/x86_64/kernel/e820.c
+++ b/arch/x86_64/kernel/e820.c
@@ -226,7 +226,8 @@ void __init e820_reserve_resources(void)
request_resource(res, &code_resource);
request_resource(res, &data_resource);
#ifdef CONFIG_KEXEC
- request_resource(res, &crashk_res);
+ if (crashk_res.start != crashk_res.end)
+ request_resource(res, &crashk_res);
#endif
}
}
--- a/arch/x86_64/kernel/machine_kexec.c
+++ b/arch/x86_64/kernel/machine_kexec.c
@@ -231,33 +231,6 @@ NORET_TYPE void machine_kexec(struct kim
image->start);
}

-/* crashkernel=size@addr specifies the location to reserve for
- * a crash kernel. By reserving this memory we guarantee
- * that linux never set's it up as a DMA target.
- * Useful for holding code to do something appropriate
- * after a kernel panic.
- */
-static int __init setup_crashkernel(char *arg)
-{
- unsigned long size, base;
- char *p;
- if (!arg)
- return -EINVAL;
- size = memparse(arg, &p);
- if (arg == p)
- return -EINVAL;
- if (*p == '@') {
- base = memparse(p+1, &p);
- /* FIXME: Do I want a sanity check to validate the
- * memory range? Yes you do, but it's too early for
- * e820 -AK */
- crashk_res.start = base;
- crashk_res.end = base + size - 1;
- }
- return 0;
-}
-early_param("crashkernel", setup_crashkernel);
-
void arch_crash_save_vmcoreinfo(void)
{
#ifdef CONFIG_ARCH_DISCONTIGMEM_ENABLE
--- a/arch/x86_64/kernel/setup.c
+++ b/arch/x86_64/kernel/setup.c
@@ -196,6 +196,33 @@ static inline void copy_edd(void)
}
#endif

+#ifdef CONFIG_KEXEC
+static inline void reserve_crashkernel(void)
+{
+ unsigned long long free_mem;
+ unsigned long long crash_size, crash_base;
+ int ret;
+
+ free_mem = max_low_pfn << PAGE_SHIFT;
+
+ ret = parse_crashkernel(boot_command_line, free_mem,
+ &crash_size, &crash_base);
+ if (ret == 0 && crash_size > 0) {
+ printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
+ "for crashkernel (System RAM: %ldMB)\n",
+ (unsigned long)(crash_size >> 20),
+ (unsigned long)(crash_base >> 20),
+ (unsigned long)(free_mem >> 20));
+ crashk_res.start = crash_base;
+ crashk_res.end = crash_base + crash_size;
+ reserve_bootmem(crash_base, crash_size);
+ }
+}
+#else
+static inline void reserve_crashkernel(void)
+{}
+#endif
+
#define EBDA_ADDR_POINTER 0x40E

unsigned __initdata ebda_addr;
@@ -388,13 +415,7 @@ void __init setup_arch(char **cmdline_p)
}
}
#endif
-#ifdef CONFIG_KEXEC
- if (crashk_res.start != crashk_res.end) {
- reserve_bootmem_generic(crashk_res.start,
- crashk_res.end - crashk_res.start + 1);
- }
-#endif
-
+ reserve_crashkernel();
paging_init();

#ifdef CONFIG_PCI

--


2007-09-09 17:28:17

by Yinghai Lu

[permalink] [raw]
Subject: Re: [discuss] [patch 3/5] Use extended crashkernel command line on x86_64

On 9/9/07, Bernhard Walle <[email protected]> wrote:
> This patch removes the crashkernel parsing from
> arch/x86_64/kernel/machine_kexec.c and calls the generic function, introduced in
> the last patch, in setup_bootmem_allocator().
>
> This is necessary because the amount of System RAM must be known in this
> function now because of the new syntax.
>
>
> Signed-off-by: Bernhard Walle <[email protected]>
>
>
> ---
> arch/x86_64/kernel/e820.c | 3 ++-
> arch/x86_64/kernel/machine_kexec.c | 27 ---------------------------
> arch/x86_64/kernel/setup.c | 35 ++++++++++++++++++++++++++++-------
> 3 files changed, 30 insertions(+), 35 deletions(-)
>
> --- a/arch/x86_64/kernel/e820.c
> +++ b/arch/x86_64/kernel/e820.c
> @@ -226,7 +226,8 @@ void __init e820_reserve_resources(void)
> request_resource(res, &code_resource);
> request_resource(res, &data_resource);
> #ifdef CONFIG_KEXEC
...
> --- a/arch/x86_64/kernel/machine_kexec.c
> +++ b/arch/x86_64/kernel/machine_kexec.c
> @@ -231,33 +231,6 @@ NORET_TYPE void machine_kexec(struct kim
> image->start);
> }
>
> -/* crashkernel=size@addr specifies the location to reserve for
> - * a crash kernel. By reserving this memory we guarantee
> - * that linux never set's it up as a DMA target.
> - * Useful for holding code to do something appropriate
> - * after a kernel panic.
> - */
> -static int __init setup_crashkernel(char *arg)
> -{
> - unsigned long size, base;
> - char *p;
> - if (!arg)
> - return -EINVAL;
> - size = memparse(arg, &p);
> - if (arg == p)
> - return -EINVAL;
> - if (*p == '@') {
> - base = memparse(p+1, &p);
> - /* FIXME: Do I want a sanity check to validate the
> - * memory range? Yes you do, but it's too early for
> - * e820 -AK */
> - crashk_res.start = base;
> - crashk_res.end = base + size - 1;
> - }
> - return 0;
> -}
> -early_param("crashkernel", setup_crashkernel);
> -
> void arch_crash_save_vmcoreinfo(void)
> {
> #ifdef CONFIG_ARCH_DISCONTIGMEM_ENABLE
> --- a/arch/x86_64/kernel/setup.c
> +++ b/arch/x86_64/kernel/setup.c
> @@ -196,6 +196,33 @@ static inline void copy_edd(void)
> }
> #endif
>
> +#ifdef CONFIG_KEXEC
...

CONFIG_KEXEC or CONFIG_CRASH_DUMP?

YH

2007-09-09 18:53:17

by Bernhard Walle

[permalink] [raw]
Subject: Re: [discuss] [patch 3/5] Use extended crashkernel command line on x86_64

* Yinghai Lu <[email protected]> [2007-09-09 19:27]:
> >
> > +#ifdef CONFIG_KEXEC
> ...
>
> CONFIG_KEXEC or CONFIG_CRASH_DUMP?

Good question. The crashkernel parameter was CONFIG_KEXEC before, and
I also wondered why, but I didn't change this because maybe there's
some reason I don't know.

Vivek, do you know why this was CONFIG_KEXEC?


Thanks,
Bernhard

2007-09-09 21:09:47

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [discuss] [patch 3/5] Use extended crashkernel command line on x86_64

Bernhard Walle <[email protected]> writes:

> * Yinghai Lu <[email protected]> [2007-09-09 19:27]:
>> >
>> > +#ifdef CONFIG_KEXEC
>> ...
>>
>> CONFIG_KEXEC or CONFIG_CRASH_DUMP?
>
> Good question. The crashkernel parameter was CONFIG_KEXEC before, and
> I also wondered why, but I didn't change this because maybe there's
> some reason I don't know.
>
> Vivek, do you know why this was CONFIG_KEXEC?

Probably because you use it in the primary kernel you use it.
The option reserves an area of memory for the kernel we switch
to on panic or another kernel crash.

Generally CONFIG_CRASH_DUMP seems to be about the options needed
to read out the crash dump after the fact.

Eric

2007-09-11 05:14:53

by Vivek Goyal

[permalink] [raw]
Subject: Re: [discuss] [patch 3/5] Use extended crashkernel command line on x86_64

On Sun, Sep 09, 2007 at 08:52:58PM +0200, Bernhard Walle wrote:
> * Yinghai Lu <[email protected]> [2007-09-09 19:27]:
> > >
> > > +#ifdef CONFIG_KEXEC
> > ...
> >
> > CONFIG_KEXEC or CONFIG_CRASH_DUMP?
>
> Good question. The crashkernel parameter was CONFIG_KEXEC before, and
> I also wondered why, but I didn't change this because maybe there's
> some reason I don't know.
>
> Vivek, do you know why this was CONFIG_KEXEC?
>
>

Bernhard,

As Eric mentioned, CONFIG_CRASH_DUMP has been used for all dump capturing
infrastructure and rest of the kexec and kexec on panic functionality
has been put under CONFIG_KEXEC.

Keeping memory reservation under CONFIG_KEXEC helps in a sense when
somebody is not using a relocatable kernel and uses a custom kernel for dump
capture. In that case he does not have to enable CONFIG_CRASH_DUMP in the
first kernel.

Thanks
Vivek

2007-09-11 10:01:56

by Bernhard Walle

[permalink] [raw]
Subject: Re: [discuss] [patch 3/5] Use extended crashkernel command line on x86_64

* Vivek Goyal <[email protected]> [2007-09-11 07:14]:
> On Sun, Sep 09, 2007 at 08:52:58PM +0200, Bernhard Walle wrote:
> > * Yinghai Lu <[email protected]> [2007-09-09 19:27]:
> > > >
> > > > +#ifdef CONFIG_KEXEC
> > > ...
> > >
> > > CONFIG_KEXEC or CONFIG_CRASH_DUMP?
> >
> > Good question. The crashkernel parameter was CONFIG_KEXEC before, and
> > I also wondered why, but I didn't change this because maybe there's
> > some reason I don't know.
> >
> > Vivek, do you know why this was CONFIG_KEXEC?
>
> As Eric mentioned, CONFIG_CRASH_DUMP has been used for all dump capturing
> infrastructure and rest of the kexec and kexec on panic functionality
> has been put under CONFIG_KEXEC.
>
> Keeping memory reservation under CONFIG_KEXEC helps in a sense when
> somebody is not using a relocatable kernel and uses a custom kernel for dump
> capture. In that case he does not have to enable CONFIG_CRASH_DUMP in the
> first kernel.

Yes, you all are right ... sorry for the noise ;)


Thanks,
Bernhard