>From aebc336baa7ec2d4ccb6f21166770c7d2ee26cba Mon Sep 17 00:00:00 2001
From: jzha144 <[email protected]>
Date: Wed, 31 Oct 2012 08:51:18 +0800
Subject: [PATCH] To crash dump, we need keep other memory type except
E820_RAM, because other type come from BIOS or firmware is
used by other code(for example: PCI_MMCONFIG).
Signed-off-by: jzha144 <[email protected]>
---
arch/x86/kernel/e820.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index df06ade..8760427 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -851,6 +851,15 @@ static int __init parse_memmap_opt(char *p)
* reset.
*/
saved_max_pfn = e820_end_of_ram_pfn();
+
+ /*
+ * To CRASH DUMP, only remove E820_RAM.
+ * some other memory typecome from BIOS or firmware,
+ * it must be same with system kernel.
+ */
+ e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
+ userdef = 1;
+ return 0;
#endif
e820.nr_map = 0;
userdef = 1;
--
1.7.6
On 10/30/2012 06:26 PM, Zhang, Jun wrote:
> From aebc336baa7ec2d4ccb6f21166770c7d2ee26cba Mon Sep 17 00:00:00 2001
> From: jzha144 <[email protected]>
> Date: Wed, 31 Oct 2012 08:51:18 +0800
> Subject: [PATCH] To crash dump, we need keep other memory type except
> E820_RAM, because other type come from BIOS or firmware is
> used by other code(for example: PCI_MMCONFIG).
I'm sorry, I can't quite parse the description or the comment... could
you clarify it a bit? I think I know what you mean, but there is
clearly risk for misunderstandings.
-hpa
Hello, Anvin
Thanks!
Hello, all
Next is my the latest version, please review it.
Thanks!
>From 141546c77ff7be523a9e72f5259df4a6827f2c1a Mon Sep 17 00:00:00 2001
From: jzha144 <[email protected]>
Date: Wed, 31 Oct 2012 08:51:18 +0800
Subject: [PATCH] If we are doing a crash dump, we still need non-E820_RAM
memory type address information, which come from BIOS or
firmware. for example: PCI_MMCONFIG check this address.
Signed-off-by: jzha144 <[email protected]>
---
arch/x86/kernel/e820.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index df06ade..f8672d0 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -851,6 +851,15 @@ static int __init parse_memmap_opt(char *p)
* reset.
*/
saved_max_pfn = e820_end_of_ram_pfn();
+
+ /*
+ * If we are doing a crash dump, we still need non-E820_RAM
+ * memory type address information. so we only remove
+ * E820_RAM type.
+ */
+ e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
+ userdef = 1;
+ return 0;
#endif
e820.nr_map = 0;
userdef = 1;
--
1.7.6
Best Regards!
Jun Zhang
Inet: 8821-4273
Dir.Tel: 86-21-6116-4273
Email: [email protected]
-----Original Message-----
From: H. Peter Anvin [mailto:[email protected]]
Sent: Wednesday, October 31, 2012 10:47 AM
To: Zhang, Jun
Cc: Thomas Gleixner; Ingo Molnar; [email protected]; Andrew Morton; Fleming, Matt; Paul Gortmaker; [email protected]
Subject: Re: [PATCH] To crash dump, we need keep other memory type except E820_RAM, because other type come from BIOS or firmware is used by other code(for example: PCI_MMCONFIG).
On 10/30/2012 06:26 PM, Zhang, Jun wrote:
> From aebc336baa7ec2d4ccb6f21166770c7d2ee26cba Mon Sep 17 00:00:00 2001
> From: jzha144 <[email protected]>
> Date: Wed, 31 Oct 2012 08:51:18 +0800
> Subject: [PATCH] To crash dump, we need keep other memory type except
> E820_RAM, because other type come from BIOS or firmware is used by
> other code(for example: PCI_MMCONFIG).
I'm sorry, I can't quite parse the description or the comment... could you clarify it a bit? I think I know what you mean, but there is clearly risk for misunderstandings.
-hpa
On 10/30/2012 08:39 PM, Zhang, Jun wrote:
> Hello, Anvin
> Thanks!
>
> Hello, all
> Next is my the latest version, please review it.
> Thanks!
You're still starting in the wrong end which is confusing for the reader.
What you probably want to say is something more like:
"We are doing a crash dump, so remove all RAM ranges as they are the
ones that need to be dumped. We still need all non-RAM information in
order to do I/O."
At that point it should be pretty obvious that the patch is wrong. What
if we are *not* doing a crash dump? Just because crash dump is compiled
in doesn't mean that that is what we are doing right now.
-hpa
> From 141546c77ff7be523a9e72f5259df4a6827f2c1a Mon Sep 17 00:00:00 2001
> From: jzha144 <[email protected]>
> Date: Wed, 31 Oct 2012 08:51:18 +0800
> Subject: [PATCH] If we are doing a crash dump, we still need non-E820_RAM
> memory type address information, which come from BIOS or
> firmware. for example: PCI_MMCONFIG check this address.
>
> Signed-off-by: jzha144 <[email protected]>
> ---
> arch/x86/kernel/e820.c | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index df06ade..f8672d0 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -851,6 +851,15 @@ static int __init parse_memmap_opt(char *p)
> * reset.
> */
> saved_max_pfn = e820_end_of_ram_pfn();
> +
> + /*
> + * If we are doing a crash dump, we still need non-E820_RAM
> + * memory type address information. so we only remove
> + * E820_RAM type.
> + */
> + e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
> + userdef = 1;
> + return 0;
> #endif
> e820.nr_map = 0;
> userdef = 1;
>
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
Hello, Anvin
You are right. Thanks!
Hello, All
Please review it again. Thanks!
>From bf7506ac7e9ce0df0b915164dbb7a6d858ef2e40 Mon Sep 17 00:00:00 2001
From: jzha144 <[email protected]>
Date: Wed, 31 Oct 2012 08:51:18 +0800
Subject: [PATCH] When we are doing a crash dump, we still need non-E820_RAM
memory type address information in order to do I/O. so only
remove all RAM ranges which need to be dumped.
Signed-off-by: jzha144 <[email protected]>
---
arch/x86/kernel/e820.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index df06ade..77be839 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -851,6 +851,15 @@ static int __init parse_memmap_opt(char *p)
* reset.
*/
saved_max_pfn = e820_end_of_ram_pfn();
+
+ /*
+ * We are doing a crash dump, so remove all RAM ranges
+ * as they are the ones that need to be dumped.
+ * We still need all non-RAM information in order to do I/O.
+ */
+ e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
+ userdef = 1;
+ return 0;
#endif
e820.nr_map = 0;
userdef = 1;
--
1.7.6
Best Regards!
Zhang, jun
-----Original Message-----
From: H. Peter Anvin [mailto:[email protected]]
Sent: Wednesday, October 31, 2012 12:38 PM
To: Zhang, Jun
Cc: Thomas Gleixner; Ingo Molnar; [email protected]; Andrew Morton; Fleming, Matt; Paul Gortmaker; [email protected]
Subject: Re: [PATCH] To crash dump, we need keep other memory type except E820_RAM, because other type come from BIOS or firmware is used by other code(for example: PCI_MMCONFIG).
On 10/30/2012 08:39 PM, Zhang, Jun wrote:
> Hello, Anvin
> Thanks!
>
> Hello, all
> Next is my the latest version, please review it.
> Thanks!
You're still starting in the wrong end which is confusing for the reader.
What you probably want to say is something more like:
"We are doing a crash dump, so remove all RAM ranges as they are the ones that need to be dumped. We still need all non-RAM information in order to do I/O."
At that point it should be pretty obvious that the patch is wrong. What if we are *not* doing a crash dump? Just because crash dump is compiled in doesn't mean that that is what we are doing right now.
-hpa
> From 141546c77ff7be523a9e72f5259df4a6827f2c1a Mon Sep 17 00:00:00
> 2001
> From: jzha144 <[email protected]>
> Date: Wed, 31 Oct 2012 08:51:18 +0800
> Subject: [PATCH] If we are doing a crash dump, we still need non-E820_RAM
> memory type address information, which come from BIOS or
> firmware. for example: PCI_MMCONFIG check this address.
>
> Signed-off-by: jzha144 <[email protected]>
> ---
> arch/x86/kernel/e820.c | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c index
> df06ade..f8672d0 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -851,6 +851,15 @@ static int __init parse_memmap_opt(char *p)
> * reset.
> */
> saved_max_pfn = e820_end_of_ram_pfn();
> +
> + /*
> + * If we are doing a crash dump, we still need non-E820_RAM
> + * memory type address information. so we only remove
> + * E820_RAM type.
> + */
> + e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
> + userdef = 1;
> + return 0;
> #endif
> e820.nr_map = 0;
> userdef = 1;
>
--
H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On 10/30/2012 10:22 PM, Zhang, Jun wrote:
> Hello, Anvin
> You are right. Thanks!
>
> Hello, All
> Please review it again. Thanks!
>
> From bf7506ac7e9ce0df0b915164dbb7a6d858ef2e40 Mon Sep 17 00:00:00 2001
> From: jzha144 <[email protected]>
> Date: Wed, 31 Oct 2012 08:51:18 +0800
> Subject: [PATCH] When we are doing a crash dump, we still need non-E820_RAM
> memory type address information in order to do I/O. so only
> remove all RAM ranges which need to be dumped.
>
> Signed-off-by: jzha144 <[email protected]>
> ---
> arch/x86/kernel/e820.c | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index df06ade..77be839 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -851,6 +851,15 @@ static int __init parse_memmap_opt(char *p)
> * reset.
> */
> saved_max_pfn = e820_end_of_ram_pfn();
> +
> + /*
> + * We are doing a crash dump, so remove all RAM ranges
> + * as they are the ones that need to be dumped.
> + * We still need all non-RAM information in order to do I/O.
> + */
> + e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
> + userdef = 1;
> + return 0;
> #endif
> e820.nr_map = 0;
> userdef = 1;
>
The code is still wrong...
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
Hello, Anvin
I want to explain why I modify in this place. In kexec, it pass three parameters, memmap=exactmap memmap=544K@64K memmap=64964K@32768K
I think my patch modify the least code.
Actually, there are some choise to fix it.
1) my patch.
2) modify kexec, only pass two parameters -- memmap=544K@64K memmap=64964K@32768K, in kernel setup_memory_map, we can remove RAM range.
3) add extra optional, like memmap=REMOVERAM
Which one do you like? Maybe you have better solution, please share it.
Thanks!
Best Regards!
Jun Zhang
Inet: 8821-4273
Dir.Tel: 86-21-6116-4273
Email: [email protected]
-----Original Message-----
From: H. Peter Anvin [mailto:[email protected]]
Sent: Wednesday, October 31, 2012 1:39 PM
To: Zhang, Jun
Cc: Thomas Gleixner; Ingo Molnar; [email protected]; Andrew Morton; Fleming, Matt; Paul Gortmaker; [email protected]
Subject: Re: [PATCH] To crash dump, we need keep other memory type except E820_RAM, because other type come from BIOS or firmware is used by other code(for example: PCI_MMCONFIG).
On 10/30/2012 10:22 PM, Zhang, Jun wrote:
> Hello, Anvin
> You are right. Thanks!
>
> Hello, All
> Please review it again. Thanks!
>
> From bf7506ac7e9ce0df0b915164dbb7a6d858ef2e40 Mon Sep 17 00:00:00
> 2001
> From: jzha144 <[email protected]>
> Date: Wed, 31 Oct 2012 08:51:18 +0800
> Subject: [PATCH] When we are doing a crash dump, we still need non-E820_RAM
> memory type address information in order to do I/O. so only
> remove all RAM ranges which need to be dumped.
>
> Signed-off-by: jzha144 <[email protected]>
> ---
> arch/x86/kernel/e820.c | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c index
> df06ade..77be839 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -851,6 +851,15 @@ static int __init parse_memmap_opt(char *p)
> * reset.
> */
> saved_max_pfn = e820_end_of_ram_pfn();
> +
> + /*
> + * We are doing a crash dump, so remove all RAM ranges
> + * as they are the ones that need to be dumped.
> + * We still need all non-RAM information in order to do I/O.
> + */
> + e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
> + userdef = 1;
> + return 0;
> #endif
> e820.nr_map = 0;
> userdef = 1;
>
The code is still wrong...
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
2) would make most sense to me, but I'd be okay with 3) as well.
"Zhang, Jun" <[email protected]> wrote:
>Hello, Anvin
>
>I want to explain why I modify in this place. In kexec, it pass three
>parameters, memmap=exactmap memmap=544K@64K memmap=64964K@32768K
>I think my patch modify the least code.
>Actually, there are some choise to fix it.
>1) my patch.
>2) modify kexec, only pass two parameters -- memmap=544K@64K
>memmap=64964K@32768K, in kernel setup_memory_map, we can remove RAM
>range.
>3) add extra optional, like memmap=REMOVERAM
>
>Which one do you like? Maybe you have better solution, please share it.
>Thanks!
>
>Best Regards!
>
>Jun Zhang
>Inet: 8821-4273
>Dir.Tel: 86-21-6116-4273
>Email: [email protected]
>
>-----Original Message-----
>From: H. Peter Anvin [mailto:[email protected]]
>Sent: Wednesday, October 31, 2012 1:39 PM
>To: Zhang, Jun
>Cc: Thomas Gleixner; Ingo Molnar; [email protected]; Andrew Morton;
>Fleming, Matt; Paul Gortmaker; [email protected]
>Subject: Re: [PATCH] To crash dump, we need keep other memory type
>except E820_RAM, because other type come from BIOS or firmware is used
>by other code(for example: PCI_MMCONFIG).
>
>On 10/30/2012 10:22 PM, Zhang, Jun wrote:
>> Hello, Anvin
>> You are right. Thanks!
>>
>> Hello, All
>> Please review it again. Thanks!
>>
>> From bf7506ac7e9ce0df0b915164dbb7a6d858ef2e40 Mon Sep 17 00:00:00
>> 2001
>> From: jzha144 <[email protected]>
>> Date: Wed, 31 Oct 2012 08:51:18 +0800
>> Subject: [PATCH] When we are doing a crash dump, we still need
>non-E820_RAM
>> memory type address information in order to do I/O. so only
>> remove all RAM ranges which need to be dumped.
>>
>> Signed-off-by: jzha144 <[email protected]>
>> ---
>> arch/x86/kernel/e820.c | 9 +++++++++
>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c index
>> df06ade..77be839 100644
>> --- a/arch/x86/kernel/e820.c
>> +++ b/arch/x86/kernel/e820.c
>> @@ -851,6 +851,15 @@ static int __init parse_memmap_opt(char *p)
>> * reset.
>> */
>> saved_max_pfn = e820_end_of_ram_pfn();
>> +
>> + /*
>> + * We are doing a crash dump, so remove all RAM ranges
>> + * as they are the ones that need to be dumped.
>> + * We still need all non-RAM information in order to do I/O.
>> + */
>> + e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
>> + userdef = 1;
>> + return 0;
>> #endif
>> e820.nr_map = 0;
>> userdef = 1;
>>
>
>The code is still wrong...
>
> -hpa
>
>
>--
>H. Peter Anvin, Intel Open Source Technology Center I work for Intel.
>I don't speak on their behalf.
--
Sent from my mobile phone. Please excuse brevity and lack of formatting.
Hello, Anvin
Thank for your advice.
Hello, All
the next patch is made by 2), please review it. Thanks!
Subject: [PATCH] When we are doing a crash dump, we still need non-E820_RAM
memory information in order to do I/O. So only remove all
RAM ranges which need to be dumped.
Signed-off-by: jzha144 <[email protected]>
---
arch/x86/kernel/e820.c | 8 --------
arch/x86/kernel/setup.c | 22 ++++++++++++++++++++++
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index df06ade..0bc1687 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -844,14 +844,6 @@ static int __init parse_memmap_opt(char *p)
return -EINVAL;
if (!strncmp(p, "exactmap", 8)) {
-#ifdef CONFIG_CRASH_DUMP
- /*
- * If we are doing a crash dump, we still need to know
- * the real mem size before original memory map is
- * reset.
- */
- saved_max_pfn = e820_end_of_ram_pfn();
-#endif
e820.nr_map = 0;
userdef = 1;
return 0;
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index ca45696..5eb178b 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -480,6 +480,25 @@ static void __init e820_reserve_setup_data(void)
e820_print_map("reserve setup_data");
}
+#ifdef CONFIG_CRASH_DUMP
+static void __init e820_crashdump_remove_ram(void)
+{
+ /*
+ * We are doing a crash dump, so remove all RAM ranges
+ * as they are the ones that need to be dumped.
+ * We still need all non-RAM information in order to do I/O.
+ */
+ /* NOTE: if you use old kexec, please remove memmap=exactmap
+ * which remove all ranges, not only RAM ranges.
+ */
+ saved_max_pfn = e820_end_of_ram_pfn();
+ e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
+ sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+ printk(KERN_INFO "crash dump non-RAM map:\n");
+ e820_print_map("crash_dump");
+}
+#endif
+
static void __init memblock_x86_reserve_range_setup_data(void)
{
struct setup_data *data;
@@ -751,6 +770,9 @@ void __init setup_arch(char **cmdline_p)
parse_setup_data();
/* update the e820_saved too */
e820_reserve_setup_data();
+#ifdef CONFIG_CRASH_DUMP
+ e820_crashdump_remove_ram();
+#endif
copy_edd();
--
1.7.6
Best Regards!
Jun Zhang
Inet: 8821-4273
Dir.Tel: 86-21-6116-4273
Email: [email protected]
-----Original Message-----
From: H. Peter Anvin [mailto:[email protected]]
Sent: Thursday, November 01, 2012 12:20 PM
To: Zhang, Jun
Cc: Thomas Gleixner; Ingo Molnar; [email protected]; Andrew Morton; Fleming, Matt; Paul Gortmaker; [email protected]
Subject: RE: [PATCH] To crash dump, we need keep other memory type except E820_RAM, because other type come from BIOS or firmware is used by other code(for example: PCI_MMCONFIG).
2) would make most sense to me, but I'd be okay with 3) as well.
"Zhang, Jun" <[email protected]> wrote:
>Hello, Anvin
>
>I want to explain why I modify in this place. In kexec, it pass three
>parameters, memmap=exactmap memmap=544K@64K memmap=64964K@32768K I
>think my patch modify the least code.
>Actually, there are some choise to fix it.
>1) my patch.
>2) modify kexec, only pass two parameters -- memmap=544K@64K
>memmap=64964K@32768K, in kernel setup_memory_map, we can remove RAM
>range.
>3) add extra optional, like memmap=REMOVERAM
>
>Which one do you like? Maybe you have better solution, please share it.
>Thanks!
>
>Best Regards!
>
>Jun Zhang
>Inet: 8821-4273
>Dir.Tel: 86-21-6116-4273
>Email: [email protected]
>
>-----Original Message-----
>From: H. Peter Anvin [mailto:[email protected]]
>Sent: Wednesday, October 31, 2012 1:39 PM
>To: Zhang, Jun
>Cc: Thomas Gleixner; Ingo Molnar; [email protected]; Andrew Morton;
>Fleming, Matt; Paul Gortmaker; [email protected]
>Subject: Re: [PATCH] To crash dump, we need keep other memory type
>except E820_RAM, because other type come from BIOS or firmware is used
>by other code(for example: PCI_MMCONFIG).
>
>On 10/30/2012 10:22 PM, Zhang, Jun wrote:
>> Hello, Anvin
>> You are right. Thanks!
>>
>> Hello, All
>> Please review it again. Thanks!
>>
>> From bf7506ac7e9ce0df0b915164dbb7a6d858ef2e40 Mon Sep 17 00:00:00
>> 2001
>> From: jzha144 <[email protected]>
>> Date: Wed, 31 Oct 2012 08:51:18 +0800
>> Subject: [PATCH] When we are doing a crash dump, we still need
>non-E820_RAM
>> memory type address information in order to do I/O. so only
>> remove all RAM ranges which need to be dumped.
>>
>> Signed-off-by: jzha144 <[email protected]>
>> ---
>> arch/x86/kernel/e820.c | 9 +++++++++
>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c index
>> df06ade..77be839 100644
>> --- a/arch/x86/kernel/e820.c
>> +++ b/arch/x86/kernel/e820.c
>> @@ -851,6 +851,15 @@ static int __init parse_memmap_opt(char *p)
>> * reset.
>> */
>> saved_max_pfn = e820_end_of_ram_pfn();
>> +
>> + /*
>> + * We are doing a crash dump, so remove all RAM ranges
>> + * as they are the ones that need to be dumped.
>> + * We still need all non-RAM information in order to do I/O.
>> + */
>> + e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
>> + userdef = 1;
>> + return 0;
>> #endif
>> e820.nr_map = 0;
>> userdef = 1;
>>
>
>The code is still wrong...
>
> -hpa
>
>
>--
>H. Peter Anvin, Intel Open Source Technology Center I work for Intel.
>I don't speak on their behalf.
--
Sent from my mobile phone. Please excuse brevity and lack of formatting.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
[RE: [PATCH] To crash dump, we need keep other memory type except E820_RAM, because other type come from BIOS or firmware is used by other code(for example: PCI_MMCONFIG).] On 01/11/2012 (Thu 08:49) Zhang, Jun wrote:
> Hello, Anvin
>
> Thank for your advice.
>
> Hello, All
>
> the next patch is made by 2), please review it. Thanks!
>
> Subject: [PATCH] When we are doing a crash dump, we still need non-E820_RAM
> memory information in order to do I/O. So only remove all
> RAM ranges which need to be dumped.
It is typical to do a "short log" in the subject, and then a "long log"
in what would be the following paragraph, i.e.
---------
Subject: crash dump: don't delete non-E820_RAM during init
Currently we delete the non-E820_RAM, which causes <describe the end
user symptoms here -- i.e. how things visibly break>
This happens because <describe the underlying technical reason
that causes the problem>
We can fix this by doing <describe the non-obvious aspects of your
change and why it is the right way to fix the problem>
---------
This "rule of three" is a good way to write commit logs. Just remember
(1)symptoms, (2)underlying problem, (3)how best to fix it.
Also, ...
>
> Signed-off-by: jzha144 <[email protected]>
> ---
> arch/x86/kernel/e820.c | 8 --------
> arch/x86/kernel/setup.c | 22 ++++++++++++++++++++++
> 2 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index df06ade..0bc1687 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -844,14 +844,6 @@ static int __init parse_memmap_opt(char *p)
> return -EINVAL;
>
> if (!strncmp(p, "exactmap", 8)) {
> -#ifdef CONFIG_CRASH_DUMP
> - /*
> - * If we are doing a crash dump, we still need to know
> - * the real mem size before original memory map is
> - * reset.
> - */
> - saved_max_pfn = e820_end_of_ram_pfn();
> -#endif
> e820.nr_map = 0;
> userdef = 1;
> return 0;
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index ca45696..5eb178b 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -480,6 +480,25 @@ static void __init e820_reserve_setup_data(void)
> e820_print_map("reserve setup_data");
> }
>
> +#ifdef CONFIG_CRASH_DUMP
> +static void __init e820_crashdump_remove_ram(void)
> +{
... if you move this ifdef/endif within the { } of the function, then
we'll have one less ugly ifdef set below at the call site.
I'll leave the real technical review for Peter, who understands this
area better than I ever will.
Thanks,
Paul.
--
> + /*
> + * We are doing a crash dump, so remove all RAM ranges
> + * as they are the ones that need to be dumped.
> + * We still need all non-RAM information in order to do I/O.
> + */
> + /* NOTE: if you use old kexec, please remove memmap=exactmap
> + * which remove all ranges, not only RAM ranges.
> + */
> + saved_max_pfn = e820_end_of_ram_pfn();
> + e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
> + sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
> + printk(KERN_INFO "crash dump non-RAM map:\n");
> + e820_print_map("crash_dump");
> +}
> +#endif
> +
> static void __init memblock_x86_reserve_range_setup_data(void)
> {
> struct setup_data *data;
> @@ -751,6 +770,9 @@ void __init setup_arch(char **cmdline_p)
> parse_setup_data();
> /* update the e820_saved too */
> e820_reserve_setup_data();
> +#ifdef CONFIG_CRASH_DUMP
> + e820_crashdump_remove_ram();
> +#endif
>
> copy_edd();
>
> --
> 1.7.6
>
> Best Regards!
>
> Jun Zhang
> Inet: 8821-4273
> Dir.Tel: 86-21-6116-4273
> Email: [email protected]
>
>
> -----Original Message-----
> From: H. Peter Anvin [mailto:[email protected]]
> Sent: Thursday, November 01, 2012 12:20 PM
> To: Zhang, Jun
> Cc: Thomas Gleixner; Ingo Molnar; [email protected]; Andrew Morton; Fleming, Matt; Paul Gortmaker; [email protected]
> Subject: RE: [PATCH] To crash dump, we need keep other memory type except E820_RAM, because other type come from BIOS or firmware is used by other code(for example: PCI_MMCONFIG).
>
> 2) would make most sense to me, but I'd be okay with 3) as well.
>
> "Zhang, Jun" <[email protected]> wrote:
>
> >Hello, Anvin
> >
> >I want to explain why I modify in this place. In kexec, it pass three
> >parameters, memmap=exactmap memmap=544K@64K memmap=64964K@32768K I
> >think my patch modify the least code.
> >Actually, there are some choise to fix it.
> >1) my patch.
> >2) modify kexec, only pass two parameters -- memmap=544K@64K
> >memmap=64964K@32768K, in kernel setup_memory_map, we can remove RAM
> >range.
> >3) add extra optional, like memmap=REMOVERAM
> >
> >Which one do you like? Maybe you have better solution, please share it.
> >Thanks!
> >
> >Best Regards!
> >
> >Jun Zhang
> >Inet: 8821-4273
> >Dir.Tel: 86-21-6116-4273
> >Email: [email protected]
> >
> >-----Original Message-----
> >From: H. Peter Anvin [mailto:[email protected]]
> >Sent: Wednesday, October 31, 2012 1:39 PM
> >To: Zhang, Jun
> >Cc: Thomas Gleixner; Ingo Molnar; [email protected]; Andrew Morton;
> >Fleming, Matt; Paul Gortmaker; [email protected]
> >Subject: Re: [PATCH] To crash dump, we need keep other memory type
> >except E820_RAM, because other type come from BIOS or firmware is used
> >by other code(for example: PCI_MMCONFIG).
> >
> >On 10/30/2012 10:22 PM, Zhang, Jun wrote:
> >> Hello, Anvin
> >> You are right. Thanks!
> >>
> >> Hello, All
> >> Please review it again. Thanks!
> >>
> >> From bf7506ac7e9ce0df0b915164dbb7a6d858ef2e40 Mon Sep 17 00:00:00
> >> 2001
> >> From: jzha144 <[email protected]>
> >> Date: Wed, 31 Oct 2012 08:51:18 +0800
> >> Subject: [PATCH] When we are doing a crash dump, we still need
> >non-E820_RAM
> >> memory type address information in order to do I/O. so only
> >> remove all RAM ranges which need to be dumped.
> >>
> >> Signed-off-by: jzha144 <[email protected]>
> >> ---
> >> arch/x86/kernel/e820.c | 9 +++++++++
> >> 1 files changed, 9 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c index
> >> df06ade..77be839 100644
> >> --- a/arch/x86/kernel/e820.c
> >> +++ b/arch/x86/kernel/e820.c
> >> @@ -851,6 +851,15 @@ static int __init parse_memmap_opt(char *p)
> >> * reset.
> >> */
> >> saved_max_pfn = e820_end_of_ram_pfn();
> >> +
> >> + /*
> >> + * We are doing a crash dump, so remove all RAM ranges
> >> + * as they are the ones that need to be dumped.
> >> + * We still need all non-RAM information in order to do I/O.
> >> + */
> >> + e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
> >> + userdef = 1;
> >> + return 0;
> >> #endif
> >> e820.nr_map = 0;
> >> userdef = 1;
> >>
> >
> >The code is still wrong...
> >
> > -hpa
> >
> >
> >--
> >H. Peter Anvin, Intel Open Source Technology Center I work for Intel.
> >I don't speak on their behalf.
>
> --
> Sent from my mobile phone. Please excuse brevity and lack of formatting.
On 11/01/2012 01:49 AM, Zhang, Jun wrote:
> Hello, Anvin
>
> Thank for your advice.
>
> Hello, All
>
> the next patch is made by 2), please review it. Thanks!
>
No, it is not.
You are still modifying the behavior of the kernel depending on
CONFIG_CRASH_DUMP.
CONFIG_CRASH_DUMP doesn't mean "we are doing a crash dump". It means
"it is possible to use this kernel to do a crash dump".
Either you are using standard kernel parameters in a standard way which
is what option 2 was supposed to be -- it should require no kernel
changes! -- or you have to put something in a code path specific to a
crash dump.
-hpa
Hello, Gortmaker
I will modify my subject. Thanks!
Hello, Anvin
from our three options, I think third option is better. But in 3) option, there are two choose, 3.1) is like memmap=REMOVERAM, 3.2) is memmap=CRASHKDUMP.
In 3.1) we maybe need ifdef/endif within the { } of the function (like exactmap).
In 3.2) we can remove the ifdef/endif.
Which one is the better? Maybe you have a better solution, please share it.
Thanks!
Next is our three option.
1) my patch.
2) modify kexec, only pass two parameters -- memmap=544K@64K
memmap=64964K@32768K, in kernel setup_memory_map, we can remove RAM
range.
3) add extra optional, 3.1) like memmap=REMOVERAM
3.2) like memmap=CRASHKDUMP
Best Regards!
Jun Zhang
Inet: 8821-4273
Dir.Tel: 86-21-6116-4273
Email: [email protected]
-----Original Message-----
From: H. Peter Anvin [mailto:[email protected]]
Sent: Saturday, November 03, 2012 12:38 AM
To: Zhang, Jun
Cc: Thomas Gleixner; Ingo Molnar; [email protected]; Andrew Morton; Fleming, Matt; Paul Gortmaker; [email protected]
Subject: Re: [PATCH] To crash dump, we need keep other memory type except E820_RAM, because other type come from BIOS or firmware is used by other code(for example: PCI_MMCONFIG).
On 11/01/2012 01:49 AM, Zhang, Jun wrote:
> Hello, Anvin
>
> Thank for your advice.
>
> Hello, All
>
> the next patch is made by 2), please review it. Thanks!
>
No, it is not.
You are still modifying the behavior of the kernel depending on CONFIG_CRASH_DUMP.
CONFIG_CRASH_DUMP doesn't mean "we are doing a crash dump". It means "it is possible to use this kernel to do a crash dump".
Either you are using standard kernel parameters in a standard way which is what option 2 was supposed to be -- it should require no kernel changes! -- or you have to put something in a code path specific to a crash dump.
-hpa
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On 11/05/2012 02:37 AM, Zhang, Jun wrote:
> Hello, Gortmaker
> I will modify my subject. Thanks!
>
> Hello, Anvin
> from our three options, I think third option is better. But in 3) option, there are two choose, 3.1) is like memmap=REMOVERAM, 3.2) is memmap=CRASHKDUMP.
> In 3.1) we maybe need ifdef/endif within the { } of the function (like exactmap).
> In 3.2) we can remove the ifdef/endif.
> Which one is the better? Maybe you have a better solution, please share it.
> Thanks!
>
> Next is our three option.
> 1) my patch.
> 2) modify kexec, only pass two parameters -- memmap=544K@64K
> memmap=64964K@32768K, in kernel setup_memory_map, we can remove RAM
> range.
> 3) add extra optional, 3.1) like memmap=REMOVERAM
> 3.2) like memmap=CRASHKDUMP
>
Again, 2 would be better because it is a localized change to kexec. If
that works I don't see why there is any reason to change anything else.
-hpa
Hello, Anvin
1) use "memmap=exactmap", which remove all ranges include ram and non-ram range. So my first patch reserve the non-ram range.
2)don't use " memmap=exactmap ", so it reserve all ranges. But we only need non-ram, so we need kernel to remove RAM, just as my second patch.
Best Regards!
Jun Zhang
Inet: 8821-4273
Dir.Tel: 86-21-6116-4273
Email: [email protected]
-----Original Message-----
From: H. Peter Anvin [mailto:[email protected]]
Sent: Monday, November 05, 2012 10:39 AM
To: Zhang, Jun
Cc: Thomas Gleixner; Ingo Molnar; [email protected]; Andrew Morton; Fleming, Matt; Paul Gortmaker; [email protected]
Subject: Re: [PATCH] crash dump: don't delete non-E820_RAM during init
On 11/05/2012 02:37 AM, Zhang, Jun wrote:
> Hello, Gortmaker
> I will modify my subject. Thanks!
>
> Hello, Anvin
> from our three options, I think third option is better. But in 3) option, there are two choose, 3.1) is like memmap=REMOVERAM, 3.2) is memmap=CRASHKDUMP.
> In 3.1) we maybe need ifdef/endif within the { } of the function (like exactmap).
> In 3.2) we can remove the ifdef/endif.
> Which one is the better? Maybe you have a better solution, please share it.
> Thanks!
>
> Next is our three option.
> 1) my patch.
> 2) modify kexec, only pass two parameters -- memmap=544K@64K
> memmap=64964K@32768K, in kernel setup_memory_map, we can remove RAM
> range.
> 3) add extra optional, 3.1) like memmap=REMOVERAM
> 3.2) like memmap=CRASHKDUMP
>
Again, 2 would be better because it is a localized change to kexec. If that works I don't see why there is any reason to change anything else.
-hpa
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On 11/05/2012 03:57 AM, Zhang, Jun wrote:
> Hello, Anvin
> 1) use "memmap=exactmap", which remove all ranges include ram and non-ram range. So my first patch reserve the non-ram range.
> 2)don't use " memmap=exactmap ", so it reserve all ranges. But we only need non-ram, so we need kernel to remove RAM, just as my second patch.
Sorry, you have completely lost me now. You seem to be contradicting
yourself from one message to another.
-hpa