* [PATCH 1/3] crash: Fix memory reserve dead loop bug in reserve_crashkernel_generic()
2024-07-08 13:33 [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation Jinjie Ruan
@ 2024-07-08 13:33 ` Jinjie Ruan
2024-08-05 7:15 ` Linus Walleij
2024-07-08 13:33 ` [PATCH 2/3] ARM: Fix crash kenrel data type bug Jinjie Ruan
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Jinjie Ruan @ 2024-07-08 13:33 UTC (permalink / raw)
To: linux, bhe, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, linus.walleij, eric.devolder, gregkh, deller,
javierm, robh, thunder.leizhen, austindh.kim, linux-arm-kernel,
linux-kernel, kexec
Cc: ruanjinjie
If the platform do not support memory above 4G, such as 32 bit arch,
and CRASH_ADDR_LOW_MAX is equal to CRASH_ADDR_HIGH_MAX, the high
crash kernel memory reservation is meaningless and it will cause
dead loop and system stall:
-> reserve_crashkernel_generic() and high is true
-> memblock_phys_alloc_range() fail and return 0
-> search_end = CRASH_ADDR_LOW_MAX(same as CRASH_ADDR_HIGH_MAX)
-> call memblock_phys_alloc_range() again and fail agin.
-> search_end == CRASH_ADDR_HIGH_MAX satisfy again
......
However, the current check only considers the case where
CRASH_ADDR_HIGH_MAX is greater than CRASH_ADDR_LOW_MAX. Fix it.
Fixes: 0ab97169aa05 ("crash_core: add generic function to do reservation")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
kernel/crash_reserve.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/kernel/crash_reserve.c b/kernel/crash_reserve.c
index 5b2722a93a48..e18fb1bb5d28 100644
--- a/kernel/crash_reserve.c
+++ b/kernel/crash_reserve.c
@@ -390,6 +390,11 @@ void __init reserve_crashkernel_generic(char *cmdline,
} else if (high) {
search_base = CRASH_ADDR_LOW_MAX;
search_end = CRASH_ADDR_HIGH_MAX;
+
+ if (search_base >= search_end) {
+ pr_warn("crashkernel high memory reservation failed.\n");
+ return;
+ }
}
retry:
@@ -410,7 +415,8 @@ void __init reserve_crashkernel_generic(char *cmdline,
* low memory, fall back to high memory, the minimum required
* low memory will be reserved later.
*/
- if (!high && search_end == CRASH_ADDR_LOW_MAX) {
+ if (!high && search_end == CRASH_ADDR_LOW_MAX &&
+ CRASH_ADDR_HIGH_MAX > CRASH_ADDR_LOW_MAX) {
search_end = CRASH_ADDR_HIGH_MAX;
search_base = CRASH_ADDR_LOW_MAX;
crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] crash: Fix memory reserve dead loop bug in reserve_crashkernel_generic()
2024-07-08 13:33 ` [PATCH 1/3] crash: Fix memory reserve dead loop bug in reserve_crashkernel_generic() Jinjie Ruan
@ 2024-08-05 7:15 ` Linus Walleij
0 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2024-08-05 7:15 UTC (permalink / raw)
To: Jinjie Ruan
Cc: linux, bhe, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, eric.devolder, gregkh, deller, javierm, robh,
thunder.leizhen, austindh.kim, linux-arm-kernel, linux-kernel,
kexec
On Mon, Jul 8, 2024 at 3:30 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> If the platform do not support memory above 4G, such as 32 bit arch,
> and CRASH_ADDR_LOW_MAX is equal to CRASH_ADDR_HIGH_MAX, the high
> crash kernel memory reservation is meaningless and it will cause
> dead loop and system stall:
>
> -> reserve_crashkernel_generic() and high is true
> -> memblock_phys_alloc_range() fail and return 0
> -> search_end = CRASH_ADDR_LOW_MAX(same as CRASH_ADDR_HIGH_MAX)
> -> call memblock_phys_alloc_range() again and fail agin.
> -> search_end == CRASH_ADDR_HIGH_MAX satisfy again
> ......
>
> However, the current check only considers the case where
> CRASH_ADDR_HIGH_MAX is greater than CRASH_ADDR_LOW_MAX. Fix it.
>
> Fixes: 0ab97169aa05 ("crash_core: add generic function to do reservation")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Looks good to me:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] ARM: Fix crash kenrel data type bug
2024-07-08 13:33 [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation Jinjie Ruan
2024-07-08 13:33 ` [PATCH 1/3] crash: Fix memory reserve dead loop bug in reserve_crashkernel_generic() Jinjie Ruan
@ 2024-07-08 13:33 ` Jinjie Ruan
2024-08-05 7:14 ` Linus Walleij
2024-07-08 13:33 ` [PATCH 3/3] ARM: Use generic interface to simplify crashkernel reservation Jinjie Ruan
2024-07-09 9:29 ` [PATCH 0/3] " Baoquan He
3 siblings, 1 reply; 14+ messages in thread
From: Jinjie Ruan @ 2024-07-08 13:33 UTC (permalink / raw)
To: linux, bhe, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, linus.walleij, eric.devolder, gregkh, deller,
javierm, robh, thunder.leizhen, austindh.kim, linux-arm-kernel,
linux-kernel, kexec
Cc: ruanjinjie
On QEMU vexpress-a9 with 1GB memory, the crash kernel "crashkernel=4G"
is ok as below:
Reserving 4096MB of memory at 2432MB for crashkernel (System RAM: 1024MB)
The above info is confusing, because the System memory is as below:
# cat /proc/iomem | grep Sys
60000000-9fffffff : System RAM
The cause is that the crash_size is parsed and printed with "unsigned long
long" data type which is 8 bytes but used with "phys_addr_t" which is
4 bytes in memblock_phys_alloc_range().
Fixes: 9d17f3372306 ("ARM: 9190/1: kdump: add invalid input check for 'crashkernel=0'")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
arch/arm/kernel/setup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index e6a857bf0ce6..59e1a13b5cf6 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1012,6 +1012,7 @@ static void __init reserve_crashkernel(void)
&crash_size, &crash_base,
NULL, NULL);
/* invalid value specified or crashkernel=0 */
+ crash_size = (phys_addr_t)crash_size;
if (ret || !crash_size)
return;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] ARM: Fix crash kenrel data type bug
2024-07-08 13:33 ` [PATCH 2/3] ARM: Fix crash kenrel data type bug Jinjie Ruan
@ 2024-08-05 7:14 ` Linus Walleij
0 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2024-08-05 7:14 UTC (permalink / raw)
To: Jinjie Ruan
Cc: linux, bhe, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, eric.devolder, gregkh, deller, javierm, robh,
thunder.leizhen, austindh.kim, linux-arm-kernel, linux-kernel,
kexec
On Mon, Jul 8, 2024 at 3:29 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> On QEMU vexpress-a9 with 1GB memory, the crash kernel "crashkernel=4G"
> is ok as below:
> Reserving 4096MB of memory at 2432MB for crashkernel (System RAM: 1024MB)
>
> The above info is confusing, because the System memory is as below:
> # cat /proc/iomem | grep Sys
> 60000000-9fffffff : System RAM
>
> The cause is that the crash_size is parsed and printed with "unsigned long
> long" data type which is 8 bytes but used with "phys_addr_t" which is
> 4 bytes in memblock_phys_alloc_range().
Is that the whole explanation?
> /* invalid value specified or crashkernel=0 */
> + crash_size = (phys_addr_t)crash_size;
> if (ret || !crash_size)
> return;
How did crash_size get assigned before you added this oneliner?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: Use generic interface to simplify crashkernel reservation
2024-07-08 13:33 [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation Jinjie Ruan
2024-07-08 13:33 ` [PATCH 1/3] crash: Fix memory reserve dead loop bug in reserve_crashkernel_generic() Jinjie Ruan
2024-07-08 13:33 ` [PATCH 2/3] ARM: Fix crash kenrel data type bug Jinjie Ruan
@ 2024-07-08 13:33 ` Jinjie Ruan
2024-07-09 9:29 ` [PATCH 0/3] " Baoquan He
3 siblings, 0 replies; 14+ messages in thread
From: Jinjie Ruan @ 2024-07-08 13:33 UTC (permalink / raw)
To: linux, bhe, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, linus.walleij, eric.devolder, gregkh, deller,
javierm, robh, thunder.leizhen, austindh.kim, linux-arm-kernel,
linux-kernel, kexec
Cc: ruanjinjie
Currently, x86, arm64, riscv and loongarch has been switched to generic
crashkernel reservation. With the help of function parse_crashkernel()
and generic reserve_crashkernel_generic(), arm32 crashkernel reservation
can also be simplified by steps:
1) Add a new header file <asm/crash_reserve.h>, and define CRASH_ALIGN,
CRASH_ADDR_LOW_MAX, CRASH_ADDR_HIGH_MAX in it;
2) Add arch_reserve_crashkernel() to call parse_crashkernel() and
reserve_crashkernel_generic();
3) Add ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION Kconfig in
arch/arm/Kconfig.
The old reserve_crashkernel() can be removed.
Following test cases have been performed as expected on QEMU vexpress-a9
(1GB system memory):
1) crashkernel=4G,high // invalid
2) crashkernel=1G,high // invalid
3) crashkernel=1G,high crashkernel=0M,low // invalid
4) crashkernel=256M,high // invalid
5) crashkernel=256M,low // invalid
6) crashkernel=256M crashkernel=256M,high // high is ignored, ok
7) crashkernel=256M crashkernel=256M,low // low is ignored, ok
8) crashkernel=256M,high crashkernel=256M,low // invalid
9) crashkernel=256M,high crashkernel=4G,low // invalid
10) crashkernel=256M // ok
11) crashkernel=512M // ok
12) crashkernel=256M@0x88000000 // ok
13) crashkernel=256M@0x78000000 // ok
14) crashkernel=512M@0x78000000 // ok
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
arch/arm/Kconfig | 3 ++
arch/arm/include/asm/crash_reserve.h | 24 +++++++++++
arch/arm/kernel/setup.c | 62 ++++------------------------
3 files changed, 36 insertions(+), 53 deletions(-)
create mode 100644 arch/arm/include/asm/crash_reserve.h
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 68990e1645d5..02e620e185c0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1596,6 +1596,9 @@ config ATAGS_PROC
config ARCH_SUPPORTS_CRASH_DUMP
def_bool y
+config ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
+ def_bool CRASH_RESERVE
+
config AUTO_ZRELADDR
bool "Auto calculation of the decompressed kernel image address" if !ARCH_MULTIPLATFORM
default !(ARCH_FOOTBRIDGE || ARCH_RPC || ARCH_SA1100)
diff --git a/arch/arm/include/asm/crash_reserve.h b/arch/arm/include/asm/crash_reserve.h
new file mode 100644
index 000000000000..85c9298bd3b7
--- /dev/null
+++ b/arch/arm/include/asm/crash_reserve.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ARM_CRASH_RESERVE_H
+#define _ARM_CRASH_RESERVE_H
+
+/*
+ * The crash region must be aligned to 128MB to avoid
+ * zImage relocating below the reserved region.
+ */
+#define CRASH_ALIGN (128 << 20)
+
+#define CRASH_ADDR_LOW_MAX crash_addr_low_max()
+#define CRASH_ADDR_HIGH_MAX memblock_end_of_DRAM()
+
+static inline unsigned long crash_addr_low_max(void)
+{
+ unsigned long long crash_max = idmap_to_phys((u32)~0);
+ unsigned long long lowmem_max = __pa(high_memory - 1) + 1;
+
+ return (crash_max > lowmem_max) ? lowmem_max : crash_max;
+}
+
+
+#define HAVE_ARCH_ADD_CRASH_RES_TO_IOMEM_EARLY
+#endif
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 59e1a13b5cf6..3a8b6f08f4ec 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -979,13 +979,6 @@ static int __init init_machine_late(void)
}
late_initcall(init_machine_late);
-#ifdef CONFIG_CRASH_RESERVE
-/*
- * The crash region must be aligned to 128MB to avoid
- * zImage relocating below the reserved region.
- */
-#define CRASH_ALIGN (128 << 20)
-
static inline unsigned long long get_total_mem(void)
{
unsigned long total;
@@ -994,61 +987,27 @@ static inline unsigned long long get_total_mem(void)
return total << PAGE_SHIFT;
}
-/**
- * reserve_crashkernel() - reserves memory are for crash kernel
- *
- * This function reserves memory area given in "crashkernel=" kernel command
- * line parameter. The memory reserved is used by a dump capture kernel when
- * primary kernel is crashing.
- */
-static void __init reserve_crashkernel(void)
+static void __init arch_reserve_crashkernel(void)
{
unsigned long long crash_size, crash_base;
+ unsigned long long low_size = 0;
unsigned long long total_mem;
+ bool high = false;
int ret;
+ if (!IS_ENABLED(CONFIG_CRASH_RESERVE))
+ return;
+
total_mem = get_total_mem();
ret = parse_crashkernel(boot_command_line, total_mem,
&crash_size, &crash_base,
- NULL, NULL);
+ &low_size, &high);
/* invalid value specified or crashkernel=0 */
crash_size = (phys_addr_t)crash_size;
if (ret || !crash_size)
return;
- if (crash_base <= 0) {
- unsigned long long crash_max = idmap_to_phys((u32)~0);
- unsigned long long lowmem_max = __pa(high_memory - 1) + 1;
- if (crash_max > lowmem_max)
- crash_max = lowmem_max;
-
- crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN,
- CRASH_ALIGN, crash_max);
- if (!crash_base) {
- pr_err("crashkernel reservation failed - No suitable area found.\n");
- return;
- }
- } else {
- unsigned long long crash_max = crash_base + crash_size;
- unsigned long long start;
-
- start = memblock_phys_alloc_range(crash_size, SECTION_SIZE,
- crash_base, crash_max);
- if (!start) {
- pr_err("crashkernel reservation failed - memory is in use.\n");
- return;
- }
- }
-
- pr_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)(total_mem >> 20));
-
- /* The crashk resource must always be located in normal mem */
- crashk_res.start = crash_base;
- crashk_res.end = crash_base + crash_size - 1;
- insert_resource(&iomem_resource, &crashk_res);
+ reserve_crashkernel_generic(boot_command_line, crash_size, crash_base, low_size, high);
if (arm_has_idmap_alias()) {
/*
@@ -1065,9 +1024,6 @@ static void __init reserve_crashkernel(void)
insert_resource(&iomem_resource, &crashk_boot_res);
}
}
-#else
-static inline void reserve_crashkernel(void) {}
-#endif /* CONFIG_CRASH_RESERVE*/
void __init hyp_mode_check(void)
{
@@ -1190,7 +1146,7 @@ void __init setup_arch(char **cmdline_p)
if (!is_smp())
hyp_mode_check();
- reserve_crashkernel();
+ arch_reserve_crashkernel();
#ifdef CONFIG_VT
#if defined(CONFIG_VGA_CONSOLE)
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation
2024-07-08 13:33 [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation Jinjie Ruan
` (2 preceding siblings ...)
2024-07-08 13:33 ` [PATCH 3/3] ARM: Use generic interface to simplify crashkernel reservation Jinjie Ruan
@ 2024-07-09 9:29 ` Baoquan He
2024-07-09 9:50 ` Jinjie Ruan
2024-07-11 6:30 ` Jinjie Ruan
3 siblings, 2 replies; 14+ messages in thread
From: Baoquan He @ 2024-07-09 9:29 UTC (permalink / raw)
To: Jinjie Ruan
Cc: linux, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, linus.walleij, eric.devolder, gregkh, deller,
javierm, robh, thunder.leizhen, austindh.kim, linux-arm-kernel,
linux-kernel, kexec
On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> Currently, x86, arm64, riscv and loongarch has been switched to generic
> crashkernel reservation. Also use generic interface to simplify crashkernel
> reservation for arm32, and fix two bugs by the way.
I am not sure if this is a good idea. I added the generic reservation
itnerfaces for ARCH which support crashkernel=,high|low and normal
crashkernel reservation, with this, the code can be simplified a lot.
However, arm32 doesn't support crashkernel=,high, I am not sure if it's
worth taking the change, most importantly, if it will cause
misunderstanding or misoperation.
Thanks
Baoquan
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation
2024-07-09 9:29 ` [PATCH 0/3] " Baoquan He
@ 2024-07-09 9:50 ` Jinjie Ruan
2024-07-09 10:39 ` Baoquan He
2024-07-11 6:30 ` Jinjie Ruan
1 sibling, 1 reply; 14+ messages in thread
From: Jinjie Ruan @ 2024-07-09 9:50 UTC (permalink / raw)
To: Baoquan He
Cc: linux, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, linus.walleij, eric.devolder, gregkh, deller,
javierm, robh, thunder.leizhen, austindh.kim, linux-arm-kernel,
linux-kernel, kexec
On 2024/7/9 17:29, Baoquan He wrote:
> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>> crashkernel reservation. Also use generic interface to simplify crashkernel
>> reservation for arm32, and fix two bugs by the way.
>
> I am not sure if this is a good idea. I added the generic reservation
> itnerfaces for ARCH which support crashkernel=,high|low and normal
> crashkernel reservation, with this, the code can be simplified a lot.
> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> worth taking the change, most importantly, if it will cause
> misunderstanding or misoperation.
Yes, arm32 doesn't support crashkernel=,high.
However, a little enhancement to the generic code (please see the first
patch), the generic reservation interfaces can also be applicable to
architectures that do not support "high" such as arm32, and it can also
simplify the code (please see the third patch).
>
> Thanks
> Baoquan
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation
2024-07-09 9:50 ` Jinjie Ruan
@ 2024-07-09 10:39 ` Baoquan He
2024-07-09 11:06 ` Jinjie Ruan
0 siblings, 1 reply; 14+ messages in thread
From: Baoquan He @ 2024-07-09 10:39 UTC (permalink / raw)
To: Jinjie Ruan
Cc: linux, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, linus.walleij, eric.devolder, gregkh, deller,
javierm, robh, thunder.leizhen, austindh.kim, linux-arm-kernel,
linux-kernel, kexec
On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
>
>
> On 2024/7/9 17:29, Baoquan He wrote:
> > On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> >> Currently, x86, arm64, riscv and loongarch has been switched to generic
> >> crashkernel reservation. Also use generic interface to simplify crashkernel
> >> reservation for arm32, and fix two bugs by the way.
> >
> > I am not sure if this is a good idea. I added the generic reservation
> > itnerfaces for ARCH which support crashkernel=,high|low and normal
> > crashkernel reservation, with this, the code can be simplified a lot.
> > However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> > worth taking the change, most importantly, if it will cause
> > misunderstanding or misoperation.
>
> Yes, arm32 doesn't support crashkernel=,high.
>
> However, a little enhancement to the generic code (please see the first
> patch), the generic reservation interfaces can also be applicable to
> architectures that do not support "high" such as arm32, and it can also
> simplify the code (please see the third patch).
Yeah, I can see the code is simplified. When you specified
'crashkernel=xM,high', do you think what should be warn out? Because
it's an unsupported syntax on arm32, we should do something to print out
appropriate message.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation
2024-07-09 10:39 ` Baoquan He
@ 2024-07-09 11:06 ` Jinjie Ruan
2024-07-09 14:06 ` Baoquan He
0 siblings, 1 reply; 14+ messages in thread
From: Jinjie Ruan @ 2024-07-09 11:06 UTC (permalink / raw)
To: Baoquan He
Cc: linux, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, linus.walleij, eric.devolder, gregkh, deller,
javierm, robh, thunder.leizhen, austindh.kim, linux-arm-kernel,
linux-kernel, kexec
On 2024/7/9 18:39, Baoquan He wrote:
> On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
>>
>>
>> On 2024/7/9 17:29, Baoquan He wrote:
>>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>>>> crashkernel reservation. Also use generic interface to simplify crashkernel
>>>> reservation for arm32, and fix two bugs by the way.
>>>
>>> I am not sure if this is a good idea. I added the generic reservation
>>> itnerfaces for ARCH which support crashkernel=,high|low and normal
>>> crashkernel reservation, with this, the code can be simplified a lot.
>>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
>>> worth taking the change, most importantly, if it will cause
>>> misunderstanding or misoperation.
>>
>> Yes, arm32 doesn't support crashkernel=,high.
>>
>> However, a little enhancement to the generic code (please see the first
>> patch), the generic reservation interfaces can also be applicable to
>> architectures that do not support "high" such as arm32, and it can also
>> simplify the code (please see the third patch).
>
> Yeah, I can see the code is simplified. When you specified
> 'crashkernel=xM,high', do you think what should be warn out? Because
> it's an unsupported syntax on arm32, we should do something to print out
> appropriate message.
Yes, you are right! In this patch it will print "crashkernel high memory
reservation failed." message and out for arm32 if you specify
'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
"CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
out for other similar architecture.
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation
2024-07-09 11:06 ` Jinjie Ruan
@ 2024-07-09 14:06 ` Baoquan He
2024-07-10 1:52 ` Jinjie Ruan
0 siblings, 1 reply; 14+ messages in thread
From: Baoquan He @ 2024-07-09 14:06 UTC (permalink / raw)
To: Jinjie Ruan
Cc: linux, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, linus.walleij, eric.devolder, gregkh, deller,
javierm, robh, thunder.leizhen, austindh.kim, linux-arm-kernel,
linux-kernel, kexec
On 07/09/24 at 07:06pm, Jinjie Ruan wrote:
>
>
> On 2024/7/9 18:39, Baoquan He wrote:
> > On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
> >>
> >>
> >> On 2024/7/9 17:29, Baoquan He wrote:
> >>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> >>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
> >>>> crashkernel reservation. Also use generic interface to simplify crashkernel
> >>>> reservation for arm32, and fix two bugs by the way.
> >>>
> >>> I am not sure if this is a good idea. I added the generic reservation
> >>> itnerfaces for ARCH which support crashkernel=,high|low and normal
> >>> crashkernel reservation, with this, the code can be simplified a lot.
> >>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> >>> worth taking the change, most importantly, if it will cause
> >>> misunderstanding or misoperation.
> >>
> >> Yes, arm32 doesn't support crashkernel=,high.
> >>
> >> However, a little enhancement to the generic code (please see the first
> >> patch), the generic reservation interfaces can also be applicable to
> >> architectures that do not support "high" such as arm32, and it can also
> >> simplify the code (please see the third patch).
> >
> > Yeah, I can see the code is simplified. When you specified
> > 'crashkernel=xM,high', do you think what should be warn out? Because
> > it's an unsupported syntax on arm32, we should do something to print out
> > appropriate message.
>
> Yes, you are right! In this patch it will print "crashkernel high memory
> reservation failed." message and out for arm32 if you specify
That message may mislead people to believe crashkernel=,high is
supported but reservation is failed, then a bug need be filed for this?
We may expect a message telling this syntax is not supported on this
ARCH.
> 'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
> "CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
> out for other similar architecture.
>
>
> >
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation
2024-07-09 14:06 ` Baoquan He
@ 2024-07-10 1:52 ` Jinjie Ruan
2024-07-10 3:40 ` Baoquan He
0 siblings, 1 reply; 14+ messages in thread
From: Jinjie Ruan @ 2024-07-10 1:52 UTC (permalink / raw)
To: Baoquan He
Cc: linux, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, linus.walleij, eric.devolder, gregkh, deller,
javierm, robh, thunder.leizhen, austindh.kim, linux-arm-kernel,
linux-kernel, kexec
On 2024/7/9 22:06, Baoquan He wrote:
> On 07/09/24 at 07:06pm, Jinjie Ruan wrote:
>>
>>
>> On 2024/7/9 18:39, Baoquan He wrote:
>>> On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
>>>>
>>>>
>>>> On 2024/7/9 17:29, Baoquan He wrote:
>>>>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>>>>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>>>>>> crashkernel reservation. Also use generic interface to simplify crashkernel
>>>>>> reservation for arm32, and fix two bugs by the way.
>>>>>
>>>>> I am not sure if this is a good idea. I added the generic reservation
>>>>> itnerfaces for ARCH which support crashkernel=,high|low and normal
>>>>> crashkernel reservation, with this, the code can be simplified a lot.
>>>>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
>>>>> worth taking the change, most importantly, if it will cause
>>>>> misunderstanding or misoperation.
>>>>
>>>> Yes, arm32 doesn't support crashkernel=,high.
>>>>
>>>> However, a little enhancement to the generic code (please see the first
>>>> patch), the generic reservation interfaces can also be applicable to
>>>> architectures that do not support "high" such as arm32, and it can also
>>>> simplify the code (please see the third patch).
>>>
>>> Yeah, I can see the code is simplified. When you specified
>>> 'crashkernel=xM,high', do you think what should be warn out? Because
>>> it's an unsupported syntax on arm32, we should do something to print out
>>> appropriate message.
>>
>> Yes, you are right! In this patch it will print "crashkernel high memory
>> reservation failed." message and out for arm32 if you specify
>
> That message may mislead people to believe crashkernel=,high is
> supported but reservation is failed, then a bug need be filed for this?
> We may expect a message telling this syntax is not supported on this
> ARCH.
"CRASH_ADDR_LOW_MAX >= CRASH_ADDR_HIGH_MAX" indicate that the arm32 does
not support "crashkernel=,high", I wonder if this is generic for similar
architecture. If so, the first patch can print such as
"crashkernel=,high is not supported on this ARCH" message.
>
>> 'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
>> "CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
>> out for other similar architecture.
>>
>>
>>>
>>>
>>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation
2024-07-10 1:52 ` Jinjie Ruan
@ 2024-07-10 3:40 ` Baoquan He
0 siblings, 0 replies; 14+ messages in thread
From: Baoquan He @ 2024-07-10 3:40 UTC (permalink / raw)
To: Jinjie Ruan
Cc: linux, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, linus.walleij, eric.devolder, gregkh, deller,
javierm, robh, thunder.leizhen, austindh.kim, linux-arm-kernel,
linux-kernel, kexec
On 07/10/24 at 09:52am, Jinjie Ruan wrote:
>
>
> On 2024/7/9 22:06, Baoquan He wrote:
> > On 07/09/24 at 07:06pm, Jinjie Ruan wrote:
> >>
> >>
> >> On 2024/7/9 18:39, Baoquan He wrote:
> >>> On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
> >>>>
> >>>>
> >>>> On 2024/7/9 17:29, Baoquan He wrote:
> >>>>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> >>>>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
> >>>>>> crashkernel reservation. Also use generic interface to simplify crashkernel
> >>>>>> reservation for arm32, and fix two bugs by the way.
> >>>>>
> >>>>> I am not sure if this is a good idea. I added the generic reservation
> >>>>> itnerfaces for ARCH which support crashkernel=,high|low and normal
> >>>>> crashkernel reservation, with this, the code can be simplified a lot.
> >>>>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> >>>>> worth taking the change, most importantly, if it will cause
> >>>>> misunderstanding or misoperation.
> >>>>
> >>>> Yes, arm32 doesn't support crashkernel=,high.
> >>>>
> >>>> However, a little enhancement to the generic code (please see the first
> >>>> patch), the generic reservation interfaces can also be applicable to
> >>>> architectures that do not support "high" such as arm32, and it can also
> >>>> simplify the code (please see the third patch).
> >>>
> >>> Yeah, I can see the code is simplified. When you specified
> >>> 'crashkernel=xM,high', do you think what should be warn out? Because
> >>> it's an unsupported syntax on arm32, we should do something to print out
> >>> appropriate message.
> >>
> >> Yes, you are right! In this patch it will print "crashkernel high memory
> >> reservation failed." message and out for arm32 if you specify
> >
> > That message may mislead people to believe crashkernel=,high is
> > supported but reservation is failed, then a bug need be filed for this?
> > We may expect a message telling this syntax is not supported on this
> > ARCH.
>
> "CRASH_ADDR_LOW_MAX >= CRASH_ADDR_HIGH_MAX" indicate that the arm32 does
> not support "crashkernel=,high", I wonder if this is generic for similar
Imagine you are a testing engineer or a distros user, how do you know
if "CRASH_ADDR_LOW_MAX >= CRASH_ADDR_HIGH_MAX" when you test
'crashkernel=,high' and see the failure message?
> architecture. If so, the first patch can print such as
> "crashkernel=,high is not supported on this ARCH" message.
Please consider conprehensively if this is doable, you can paste
draft code here to prove it.
>
> >
> >> 'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
> >> "CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
> >> out for other similar architecture.
> >>
> >>
> >>>
> >>>
> >>
> >
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation
2024-07-09 9:29 ` [PATCH 0/3] " Baoquan He
2024-07-09 9:50 ` Jinjie Ruan
@ 2024-07-11 6:30 ` Jinjie Ruan
1 sibling, 0 replies; 14+ messages in thread
From: Jinjie Ruan @ 2024-07-11 6:30 UTC (permalink / raw)
To: Baoquan He
Cc: linux, vgoyal, dyoung, paul.walmsley, palmer, aou, arnd, afd,
akpm, rmk+kernel, linus.walleij, eric.devolder, gregkh, deller,
javierm, robh, thunder.leizhen, austindh.kim, linux-arm-kernel,
linux-kernel, kexec
On 2024/7/9 17:29, Baoquan He wrote:
> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>> crashkernel reservation. Also use generic interface to simplify crashkernel
>> reservation for arm32, and fix two bugs by the way.
>
> I am not sure if this is a good idea. I added the generic reservation
> itnerfaces for ARCH which support crashkernel=,high|low and normal
> crashkernel reservation, with this, the code can be simplified a lot.
> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> worth taking the change, most importantly, if it will cause
> misunderstanding or misoperation.
It seems that x86_32 doesn't support crashkernel=,high and use the
generic interface，and also have the first patch bug. I’ll resend the
first patch and explain it.
>
> Thanks
> Baoquan
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread