* [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
@ 2022-07-11 9:03 Zhen Lei
2022-07-11 9:03 ` [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified Zhen Lei
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: Zhen Lei @ 2022-07-11 9:03 UTC (permalink / raw)
To: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Catalin Marinas, Will Deacon, linux-arm-kernel, Jonathan Corbet,
linux-doc
Cc: Zhen Lei, Eric W . Biederman, Randy Dunlap, Feng Zhou,
Kefeng Wang, Chen Zhou, John Donnelly, Dave Kleikamp
v2 --> v3:
1. Discard patch 3 in v2, a cleanup patch.
v1 --> v2:
1. Update the commit message of Patch 1, explicitly indicates that "crashkernel=X,high"
is specified but "crashkernel=Y,low" is not specified.
2. Drop Patch 4-5. Currently, focus on function integrity, performance optimization
will be considered in later versions.
3. Patch 3 is not mandatory, it's just a cleanup now, although it is a must for patch 4-5.
But to avoid subsequent duplication of effort, I'm glad it was accepted.
v1:
After the basic functions of "support reserving crashkernel above 4G on arm64
kdump"(see https://lkml.org/lkml/2022/5/6/428) are implemented, we still have
three features to be improved.
1. When crashkernel=X,high is specified but crashkernel=Y,low is not specified,
the default crash low memory size is provided.
2. For crashkernel=X without '@offset', if the low memory fails to be allocated,
fall back to reserve region from high memory(above DMA zones).
3. If crashkernel=X,high is used, page mapping is performed only for the crash
high memory, and block mapping is still used for other linear address spaces.
Compared to the previous version:
(1) For crashkernel=X[@offset], the memory above 4G is not changed to block
mapping, leave it to the next time.
(2) The implementation method is modified. Now the implementation is simpler
and clearer.
Zhen Lei (2):
arm64: kdump: Provide default size when crashkernel=Y,low is not
specified
arm64: kdump: Support crashkernel=X fall back to reserve region above
DMA zones
.../admin-guide/kernel-parameters.txt | 10 ++-----
arch/arm64/mm/init.c | 28 +++++++++++++++++--
2 files changed, 28 insertions(+), 10 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified
2022-07-11 9:03 [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization Zhen Lei
@ 2022-07-11 9:03 ` Zhen Lei
2022-08-02 8:37 ` Will Deacon
` (2 more replies)
2022-07-11 9:03 ` [PATCH v3 2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones Zhen Lei
` (2 subsequent siblings)
3 siblings, 3 replies; 23+ messages in thread
From: Zhen Lei @ 2022-07-11 9:03 UTC (permalink / raw)
To: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Catalin Marinas, Will Deacon, linux-arm-kernel, Jonathan Corbet,
linux-doc
Cc: Zhen Lei, Eric W . Biederman, Randy Dunlap, Feng Zhou,
Kefeng Wang, Chen Zhou, John Donnelly, Dave Kleikamp
To be consistent with the implementation of x86 and improve cross-platform
user experience. Try to allocate at least 256 MiB low memory automatically
for the case that crashkernel=,high is explicitly specified, while
crashkenrel=,low is omitted.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
Documentation/admin-guide/kernel-parameters.txt | 8 +-------
arch/arm64/mm/init.c | 12 +++++++++++-
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2522b11e593f239..65a2c3a22a4b57d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -843,7 +843,7 @@
available.
It will be ignored if crashkernel=X is specified.
crashkernel=size[KMG],low
- [KNL, X86-64] range under 4G. When crashkernel=X,high
+ [KNL, X86-64, ARM64] range under 4G. When crashkernel=X,high
is passed, kernel could allocate physical memory region
above 4G, that cause second kernel crash on system
that require some amount of low memory, e.g. swiotlb
@@ -857,12 +857,6 @@
It will be ignored when crashkernel=X,high is not used
or memory reserved is below 4G.
- [KNL, ARM64] range in low memory.
- This one lets the user specify a low range in the
- DMA zone for the crash dump kernel.
- It will be ignored when crashkernel=X,high is not used
- or memory reserved is located in the DMA zones.
-
cryptomgr.notests
[KNL] Disable crypto self-tests
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 339ee84e5a61a0b..5390f361208ccf7 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -96,6 +96,14 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit = PHYS_MASK + 1;
#define CRASH_ADDR_LOW_MAX arm64_dma_phys_limit
#define CRASH_ADDR_HIGH_MAX (PHYS_MASK + 1)
+/*
+ * This is an empirical value in x86_64 and taken here directly. Please
+ * refer to the code comment in reserve_crashkernel_low() of x86_64 for more
+ * details.
+ */
+#define DEFAULT_CRASH_KERNEL_LOW_SIZE \
+ max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20)
+
static int __init reserve_crashkernel_low(unsigned long long low_size)
{
unsigned long long low_base;
@@ -147,7 +155,9 @@ static void __init reserve_crashkernel(void)
* is not allowed.
*/
ret = parse_crashkernel_low(cmdline, 0, &crash_low_size, &crash_base);
- if (ret && (ret != -ENOENT))
+ if (ret == -ENOENT)
+ crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
+ else if (ret)
return;
crash_max = CRASH_ADDR_HIGH_MAX;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones
2022-07-11 9:03 [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization Zhen Lei
2022-07-11 9:03 ` [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified Zhen Lei
@ 2022-07-11 9:03 ` Zhen Lei
2022-11-07 17:13 ` Catalin Marinas
2022-08-01 8:20 ` [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization Baoquan He
2022-11-15 11:58 ` Will Deacon
3 siblings, 1 reply; 23+ messages in thread
From: Zhen Lei @ 2022-07-11 9:03 UTC (permalink / raw)
To: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Catalin Marinas, Will Deacon, linux-arm-kernel, Jonathan Corbet,
linux-doc
Cc: Zhen Lei, Eric W . Biederman, Randy Dunlap, Feng Zhou,
Kefeng Wang, Chen Zhou, John Donnelly, Dave Kleikamp
For crashkernel=X without '@offset', select a region within DMA zones
first, and fall back to reserve region above DMA zones. This allows
users to use the same configuration on multiple platforms.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
Documentation/admin-guide/kernel-parameters.txt | 2 +-
arch/arm64/mm/init.c | 16 +++++++++++++++-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 65a2c3a22a4b57d..cb6a346419a1fe0 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -823,7 +823,7 @@
memory region [offset, offset + size] for that kernel
image. If '@offset' is omitted, then a suitable offset
is selected automatically.
- [KNL, X86-64] Select a region under 4G first, and
+ [KNL, X86-64, ARM64] Select a region under 4G first, and
fall back to reserve region above 4G when '@offset'
hasn't been specified.
See Documentation/admin-guide/kdump/kdump.rst for further details.
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 5390f361208ccf7..8539598f9e58b4d 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -138,6 +138,7 @@ static void __init reserve_crashkernel(void)
unsigned long long crash_max = CRASH_ADDR_LOW_MAX;
char *cmdline = boot_command_line;
int ret;
+ bool fixed_base;
if (!IS_ENABLED(CONFIG_KEXEC_CORE))
return;
@@ -166,15 +167,28 @@ static void __init reserve_crashkernel(void)
return;
}
+ fixed_base = !!crash_base;
crash_size = PAGE_ALIGN(crash_size);
/* User specifies base address explicitly. */
- if (crash_base)
+ if (fixed_base)
crash_max = crash_base + crash_size;
+retry:
crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN,
crash_base, crash_max);
if (!crash_base) {
+ /*
+ * Attempt to fully allocate low memory failed, fall back
+ * to high memory, the minimum required low memory will be
+ * reserved later.
+ */
+ if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) {
+ crash_max = CRASH_ADDR_HIGH_MAX;
+ crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
+ goto retry;
+ }
+
pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
crash_size);
return;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
2022-07-11 9:03 [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization Zhen Lei
2022-07-11 9:03 ` [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified Zhen Lei
2022-07-11 9:03 ` [PATCH v3 2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones Zhen Lei
@ 2022-08-01 8:20 ` Baoquan He
2022-08-02 2:47 ` Leizhen (ThunderTown)
2022-11-15 11:58 ` Will Deacon
3 siblings, 1 reply; 23+ messages in thread
From: Baoquan He @ 2022-08-01 8:20 UTC (permalink / raw)
To: Zhen Lei
Cc: Dave Young, Vivek Goyal, kexec, linux-kernel, Catalin Marinas,
Will Deacon, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
Hi Catalin,
On 07/11/22 at 05:03pm, Zhen Lei wrote:
> v2 --> v3:
> 1. Discard patch 3 in v2, a cleanup patch.
>
> v1 --> v2:
> 1. Update the commit message of Patch 1, explicitly indicates that "crashkernel=X,high"
> is specified but "crashkernel=Y,low" is not specified.
> 2. Drop Patch 4-5. Currently, focus on function integrity, performance optimization
> will be considered in later versions.
> 3. Patch 3 is not mandatory, it's just a cleanup now, although it is a must for patch 4-5.
> But to avoid subsequent duplication of effort, I'm glad it was accepted.
>
>
> v1:
> After the basic functions of "support reserving crashkernel above 4G on arm64
> kdump"(see https://lkml.org/lkml/2022/5/6/428) are implemented, we still have
> three features to be improved.
> 1. When crashkernel=X,high is specified but crashkernel=Y,low is not specified,
> the default crash low memory size is provided.
> 2. For crashkernel=X without '@offset', if the low memory fails to be allocated,
> fall back to reserve region from high memory(above DMA zones).
> 3. If crashkernel=X,high is used, page mapping is performed only for the crash
> high memory, and block mapping is still used for other linear address spaces.
> Compared to the previous version:
> (1) For crashkernel=X[@offset], the memory above 4G is not changed to block
> mapping, leave it to the next time.
> (2) The implementation method is modified. Now the implementation is simpler
> and clearer.
Do you have plan to pick this series so that it can be taken into 5.20
rc-1~3?
We have back ported the basic crashkernel=high, low, support into our
distros and have taken wide testing on arm64 servers, need this patchset
to back port for more testing.
Thanks
Baoquan
>
> Zhen Lei (2):
> arm64: kdump: Provide default size when crashkernel=Y,low is not
> specified
> arm64: kdump: Support crashkernel=X fall back to reserve region above
> DMA zones
>
> .../admin-guide/kernel-parameters.txt | 10 ++-----
> arch/arm64/mm/init.c | 28 +++++++++++++++++--
> 2 files changed, 28 insertions(+), 10 deletions(-)
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
2022-08-01 8:20 ` [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization Baoquan He
@ 2022-08-02 2:47 ` Leizhen (ThunderTown)
2022-10-06 14:55 ` john.p.donnelly
0 siblings, 1 reply; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2022-08-02 2:47 UTC (permalink / raw)
To: Baoquan He
Cc: Dave Young, Vivek Goyal, kexec, linux-kernel, Catalin Marinas,
Will Deacon, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
On 2022/8/1 16:20, Baoquan He wrote:
> Hi Catalin,
>
> On 07/11/22 at 05:03pm, Zhen Lei wrote:
>> v2 --> v3:
>> 1. Discard patch 3 in v2, a cleanup patch.
>>
>> v1 --> v2:
>> 1. Update the commit message of Patch 1, explicitly indicates that "crashkernel=X,high"
>> is specified but "crashkernel=Y,low" is not specified.
>> 2. Drop Patch 4-5. Currently, focus on function integrity, performance optimization
>> will be considered in later versions.
>> 3. Patch 3 is not mandatory, it's just a cleanup now, although it is a must for patch 4-5.
>> But to avoid subsequent duplication of effort, I'm glad it was accepted.
>>
>>
>> v1:
>> After the basic functions of "support reserving crashkernel above 4G on arm64
>> kdump"(see https://lkml.org/lkml/2022/5/6/428) are implemented, we still have
>> three features to be improved.
>> 1. When crashkernel=X,high is specified but crashkernel=Y,low is not specified,
>> the default crash low memory size is provided.
>> 2. For crashkernel=X without '@offset', if the low memory fails to be allocated,
>> fall back to reserve region from high memory(above DMA zones).
>> 3. If crashkernel=X,high is used, page mapping is performed only for the crash
>> high memory, and block mapping is still used for other linear address spaces.
>> Compared to the previous version:
>> (1) For crashkernel=X[@offset], the memory above 4G is not changed to block
>> mapping, leave it to the next time.
>> (2) The implementation method is modified. Now the implementation is simpler
>> and clearer.
>
> Do you have plan to pick this series so that it can be taken into 5.20
> rc-1~3?
Hi, Catalin:
Only function reserve_crashkernel() is modified in these two patches. The core
process of the arm64 architecture is not affected. I remember you suggested that
arm64 and x86 share the same kdump code, so these two subfeatures are needed.
Maybe we can lay the foundation first for the people who build the road. Unifying
the external interfaces of kdump on arm64 and x86 does not seem to hurt.
>
> We have back ported the basic crashkernel=high, low, support into our
> distros and have taken wide testing on arm64 servers, need this patchset
> to back port for more testing.
>
> Thanks
> Baoquan
>
>>
>> Zhen Lei (2):
>> arm64: kdump: Provide default size when crashkernel=Y,low is not
>> specified
>> arm64: kdump: Support crashkernel=X fall back to reserve region above
>> DMA zones
>>
>> .../admin-guide/kernel-parameters.txt | 10 ++-----
>> arch/arm64/mm/init.c | 28 +++++++++++++++++--
>> 2 files changed, 28 insertions(+), 10 deletions(-)
>>
>> --
>> 2.25.1
>>
>
> .
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified
2022-07-11 9:03 ` [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified Zhen Lei
@ 2022-08-02 8:37 ` Will Deacon
2022-08-02 10:12 ` Leizhen (ThunderTown)
2022-11-07 14:07 ` Catalin Marinas
2022-11-07 17:18 ` Catalin Marinas
2 siblings, 1 reply; 23+ messages in thread
From: Will Deacon @ 2022-08-02 8:37 UTC (permalink / raw)
To: Zhen Lei
Cc: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Catalin Marinas, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp, ardb
On Mon, Jul 11, 2022 at 05:03:18PM +0800, Zhen Lei wrote:
> To be consistent with the implementation of x86 and improve cross-platform
> user experience. Try to allocate at least 256 MiB low memory automatically
> for the case that crashkernel=,high is explicitly specified, while
> crashkenrel=,low is omitted.
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> Acked-by: Baoquan He <bhe@redhat.com>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 8 +-------
> arch/arm64/mm/init.c | 12 +++++++++++-
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2522b11e593f239..65a2c3a22a4b57d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -843,7 +843,7 @@
> available.
> It will be ignored if crashkernel=X is specified.
> crashkernel=size[KMG],low
> - [KNL, X86-64] range under 4G. When crashkernel=X,high
> + [KNL, X86-64, ARM64] range under 4G. When crashkernel=X,high
> is passed, kernel could allocate physical memory region
> above 4G, that cause second kernel crash on system
> that require some amount of low memory, e.g. swiotlb
> @@ -857,12 +857,6 @@
> It will be ignored when crashkernel=X,high is not used
> or memory reserved is below 4G.
>
> - [KNL, ARM64] range in low memory.
> - This one lets the user specify a low range in the
> - DMA zone for the crash dump kernel.
> - It will be ignored when crashkernel=X,high is not used
> - or memory reserved is located in the DMA zones.
> -
> cryptomgr.notests
> [KNL] Disable crypto self-tests
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 339ee84e5a61a0b..5390f361208ccf7 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -96,6 +96,14 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit = PHYS_MASK + 1;
> #define CRASH_ADDR_LOW_MAX arm64_dma_phys_limit
> #define CRASH_ADDR_HIGH_MAX (PHYS_MASK + 1)
>
> +/*
> + * This is an empirical value in x86_64 and taken here directly. Please
> + * refer to the code comment in reserve_crashkernel_low() of x86_64 for more
> + * details.
Honestly, I read that comment and I'm none the wiser. What does "due to
mapping restrictions" mean? The remainder of the comment appears to be
specific to x86 and refers to jump ranges with 5-level page-tables.
> +#define DEFAULT_CRASH_KERNEL_LOW_SIZE \
> + max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20)
So why does this value make sense for arm64? We have considerable platform
fragmentation^Wdiversity compared to x86 and picking a one-size-fits-all
default is more likely to cause weird problems when it doesn't work imo. I'd
actually prefer that the default is something that fails obviously (e.g. 0)
and we force an appropriate size to be specified.
On the other hand, if you can convince me that having a global constant is
the right way forward, then please move this out of the arch code.
Will
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified
2022-08-02 8:37 ` Will Deacon
@ 2022-08-02 10:12 ` Leizhen (ThunderTown)
2022-08-02 12:46 ` Baoquan He
0 siblings, 1 reply; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2022-08-02 10:12 UTC (permalink / raw)
To: Will Deacon
Cc: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Catalin Marinas, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp, ardb
On 2022/8/2 16:37, Will Deacon wrote:
> On Mon, Jul 11, 2022 at 05:03:18PM +0800, Zhen Lei wrote:
>> To be consistent with the implementation of x86 and improve cross-platform
>> user experience. Try to allocate at least 256 MiB low memory automatically
>> for the case that crashkernel=,high is explicitly specified, while
>> crashkenrel=,low is omitted.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> Acked-by: Baoquan He <bhe@redhat.com>
>> ---
>> Documentation/admin-guide/kernel-parameters.txt | 8 +-------
>> arch/arm64/mm/init.c | 12 +++++++++++-
>> 2 files changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 2522b11e593f239..65a2c3a22a4b57d 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -843,7 +843,7 @@
>> available.
>> It will be ignored if crashkernel=X is specified.
>> crashkernel=size[KMG],low
>> - [KNL, X86-64] range under 4G. When crashkernel=X,high
>> + [KNL, X86-64, ARM64] range under 4G. When crashkernel=X,high
>> is passed, kernel could allocate physical memory region
>> above 4G, that cause second kernel crash on system
>> that require some amount of low memory, e.g. swiotlb
>> @@ -857,12 +857,6 @@
>> It will be ignored when crashkernel=X,high is not used
>> or memory reserved is below 4G.
>>
>> - [KNL, ARM64] range in low memory.
>> - This one lets the user specify a low range in the
>> - DMA zone for the crash dump kernel.
>> - It will be ignored when crashkernel=X,high is not used
>> - or memory reserved is located in the DMA zones.
>> -
>> cryptomgr.notests
>> [KNL] Disable crypto self-tests
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 339ee84e5a61a0b..5390f361208ccf7 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -96,6 +96,14 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit = PHYS_MASK + 1;
>> #define CRASH_ADDR_LOW_MAX arm64_dma_phys_limit
>> #define CRASH_ADDR_HIGH_MAX (PHYS_MASK + 1)
>>
>> +/*
>> + * This is an empirical value in x86_64 and taken here directly. Please
>> + * refer to the code comment in reserve_crashkernel_low() of x86_64 for more
>> + * details.
>
> Honestly, I read that comment and I'm none the wiser. What does "due to
> mapping restrictions" mean? The remainder of the comment appears to be
Because the comments you read is addressed to CRASH_ADDR_LOW_MAX, not
for DEFAULT_CRASH_KERNEL_LOW_SIZE. Please see the following patch:
94fb9334182284 x86/crash: Allocate enough low memory when crashkernel=high
> specific to x86 and refers to jump ranges with 5-level page-tables.
>
>> +#define DEFAULT_CRASH_KERNEL_LOW_SIZE \
>> + max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20)
>
> So why does this value make sense for arm64? We have considerable platform
> fragmentation^Wdiversity compared to x86 and picking a one-size-fits-all
> default is more likely to cause weird problems when it doesn't work imo. I'd
> actually prefer that the default is something that fails obviously (e.g. 0)
> and we force an appropriate size to be specified.
>
> On the other hand, if you can convince me that having a global constant is
> the right way forward, then please move this out of the arch code.
Yes, the default value may not be the same as that of x86. For example,
128 MB may be sufficient.
So we need to discuss first, do we need a default value? Personally, I
don't think it hurts.
>
> Will
> .
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified
2022-08-02 10:12 ` Leizhen (ThunderTown)
@ 2022-08-02 12:46 ` Baoquan He
0 siblings, 0 replies; 23+ messages in thread
From: Baoquan He @ 2022-08-02 12:46 UTC (permalink / raw)
To: Will Deacon, Leizhen (ThunderTown)
Cc: Dave Young, Vivek Goyal, kexec, linux-kernel, Catalin Marinas,
linux-arm-kernel, Jonathan Corbet, linux-doc, Eric W . Biederman,
Randy Dunlap, Feng Zhou, Kefeng Wang, Chen Zhou, John Donnelly,
Dave Kleikamp, ardb
On 08/02/22 at 06:12pm, Leizhen (ThunderTown) wrote:
......snip...
> >> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> >> index 339ee84e5a61a0b..5390f361208ccf7 100644
> >> --- a/arch/arm64/mm/init.c
> >> +++ b/arch/arm64/mm/init.c
> >> @@ -96,6 +96,14 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit = PHYS_MASK + 1;
> >> #define CRASH_ADDR_LOW_MAX arm64_dma_phys_limit
> >> #define CRASH_ADDR_HIGH_MAX (PHYS_MASK + 1)
> >>
> >> +/*
> >> + * This is an empirical value in x86_64 and taken here directly. Please
> >> + * refer to the code comment in reserve_crashkernel_low() of x86_64 for more
> >> + * details.
> >
> > Honestly, I read that comment and I'm none the wiser. What does "due to
> > mapping restrictions" mean? The remainder of the comment appears to be
>
> Because the comments you read is addressed to CRASH_ADDR_LOW_MAX, not
> for DEFAULT_CRASH_KERNEL_LOW_SIZE. Please see the following patch:
>
> 94fb9334182284 x86/crash: Allocate enough low memory when crashkernel=high
>
> > specific to x86 and refers to jump ranges with 5-level page-tables.
> >
> >> +#define DEFAULT_CRASH_KERNEL_LOW_SIZE \
> >> + max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20)
> >
> > So why does this value make sense for arm64? We have considerable platform
> > fragmentation^Wdiversity compared to x86 and picking a one-size-fits-all
> > default is more likely to cause weird problems when it doesn't work imo. I'd
> > actually prefer that the default is something that fails obviously (e.g. 0)
> > and we force an appropriate size to be specified.
The default value mainly serves the crashkernel=xM case, not just for
crashkernel=,high specified while no crahskernel=,low. The simplest
usage of crashkernel reservation is user only need set crashkernel=xM in
cmdline, we will try to get available memory from low memory region
firstly, if failed, go above 4G to find again. If we finally get memory
from above 4G, then the default low memory is needed. E.g if
crashkernel=512M is set, and no sufficient memory under 4G. With it,
user don't need to know about crashkernel=,high or ,low, and even memory
has type of high, low, or dma stuff.
> >
> > On the other hand, if you can convince me that having a global constant is
> > the right way forward, then please move this out of the arch code.
>
> Yes, the default value may not be the same as that of x86. For example,
> 128 MB may be sufficient.
>
> So we need to discuss first, do we need a default value? Personally, I
> don't think it hurts.
Yes, we can discuss. Welcome anyone to help provide information how we
should take care to make a small but enough value. In fact, on x86_64,
we didn't set the value at one time. It was set as 72M at the beginning,
later Joerg found it's not enough, we finally decided to make it as
256M. People who mind the wasting of 256M can use crashkernel=,high and
crashkernel=,low pair to specify the value exactly.
commit c729de8fcea3 ("x86, kdump: Set crashkernel_low automatically")
commit 94fb93341822 ("x86/crash: Allocate enough low memory when crashkernel=high")
Thanks
Baoquan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
2022-08-02 2:47 ` Leizhen (ThunderTown)
@ 2022-10-06 14:55 ` john.p.donnelly
2022-10-13 10:46 ` Baoquan He
0 siblings, 1 reply; 23+ messages in thread
From: john.p.donnelly @ 2022-10-06 14:55 UTC (permalink / raw)
To: Leizhen (ThunderTown), Baoquan He
Cc: Dave Young, Vivek Goyal, kexec, linux-kernel, Catalin Marinas,
Will Deacon, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, Dave Kleikamp, John Donnelly, samasth.norway.ananda
On 8/1/22 9:47 PM, Leizhen (ThunderTown) wrote:
>
>
> On 2022/8/1 16:20, Baoquan He wrote:
>> Hi Catalin,
>>
>> On 07/11/22 at 05:03pm, Zhen Lei wrote:
>>> v2 --> v3:
>>> 1. Discard patch 3 in v2, a cleanup patch.
>>>
>>> v1 --> v2:
>>> 1. Update the commit message of Patch 1, explicitly indicates that "crashkernel=X,high"
>>> is specified but "crashkernel=Y,low" is not specified.
>>> 2. Drop Patch 4-5. Currently, focus on function integrity, performance optimization
>>> will be considered in later versions.
>>> 3. Patch 3 is not mandatory, it's just a cleanup now, although it is a must for patch 4-5.
>>> But to avoid subsequent duplication of effort, I'm glad it was accepted.
>>>
>>>
>>> v1:
>>> After the basic functions of "support reserving crashkernel above 4G on arm64
>>> kdump"(see https://urldefense.com/v3/__https://lkml.org/lkml/2022/5/6/428__;!!ACWV5N9M2RV99hQ!ORBFa4UAmMss_79nuwu1kpW3D-mTela240vFo0FXOuV9QpGWy7Fp2H81ZjLPOuaufAQC_XBFEFGjAqs5njfGS6Rd4dZLhaez$ ) are implemented, we still have
>>> three features to be improved.
>>> 1. When crashkernel=X,high is specified but crashkernel=Y,low is not specified,
>>> the default crash low memory size is provided.
>>> 2. For crashkernel=X without '@offset', if the low memory fails to be allocated,
>>> fall back to reserve region from high memory(above DMA zones).
>>> 3. If crashkernel=X,high is used, page mapping is performed only for the crash
>>> high memory, and block mapping is still used for other linear address spaces.
>>> Compared to the previous version:
>>> (1) For crashkernel=X[@offset], the memory above 4G is not changed to block
>>> mapping, leave it to the next time.
>>> (2) The implementation method is modified. Now the implementation is simpler
>>> and clearer.
>>
>> Do you have plan to pick this series so that it can be taken into 5.20
>> rc-1~3?
>
> Hi, Catalin:
> Only function reserve_crashkernel() is modified in these two patches. The core
> process of the arm64 architecture is not affected. I remember you suggested that
> arm64 and x86 share the same kdump code, so these two subfeatures are needed.
> Maybe we can lay the foundation first for the people who build the road. Unifying
> the external interfaces of kdump on arm64 and x86 does not seem to hurt.
>
>
>>
>> We have back ported the basic crashkernel=high, low, support into our
>> distros and have taken wide testing on arm64 servers, need this patchset
>> to back port for more testing.
>>
>> Thanks
>> Baoquan
>>
>>>
>>> Zhen Lei (2):
>>> arm64: kdump: Provide default size when crashkernel=Y,low is not
>>> specified
>>> arm64: kdump: Support crashkernel=X fall back to reserve region above
>>> DMA zones
>>>
>>> .../admin-guide/kernel-parameters.txt | 10 ++-----
>>> arch/arm64/mm/init.c | 28 +++++++++++++++++--
>>> 2 files changed, 28 insertions(+), 10 deletions(-)
>>>
>>> --
>>> 2.25.1
>>>
>>
>> .
>>
>
Hi ,
What is the progress of this series ?
Without this patch set we are seeing larger crashkernel=896M failures
on Arm with Linux-6.0.rc7. This larger value is needed for
iSCSI booted systems with certain network adapters.
Thank you,
John.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
2022-10-06 14:55 ` john.p.donnelly
@ 2022-10-13 10:46 ` Baoquan He
2022-10-14 16:25 ` John Donnelly
2022-10-14 16:29 ` Catalin Marinas
0 siblings, 2 replies; 23+ messages in thread
From: Baoquan He @ 2022-10-13 10:46 UTC (permalink / raw)
To: john.p.donnelly, Catalin Marinas, Will Deacon
Cc: Leizhen (ThunderTown),
Dave Young, Vivek Goyal, kexec, linux-kernel, linux-arm-kernel,
Jonathan Corbet, linux-doc, Eric W . Biederman, Randy Dunlap,
Feng Zhou, Kefeng Wang, Chen Zhou, Dave Kleikamp,
samasth.norway.ananda
On 10/06/22 at 09:55am, john.p.donnelly@oracle.com wrote:
> On 8/1/22 9:47 PM, Leizhen (ThunderTown) wrote:
......
> > > Do you have plan to pick this series so that it can be taken into 5.20
> > > rc-1~3?
> >
> > Hi, Catalin:
> > Only function reserve_crashkernel() is modified in these two patches. The core
> > process of the arm64 architecture is not affected. I remember you suggested that
> > arm64 and x86 share the same kdump code, so these two subfeatures are needed.
> > Maybe we can lay the foundation first for the people who build the road. Unifying
> > the external interfaces of kdump on arm64 and x86 does not seem to hurt.
> >
> >
> > >
> > > We have back ported the basic crashkernel=high, low, support into our
> > > distros and have taken wide testing on arm64 servers, need this patchset
> > > to back port for more testing.
> > >
> >
> Hi ,
>
> What is the progress of this series ?
>
> Without this patch set we are seeing larger crashkernel=896M failures on
> Arm with Linux-6.0.rc7. This larger value is needed for
> iSCSI booted systems with certain network adapters.
This change is located in arch/arm64 folder, I have pinged arm64
maintainer to consider merging this patchset. Not sure if they are
still thinking, or ignore this.
Hi Catalin, Will,
Ping again!
Do you have plan to accept this patchset? It's very important for
crashkernel setting on arm64 with a simple and default syntax.
Thanks
Baoquan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
2022-10-13 10:46 ` Baoquan He
@ 2022-10-14 16:25 ` John Donnelly
2022-10-14 16:29 ` Catalin Marinas
1 sibling, 0 replies; 23+ messages in thread
From: John Donnelly @ 2022-10-14 16:25 UTC (permalink / raw)
To: Baoquan He, Catalin Marinas, Will Deacon
Cc: Leizhen (ThunderTown),
Dave Young, Vivek Goyal, kexec, linux-kernel, linux-arm-kernel,
Jonathan Corbet, linux-doc, Eric W . Biederman, Randy Dunlap,
Feng Zhou, Kefeng Wang, Chen Zhou, Dave Kleikamp,
samasth.norway.ananda, John Donnelly
On 10/13/22 05:46, Baoquan He wrote:
> On 10/06/22 at 09:55am, john.p.donnelly@oracle.com wrote:
>> On 8/1/22 9:47 PM, Leizhen (ThunderTown) wrote:
> ......
>>>> Do you have plan to pick this series so that it can be taken into 5.20
>>>> rc-1~3?
>>>
>>> Hi, Catalin:
>>> Only function reserve_crashkernel() is modified in these two patches. The core
>>> process of the arm64 architecture is not affected. I remember you suggested that
>>> arm64 and x86 share the same kdump code, so these two subfeatures are needed.
>>> Maybe we can lay the foundation first for the people who build the road. Unifying
>>> the external interfaces of kdump on arm64 and x86 does not seem to hurt.
>>>
>>>
>>>>
>>>> We have back ported the basic crashkernel=high, low, support into our
>>>> distros and have taken wide testing on arm64 servers, need this patchset
>>>> to back port for more testing.
>>>>
>>>
>> Hi ,
>>
>> What is the progress of this series ?
>>
>> Without this patch set we are seeing larger crashkernel=896M failures on
>> Arm with Linux-6.0.rc7. This larger value is needed for
>> iSCSI booted systems with certain network adapters.
>
> This change is located in arch/arm64 folder, I have pinged arm64
> maintainer to consider merging this patchset. Not sure if they are
> still thinking, or ignore this.
>
> Hi Catalin, Will,
>
> Ping again!
>
> Do you have plan to accept this patchset? It's very important fo
> crashkernel setting on arm64 with a simple and default syntax.
>
> Thanks
> Baoquan
>
Hi,
We have pulled this into our Linux 6.0.0 QA test kernels for now. We
would like it added too.
Thank you,
John
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
2022-10-13 10:46 ` Baoquan He
2022-10-14 16:25 ` John Donnelly
@ 2022-10-14 16:29 ` Catalin Marinas
2022-10-26 19:41 ` john.p.donnelly
1 sibling, 1 reply; 23+ messages in thread
From: Catalin Marinas @ 2022-10-14 16:29 UTC (permalink / raw)
To: Baoquan He
Cc: john.p.donnelly, Will Deacon, Leizhen (ThunderTown),
Dave Young, Vivek Goyal, kexec, linux-kernel, linux-arm-kernel,
Jonathan Corbet, linux-doc, Eric W . Biederman, Randy Dunlap,
Feng Zhou, Kefeng Wang, Chen Zhou, Dave Kleikamp,
samasth.norway.ananda
On Thu, Oct 13, 2022 at 06:46:35PM +0800, Baoquan He wrote:
> On 10/06/22 at 09:55am, john.p.donnelly@oracle.com wrote:
> > What is the progress of this series ?
> >
> > Without this patch set we are seeing larger crashkernel=896M failures on
> > Arm with Linux-6.0.rc7. This larger value is needed for
> > iSCSI booted systems with certain network adapters.
>
> This change is located in arch/arm64 folder, I have pinged arm64
> maintainer to consider merging this patchset. Not sure if they are
> still thinking, or ignore this.
>
> Hi Catalin, Will,
>
> Ping again!
>
> Do you have plan to accept this patchset? It's very important for
> crashkernel setting on arm64 with a simple and default syntax.
I'll look at it once the merging window closes. I saw discussions on
this thread and I ignored it until you all agreed ;).
--
Catalin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
2022-10-14 16:29 ` Catalin Marinas
@ 2022-10-26 19:41 ` john.p.donnelly
0 siblings, 0 replies; 23+ messages in thread
From: john.p.donnelly @ 2022-10-26 19:41 UTC (permalink / raw)
To: Catalin Marinas, Baoquan He
Cc: Will Deacon, Leizhen (ThunderTown),
Dave Young, Vivek Goyal, kexec, linux-kernel, linux-arm-kernel,
Jonathan Corbet, linux-doc, Eric W . Biederman, Randy Dunlap,
Feng Zhou, Kefeng Wang, Chen Zhou, Dave Kleikamp,
samasth.norway.ananda, John Donnelly
On 10/14/22 11:29 AM, Catalin Marinas wrote:
> On Thu, Oct 13, 2022 at 06:46:35PM +0800, Baoquan He wrote:
>> On 10/06/22 at 09:55am, john.p.donnelly@oracle.com wrote:
>>> What is the progress of this series ?
>>>
>>> Without this patch set we are seeing larger crashkernel=896M failures on
>>> Arm with Linux-6.0.rc7. This larger value is needed for
>>> iSCSI booted systems with certain network adapters.
>>
>> This change is located in arch/arm64 folder, I have pinged arm64
>> maintainer to consider merging this patchset. Not sure if they are
>> still thinking, or ignore this.
>>
>> Hi Catalin, Will,
>>
>> Ping again!
>>
>> Do you have plan to accept this patchset? It's very important for
>> crashkernel setting on arm64 with a simple and default syntax.
>
> I'll look at it once the merging window closes. I saw discussions on
> this thread and I ignored it until you all agreed ;).
>
Hi,
Do you have a timeline for this ? This crashkernel > 4G for Arm item
has been lingering for 3 years. I think it is time for it to be
incorporated.
Thanks,
John.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified
2022-07-11 9:03 ` [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified Zhen Lei
2022-08-02 8:37 ` Will Deacon
@ 2022-11-07 14:07 ` Catalin Marinas
2022-11-16 11:50 ` Leizhen (ThunderTown)
2022-11-07 17:18 ` Catalin Marinas
2 siblings, 1 reply; 23+ messages in thread
From: Catalin Marinas @ 2022-11-07 14:07 UTC (permalink / raw)
To: Zhen Lei
Cc: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Will Deacon, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
On Mon, Jul 11, 2022 at 05:03:18PM +0800, Zhen Lei wrote:
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 339ee84e5a61a0b..5390f361208ccf7 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -96,6 +96,14 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit = PHYS_MASK + 1;
> #define CRASH_ADDR_LOW_MAX arm64_dma_phys_limit
> #define CRASH_ADDR_HIGH_MAX (PHYS_MASK + 1)
>
> +/*
> + * This is an empirical value in x86_64 and taken here directly. Please
> + * refer to the code comment in reserve_crashkernel_low() of x86_64 for more
> + * details.
> + */
> +#define DEFAULT_CRASH_KERNEL_LOW_SIZE \
> + max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20)
I agree with Will here, we need a better comment and we might as well
change the default value to something else until someone tells us that
the default is not large enough. The default swiotlb size is 64M, so we
need to cover that. The extra 8MB for any additional low allocations are
ok as well but the 256MB doesn't make much sense to me, or at least not
together with the rest.
If the main kernel got a command line option for a larger swiotlb, does
the crash kernel boot with the same command line? If not, we can just go
for a fixed 128M value here, which is double the default swiotlb buffer.
--
Catalin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones
2022-07-11 9:03 ` [PATCH v3 2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones Zhen Lei
@ 2022-11-07 17:13 ` Catalin Marinas
2022-11-08 2:06 ` Leizhen (ThunderTown)
0 siblings, 1 reply; 23+ messages in thread
From: Catalin Marinas @ 2022-11-07 17:13 UTC (permalink / raw)
To: Zhen Lei
Cc: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Will Deacon, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
On Mon, Jul 11, 2022 at 05:03:19PM +0800, Zhen Lei wrote:
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 5390f361208ccf7..8539598f9e58b4d 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -138,6 +138,7 @@ static void __init reserve_crashkernel(void)
> unsigned long long crash_max = CRASH_ADDR_LOW_MAX;
> char *cmdline = boot_command_line;
> int ret;
> + bool fixed_base;
>
> if (!IS_ENABLED(CONFIG_KEXEC_CORE))
> return;
> @@ -166,15 +167,28 @@ static void __init reserve_crashkernel(void)
> return;
> }
>
> + fixed_base = !!crash_base;
> crash_size = PAGE_ALIGN(crash_size);
>
> /* User specifies base address explicitly. */
> - if (crash_base)
> + if (fixed_base)
> crash_max = crash_base + crash_size;
Not a fan of '!!', it is converted automatically. If you don't like the
conversion, just initialise fixed_base to false and here:
if (crash_base) {
fixed_base = true;
crash_max = crash_base + crash_size;
}
> +retry:
> crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN,
> crash_base, crash_max);
> if (!crash_base) {
> + /*
> + * Attempt to fully allocate low memory failed, fall back
> + * to high memory, the minimum required low memory will be
> + * reserved later.
> + */
I'm not sure this comment makes sense. If !crash_base, it doesn't mean
the kernel failed to fully allocate low memory. crash_max here could be
CRASH_ADDR_HIGH_MAX if crashkerne=X,high was specified. Maybe says
something like "If the first attempt was for low memory, fall back to
high ..."
> + if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) {
> + crash_max = CRASH_ADDR_HIGH_MAX;
> + crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
> + goto retry;
> + }
The retry logic looks fine, it only happens once as crash_max is
updated.
--
Catalin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified
2022-07-11 9:03 ` [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified Zhen Lei
2022-08-02 8:37 ` Will Deacon
2022-11-07 14:07 ` Catalin Marinas
@ 2022-11-07 17:18 ` Catalin Marinas
2022-11-08 2:47 ` Leizhen (ThunderTown)
2 siblings, 1 reply; 23+ messages in thread
From: Catalin Marinas @ 2022-11-07 17:18 UTC (permalink / raw)
To: Zhen Lei
Cc: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Will Deacon, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
On Mon, Jul 11, 2022 at 05:03:18PM +0800, Zhen Lei wrote:
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2522b11e593f239..65a2c3a22a4b57d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -843,7 +843,7 @@
> available.
> It will be ignored if crashkernel=X is specified.
> crashkernel=size[KMG],low
> - [KNL, X86-64] range under 4G. When crashkernel=X,high
> + [KNL, X86-64, ARM64] range under 4G. When crashkernel=X,high
> is passed, kernel could allocate physical memory region
> above 4G, that cause second kernel crash on system
> that require some amount of low memory, e.g. swiotlb
> @@ -857,12 +857,6 @@
> It will be ignored when crashkernel=X,high is not used
> or memory reserved is below 4G.
>
> - [KNL, ARM64] range in low memory.
> - This one lets the user specify a low range in the
> - DMA zone for the crash dump kernel.
> - It will be ignored when crashkernel=X,high is not used
> - or memory reserved is located in the DMA zones.
> -
> cryptomgr.notests
> [KNL] Disable crypto self-tests
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 339ee84e5a61a0b..5390f361208ccf7 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -96,6 +96,14 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit = PHYS_MASK + 1;
> #define CRASH_ADDR_LOW_MAX arm64_dma_phys_limit
> #define CRASH_ADDR_HIGH_MAX (PHYS_MASK + 1)
>
> +/*
> + * This is an empirical value in x86_64 and taken here directly. Please
> + * refer to the code comment in reserve_crashkernel_low() of x86_64 for more
> + * details.
> + */
> +#define DEFAULT_CRASH_KERNEL_LOW_SIZE \
> + max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20)
> +
> static int __init reserve_crashkernel_low(unsigned long long low_size)
> {
> unsigned long long low_base;
> @@ -147,7 +155,9 @@ static void __init reserve_crashkernel(void)
> * is not allowed.
> */
> ret = parse_crashkernel_low(cmdline, 0, &crash_low_size, &crash_base);
> - if (ret && (ret != -ENOENT))
> + if (ret == -ENOENT)
> + crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
> + else if (ret)
> return;
BTW, since we want a default low allocation, I think we should change
the checking logic slightly. Currently we have:
if ((crash_base >= CRASH_ADDR_LOW_MAX) &&
crash_low_size && reserve_crashkernel_low(crash_low_size)) {
...
If crash_base is just below CRASH_ADDR_LOW_MAX, we deem it sufficient
but a crashkernel trying to allocate 64MB of swiotlb may fail. So maybe
change this to crash_base >= CRASH_ADDR_LOW_MAX - crash_low_size.
--
Catalin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones
2022-11-07 17:13 ` Catalin Marinas
@ 2022-11-08 2:06 ` Leizhen (ThunderTown)
0 siblings, 0 replies; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2022-11-08 2:06 UTC (permalink / raw)
To: Catalin Marinas
Cc: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Will Deacon, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
On 2022/11/8 1:13, Catalin Marinas wrote:
> On Mon, Jul 11, 2022 at 05:03:19PM +0800, Zhen Lei wrote:
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 5390f361208ccf7..8539598f9e58b4d 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -138,6 +138,7 @@ static void __init reserve_crashkernel(void)
>> unsigned long long crash_max = CRASH_ADDR_LOW_MAX;
>> char *cmdline = boot_command_line;
>> int ret;
>> + bool fixed_base;
>>
>> if (!IS_ENABLED(CONFIG_KEXEC_CORE))
>> return;
>> @@ -166,15 +167,28 @@ static void __init reserve_crashkernel(void)
>> return;
>> }
>>
>> + fixed_base = !!crash_base;
>> crash_size = PAGE_ALIGN(crash_size);
>>
>> /* User specifies base address explicitly. */
>> - if (crash_base)
>> + if (fixed_base)
>> crash_max = crash_base + crash_size;
>
> Not a fan of '!!', it is converted automatically. If you don't like the
> conversion, just initialise fixed_base to false and here:
>
> if (crash_base) {
> fixed_base = true;
OK, This way would be better.
> crash_max = crash_base + crash_size;
> }
>
>> +retry:
>> crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN,
>> crash_base, crash_max);
>> if (!crash_base) {
>> + /*
>> + * Attempt to fully allocate low memory failed, fall back
>> + * to high memory, the minimum required low memory will be
>> + * reserved later.
>> + */
>
> I'm not sure this comment makes sense. If !crash_base, it doesn't mean
> the kernel failed to fully allocate low memory. crash_max here could be
> CRASH_ADDR_HIGH_MAX if crashkerne=X,high was specified. Maybe says
> something like "If the first attempt was for low memory, fall back to
> high ..."
This description is accurate. I'll update. Thanks.
>
>> + if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) {
>> + crash_max = CRASH_ADDR_HIGH_MAX;
>> + crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
>> + goto retry;
>> + }
>
> The retry logic looks fine, it only happens once as crash_max is
> updated.
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified
2022-11-07 17:18 ` Catalin Marinas
@ 2022-11-08 2:47 ` Leizhen (ThunderTown)
0 siblings, 0 replies; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2022-11-08 2:47 UTC (permalink / raw)
To: Catalin Marinas
Cc: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Will Deacon, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
On 2022/11/8 1:18, Catalin Marinas wrote:
> On Mon, Jul 11, 2022 at 05:03:18PM +0800, Zhen Lei wrote:
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 2522b11e593f239..65a2c3a22a4b57d 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -843,7 +843,7 @@
>> available.
>> It will be ignored if crashkernel=X is specified.
>> crashkernel=size[KMG],low
>> - [KNL, X86-64] range under 4G. When crashkernel=X,high
>> + [KNL, X86-64, ARM64] range under 4G. When crashkernel=X,high
>> is passed, kernel could allocate physical memory region
>> above 4G, that cause second kernel crash on system
>> that require some amount of low memory, e.g. swiotlb
>> @@ -857,12 +857,6 @@
>> It will be ignored when crashkernel=X,high is not used
>> or memory reserved is below 4G.
>>
>> - [KNL, ARM64] range in low memory.
>> - This one lets the user specify a low range in the
>> - DMA zone for the crash dump kernel.
>> - It will be ignored when crashkernel=X,high is not used
>> - or memory reserved is located in the DMA zones.
>> -
>> cryptomgr.notests
>> [KNL] Disable crypto self-tests
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 339ee84e5a61a0b..5390f361208ccf7 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -96,6 +96,14 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit = PHYS_MASK + 1;
>> #define CRASH_ADDR_LOW_MAX arm64_dma_phys_limit
>> #define CRASH_ADDR_HIGH_MAX (PHYS_MASK + 1)
>>
>> +/*
>> + * This is an empirical value in x86_64 and taken here directly. Please
>> + * refer to the code comment in reserve_crashkernel_low() of x86_64 for more
>> + * details.
>> + */
>> +#define DEFAULT_CRASH_KERNEL_LOW_SIZE \
>> + max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20)
>> +
>> static int __init reserve_crashkernel_low(unsigned long long low_size)
>> {
>> unsigned long long low_base;
>> @@ -147,7 +155,9 @@ static void __init reserve_crashkernel(void)
>> * is not allowed.
>> */
>> ret = parse_crashkernel_low(cmdline, 0, &crash_low_size, &crash_base);
>> - if (ret && (ret != -ENOENT))
>> + if (ret == -ENOENT)
>> + crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
>> + else if (ret)
>> return;
>
> BTW, since we want a default low allocation, I think we should change
> the checking logic slightly. Currently we have:
>
> if ((crash_base >= CRASH_ADDR_LOW_MAX) &&
> crash_low_size && reserve_crashkernel_low(crash_low_size)) {
> ...
>
> If crash_base is just below CRASH_ADDR_LOW_MAX, we deem it sufficient
> but a crashkernel trying to allocate 64MB of swiotlb may fail. So maybe
> change this to crash_base >= CRASH_ADDR_LOW_MAX - crash_low_size.
The equal sign needs to be removed.
The situation should be the allocation of "crashkernel=X,high".
This possibility is too small, the high memory is unlikely to be that small.
memblock_phys_alloc_range() always search for memory block from high addresses
to low addresses. In the initial phase, high-end memory is not fragmented.
Of course, the modification can make people look more reassuring. OK, I'll
update it.
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
2022-07-11 9:03 [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization Zhen Lei
` (2 preceding siblings ...)
2022-08-01 8:20 ` [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization Baoquan He
@ 2022-11-15 11:58 ` Will Deacon
2022-11-15 12:18 ` Leizhen (ThunderTown)
3 siblings, 1 reply; 23+ messages in thread
From: Will Deacon @ 2022-11-15 11:58 UTC (permalink / raw)
To: Zhen Lei
Cc: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Catalin Marinas, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
On Mon, Jul 11, 2022 at 05:03:17PM +0800, Zhen Lei wrote:
> v2 --> v3:
> 1. Discard patch 3 in v2, a cleanup patch.
Do you plan to respin this series, addressing the various comments on v3?
Will
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
2022-11-15 11:58 ` Will Deacon
@ 2022-11-15 12:18 ` Leizhen (ThunderTown)
2022-11-15 13:30 ` Catalin Marinas
0 siblings, 1 reply; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2022-11-15 12:18 UTC (permalink / raw)
To: Will Deacon
Cc: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Catalin Marinas, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
On 2022/11/15 19:58, Will Deacon wrote:
> On Mon, Jul 11, 2022 at 05:03:17PM +0800, Zhen Lei wrote:
>> v2 --> v3:
>> 1. Discard patch 3 in v2, a cleanup patch.
>
> Do you plan to respin this series, addressing the various comments on v3?
Yes, I haven't figured out where to make DEFAULT_CRASH_KERNEL_LOW_SIZE generic.
>
> Will
> .
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
2022-11-15 12:18 ` Leizhen (ThunderTown)
@ 2022-11-15 13:30 ` Catalin Marinas
2022-11-15 13:40 ` Leizhen (ThunderTown)
0 siblings, 1 reply; 23+ messages in thread
From: Catalin Marinas @ 2022-11-15 13:30 UTC (permalink / raw)
To: Leizhen (ThunderTown)
Cc: Will Deacon, Dave Young, Baoquan He, Vivek Goyal, kexec,
linux-kernel, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
On Tue, Nov 15, 2022 at 08:18:21PM +0800, Leizhen (ThunderTown) wrote:
> On 2022/11/15 19:58, Will Deacon wrote:
> > On Mon, Jul 11, 2022 at 05:03:17PM +0800, Zhen Lei wrote:
> >> v2 --> v3:
> >> 1. Discard patch 3 in v2, a cleanup patch.
> >
> > Do you plan to respin this series, addressing the various comments on v3?
>
> Yes, I haven't figured out where to make DEFAULT_CRASH_KERNEL_LOW_SIZE generic.
Do we need to? I'd just go with something like 128MB, specific to arm64,
and we can increase it later if anyone comes up with a good argument.
--
Catalin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization
2022-11-15 13:30 ` Catalin Marinas
@ 2022-11-15 13:40 ` Leizhen (ThunderTown)
0 siblings, 0 replies; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2022-11-15 13:40 UTC (permalink / raw)
To: Catalin Marinas
Cc: Will Deacon, Dave Young, Baoquan He, Vivek Goyal, kexec,
linux-kernel, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
On 2022/11/15 21:30, Catalin Marinas wrote:
> On Tue, Nov 15, 2022 at 08:18:21PM +0800, Leizhen (ThunderTown) wrote:
>> On 2022/11/15 19:58, Will Deacon wrote:
>>> On Mon, Jul 11, 2022 at 05:03:17PM +0800, Zhen Lei wrote:
>>>> v2 --> v3:
>>>> 1. Discard patch 3 in v2, a cleanup patch.
>>>
>>> Do you plan to respin this series, addressing the various comments on v3?
>>
>> Yes, I haven't figured out where to make DEFAULT_CRASH_KERNEL_LOW_SIZE generic.
>
> Do we need to? I'd just go with something like 128MB, specific to arm64,
> and we can increase it later if anyone comes up with a good argument.
Okay, then v3's easy. I'll do it tomorrow. I've tried it before. 128M is enough.
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified
2022-11-07 14:07 ` Catalin Marinas
@ 2022-11-16 11:50 ` Leizhen (ThunderTown)
0 siblings, 0 replies; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2022-11-16 11:50 UTC (permalink / raw)
To: Catalin Marinas
Cc: Dave Young, Baoquan He, Vivek Goyal, kexec, linux-kernel,
Will Deacon, linux-arm-kernel, Jonathan Corbet, linux-doc,
Eric W . Biederman, Randy Dunlap, Feng Zhou, Kefeng Wang,
Chen Zhou, John Donnelly, Dave Kleikamp
On 2022/11/7 22:07, Catalin Marinas wrote:
> On Mon, Jul 11, 2022 at 05:03:18PM +0800, Zhen Lei wrote:
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 339ee84e5a61a0b..5390f361208ccf7 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -96,6 +96,14 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit = PHYS_MASK + 1;
>> #define CRASH_ADDR_LOW_MAX arm64_dma_phys_limit
>> #define CRASH_ADDR_HIGH_MAX (PHYS_MASK + 1)
>>
>> +/*
>> + * This is an empirical value in x86_64 and taken here directly. Please
>> + * refer to the code comment in reserve_crashkernel_low() of x86_64 for more
>> + * details.
>> + */
>> +#define DEFAULT_CRASH_KERNEL_LOW_SIZE \
>> + max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20)
>
> I agree with Will here, we need a better comment and we might as well
> change the default value to something else until someone tells us that
> the default is not large enough. The default swiotlb size is 64M, so we
> need to cover that. The extra 8MB for any additional low allocations are
> ok as well but the 256MB doesn't make much sense to me, or at least not
> together with the rest.
>
> If the main kernel got a command line option for a larger swiotlb, does
> the crash kernel boot with the same command line? If not, we can just go
Sometimes the image is shared, but the command line is often different.
The command line of the crash kernel is specified by kexec --append=.
> for a fixed 128M value here, which is double the default swiotlb buffer.
Sorry, I missed this e-mail. Yes, fixed 128M would be better. I haven't
seen anyone adding "swiotlb=" to the crash kernel. And, if the default size
isn't enough, he can use crashkernel=Y,low.
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2022-11-16 11:58 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-11 9:03 [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization Zhen Lei
2022-07-11 9:03 ` [PATCH v3 1/2] arm64: kdump: Provide default size when crashkernel=Y,low is not specified Zhen Lei
2022-08-02 8:37 ` Will Deacon
2022-08-02 10:12 ` Leizhen (ThunderTown)
2022-08-02 12:46 ` Baoquan He
2022-11-07 14:07 ` Catalin Marinas
2022-11-16 11:50 ` Leizhen (ThunderTown)
2022-11-07 17:18 ` Catalin Marinas
2022-11-08 2:47 ` Leizhen (ThunderTown)
2022-07-11 9:03 ` [PATCH v3 2/2] arm64: kdump: Support crashkernel=X fall back to reserve region above DMA zones Zhen Lei
2022-11-07 17:13 ` Catalin Marinas
2022-11-08 2:06 ` Leizhen (ThunderTown)
2022-08-01 8:20 ` [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization Baoquan He
2022-08-02 2:47 ` Leizhen (ThunderTown)
2022-10-06 14:55 ` john.p.donnelly
2022-10-13 10:46 ` Baoquan He
2022-10-14 16:25 ` John Donnelly
2022-10-14 16:29 ` Catalin Marinas
2022-10-26 19:41 ` john.p.donnelly
2022-11-15 11:58 ` Will Deacon
2022-11-15 12:18 ` Leizhen (ThunderTown)
2022-11-15 13:30 ` Catalin Marinas
2022-11-15 13:40 ` Leizhen (ThunderTown)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).