2008-07-17 22:15:20

by Bernhard Walle

[permalink] [raw]
Subject: [PATCH] x86: Move crashkernel reservation before dma32_reserve_bootmem()

On a x86-64 machine (nothing special I could encounter) I had the problem that
crashkernel reservation with the usual "64M@16M" failed. While debugging that,
I encountered that dma32_reserve_bootmem() reserves a memory region which is in
that area.

Because dma32_reserve_bootmem() does not rely on a specific offset but
crashkernel does, it makes sense to move the crashkernel reservation up a bit.
I tested that patch and it works without problems. I don't see any negative
effects of that move, but maybe I oversaw something ...

While we strictly don't need that patch in 2.6.27 because we have the
automatic, dynamic offset detection, it makes sense to also include it here
because:

- it's easier to get it in -stable then,
- many people are still used to the 'crashkernel=...@16M' syntax,
- not everybody may be using a reloatable kernel.

Signed-off-by: Bernhard Walle <[email protected]>
---
arch/x86/kernel/setup.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 531b55b..16101c0 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -792,6 +792,12 @@ void __init setup_arch(char **cmdline_p)

initmem_init(0, max_pfn);

+ /*
+ * dma32_reserve_bootmem() allocates bootmem which may conflict
+ * with the crashkernel command line, so do that before
+ */
+ reserve_crashkernel();
+
#ifdef CONFIG_X86_64
dma32_reserve_bootmem();
#endif
@@ -808,7 +814,6 @@ void __init setup_arch(char **cmdline_p)
*/
find_smp_config();
#endif
- reserve_crashkernel();

reserve_ibft_region();

--
1.5.6


2008-07-17 22:50:28

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] x86: Move crashkernel reservation before dma32_reserve_bootmem()

On Thu, Jul 17, 2008 at 3:15 PM, Bernhard Walle <[email protected]> wrote:
> On a x86-64 machine (nothing special I could encounter) I had the problem that
> crashkernel reservation with the usual "64M@16M" failed. While debugging that,
> I encountered that dma32_reserve_bootmem() reserves a memory region which is in
> that area.
>
> Because dma32_reserve_bootmem() does not rely on a specific offset but
> crashkernel does, it makes sense to move the crashkernel reservation up a bit.
> I tested that patch and it works without problems. I don't see any negative
> effects of that move, but maybe I oversaw something ...
>
> While we strictly don't need that patch in 2.6.27 because we have the
> automatic, dynamic offset detection, it makes sense to also include it here
> because:
>
> - it's easier to get it in -stable then,
> - many people are still used to the 'crashkernel=...@16M' syntax,
> - not everybody may be using a reloatable kernel.
>
> Signed-off-by: Bernhard Walle <[email protected]>
> ---
> arch/x86/kernel/setup.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 531b55b..16101c0 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -792,6 +792,12 @@ void __init setup_arch(char **cmdline_p)
>
> initmem_init(0, max_pfn);
>
> + /*
> + * dma32_reserve_bootmem() allocates bootmem which may conflict
> + * with the crashkernel command line, so do that before
> + */
> + reserve_crashkernel();
> +
> #ifdef CONFIG_X86_64
> dma32_reserve_bootmem();
> #endif
> @@ -808,7 +814,6 @@ void __init setup_arch(char **cmdline_p)
> */
> find_smp_config();
> #endif
> - reserve_crashkernel();
>
> reserve_ibft_region();
>

Joe Jin already had another one to move dma32_reserve_bootmem later

YH

2008-07-18 09:51:46

by Bernhard Walle

[permalink] [raw]
Subject: Re: [PATCH] x86: Move crashkernel reservation before dma32_reserve_bootmem()

* Yinghai Lu [2008-07-17 15:50]:
>
> > + /*
> > + * dma32_reserve_bootmem() allocates bootmem which may conflict
> > + * with the crashkernel command line, so do that before
> > + */
> > + reserve_crashkernel();
> > +
> > #ifdef CONFIG_X86_64
> > dma32_reserve_bootmem();
> > #endif
> > @@ -808,7 +814,6 @@ void __init setup_arch(char **cmdline_p)
> > */
> > find_smp_config();
> > #endif
> > - reserve_crashkernel();
> >
> > reserve_ibft_region();
>
> Joe Jin already had another one to move dma32_reserve_bootmem later

Commit id? Current status?


Bernhard
--
Bernhard Walle, SUSE LINUX Products GmbH, Architecture Development

2008-07-18 16:22:37

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] x86: Move crashkernel reservation before dma32_reserve_bootmem()

On Fri, Jul 18, 2008 at 2:52 AM, Bernhard Walle <[email protected]> wrote:
> * Yinghai Lu [2008-07-17 15:50]:
>>
>> > + /*
>> > + * dma32_reserve_bootmem() allocates bootmem which may conflict
>> > + * with the crashkernel command line, so do that before
>> > + */
>> > + reserve_crashkernel();
>> > +
>> > #ifdef CONFIG_X86_64
>> > dma32_reserve_bootmem();
>> > #endif
>> > @@ -808,7 +814,6 @@ void __init setup_arch(char **cmdline_p)
>> > */
>> > find_smp_config();
>> > #endif
>> > - reserve_crashkernel();
>> >
>> > reserve_ibft_region();
>>
>> Joe Jin already had another one to move dma32_reserve_bootmem later
>
> Commit id? Current status?

it seems commit description from you is more good.
please create one version move dma32_reserve_boot_mem after
reserve_crashkernel and use your commit description.

YH

2008-07-18 17:07:35

by Bernhard Walle

[permalink] [raw]
Subject: [PATCH] x86: Move dma32_reserve_bootmem() after reserve_crashkernel()

On a x86-64 machine (nothing special I could encounter) I had the problem that
crashkernel reservation with the usual "64M@16M" failed. While debugging that,
I encountered that dma32_reserve_bootmem() reserves a memory region which is in
that area.

Because dma32_reserve_bootmem() does not rely on a specific offset but
crashkernel does, it makes sense to move the dma32_reserve_bootmem()
reservation down a bit. I tested that patch and it works without problems. I
don't see any negative effects of that move, but maybe I oversaw something ...

While we strictly don't need that patch in 2.6.27 because we have the
automatic, dynamic offset detection, it makes sense to also include it here
because:

- it's easier to get it in -stable then,
- many people are still used to the 'crashkernel=...@16M' syntax,
- not everybody may be using a reloatable kernel.

Signed-off-by: Bernhard Walle <[email protected]>
---
arch/x86/kernel/setup.c | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 531b55b..74d110e 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -792,10 +792,6 @@ void __init setup_arch(char **cmdline_p)

initmem_init(0, max_pfn);

-#ifdef CONFIG_X86_64
- dma32_reserve_bootmem();
-#endif
-
#ifdef CONFIG_ACPI_SLEEP
/*
* Reserve low memory region for sleep support.
@@ -810,6 +806,15 @@ void __init setup_arch(char **cmdline_p)
#endif
reserve_crashkernel();

+#ifdef CONFIG_X86_64
+ /*
+ * dma32_reserve_bootmem() allocates bootmem which may conflict
+ * with the crashkernel command line, so do that after
+ * reserve_crashkernel()
+ */
+ dma32_reserve_bootmem();
+#endif
+
reserve_ibft_region();

#ifdef CONFIG_KVM_CLOCK
--
1.5.6

2008-07-18 21:36:51

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86: Move dma32_reserve_bootmem() after reserve_crashkernel()


* Bernhard Walle <[email protected]> wrote:

> On a x86-64 machine (nothing special I could encounter) I had the
> problem that crashkernel reservation with the usual "64M@16M" failed.
> While debugging that, I encountered that dma32_reserve_bootmem()
> reserves a memory region which is in that area.
>
> Because dma32_reserve_bootmem() does not rely on a specific offset but
> crashkernel does, it makes sense to move the dma32_reserve_bootmem()
> reservation down a bit. I tested that patch and it works without
> problems. I don't see any negative effects of that move, but maybe I
> oversaw something ...
>
> While we strictly don't need that patch in 2.6.27 because we have the
> automatic, dynamic offset detection, it makes sense to also include it
> here because:
>
> - it's easier to get it in -stable then,
> - many people are still used to the 'crashkernel=...@16M' syntax,
> - not everybody may be using a reloatable kernel.

applied to tip/x86/crashdump, thanks Bernhard.

Ingo