[PATCH v2 3/6] crash: let arch decide crash memory export to iomem_resource

Sourabh Jain posted 6 patches 11 months ago
There is a newer version of this series
[PATCH v2 3/6] crash: let arch decide crash memory export to iomem_resource
Posted by Sourabh Jain 11 months ago
insert_crashkernel_resources() adds crash memory to iomem_resource if
generic crashkernel reservation is enabled on an architecture.

On PowerPC, system RAM is added to iomem_resource. See commit
c40dd2f766440 ("powerpc: Add System RAM to /proc/iomem").

Enabling generic crashkernel reservation on PowerPC leads to a conflict
when system RAM is added to iomem_resource because a part of the system
RAM, the crashkernel memory, has already been added to iomem_resource.

The next commit in the series "powerpc/crash: use generic crashkernel
reservation" enables generic crashkernel reservation on PowerPC. If the
crashkernel is added to iomem_resource, the kernel fails to add
system RAM to /proc/iomem and prints the following traces:

CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.13.0-rc2+
snip...
NIP [c000000002016b3c] add_system_ram_resources+0xf0/0x15c
LR [c000000002016b34] add_system_ram_resources+0xe8/0x15c
Call Trace:
[c00000000484bbc0] [c000000002016b34] add_system_ram_resources+0xe8/0x15c
[c00000000484bc20] [c000000000010a4c] do_one_initcall+0x7c/0x39c
[c00000000484bd00] [c000000002005418] do_initcalls+0x144/0x18c
[c00000000484bd90] [c000000002005714] kernel_init_freeable+0x21c/0x290
[c00000000484bdf0] [c0000000000110f4] kernel_init+0x2c/0x1b8
[c00000000484be50] [c00000000000dd3c] ret_from_kernel_user_thread+0x14/0x1c

To avoid this, an architecture hook is added in
insert_crashkernel_resources(), allowing the architecture to decide
whether crashkernel memory should be added to iomem_resource.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Baoquan he <bhe@redhat.com>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: kexec@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 include/linux/crash_reserve.h | 11 +++++++++++
 kernel/crash_reserve.c        |  3 +++
 2 files changed, 14 insertions(+)

diff --git a/include/linux/crash_reserve.h b/include/linux/crash_reserve.h
index 1fe7e7d1b214..f1205d044dae 100644
--- a/include/linux/crash_reserve.h
+++ b/include/linux/crash_reserve.h
@@ -18,6 +18,17 @@ int __init parse_crashkernel(char *cmdline, unsigned long long system_ram,
 		unsigned long long *crash_size, unsigned long long *crash_base,
 		unsigned long long *low_size, bool *high);
 
+#ifdef CONFIG_ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
+
+#ifndef arch_add_crash_res_to_iomem
+static inline bool arch_add_crash_res_to_iomem(void)
+{
+	return true;
+}
+#endif
+
+#endif
+
 #ifdef CONFIG_ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
 #ifndef DEFAULT_CRASH_KERNEL_LOW_SIZE
 #define DEFAULT_CRASH_KERNEL_LOW_SIZE	(128UL << 20)
diff --git a/kernel/crash_reserve.c b/kernel/crash_reserve.c
index aff7c0fdbefa..190104f32fe1 100644
--- a/kernel/crash_reserve.c
+++ b/kernel/crash_reserve.c
@@ -460,6 +460,9 @@ void __init reserve_crashkernel_generic(unsigned long long crash_size,
 #ifndef HAVE_ARCH_ADD_CRASH_RES_TO_IOMEM_EARLY
 static __init int insert_crashkernel_resources(void)
 {
+	if (!arch_add_crash_res_to_iomem())
+		return 0;
+
 	if (crashk_res.start < crashk_res.end)
 		insert_resource(&iomem_resource, &crashk_res);
 
-- 
2.47.1
Re: [PATCH v2 3/6] crash: let arch decide crash memory export to iomem_resource
Posted by Hari Bathini 11 months ago
Hi Sourabh,

On 21/01/25 5:24 pm, Sourabh Jain wrote:
> insert_crashkernel_resources() adds crash memory to iomem_resource if
> generic crashkernel reservation is enabled on an architecture.
> 

> On PowerPC, system RAM is added to iomem_resource. See commit
> c40dd2f766440 ("powerpc: Add System RAM to /proc/iomem").

The changelog clearly says the patch was added for kdump. But 
kexec-tools or kernel code for kdump on powerpc don't seem to have relied
on that. Device-tree memory nodes were used instead? Wondering if
dropping commit c40dd2f766440 is better..

- Hari

> 
> Enabling generic crashkernel reservation on PowerPC leads to a conflict
> when system RAM is added to iomem_resource because a part of the system
> RAM, the crashkernel memory, has already been added to iomem_resource.
> 
> The next commit in the series "powerpc/crash: use generic crashkernel
> reservation" enables generic crashkernel reservation on PowerPC. If the
> crashkernel is added to iomem_resource, the kernel fails to add
> system RAM to /proc/iomem and prints the following traces:
> 
> CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.13.0-rc2+
> snip...
> NIP [c000000002016b3c] add_system_ram_resources+0xf0/0x15c
> LR [c000000002016b34] add_system_ram_resources+0xe8/0x15c
> Call Trace:
> [c00000000484bbc0] [c000000002016b34] add_system_ram_resources+0xe8/0x15c
> [c00000000484bc20] [c000000000010a4c] do_one_initcall+0x7c/0x39c
> [c00000000484bd00] [c000000002005418] do_initcalls+0x144/0x18c
> [c00000000484bd90] [c000000002005714] kernel_init_freeable+0x21c/0x290
> [c00000000484bdf0] [c0000000000110f4] kernel_init+0x2c/0x1b8
> [c00000000484be50] [c00000000000dd3c] ret_from_kernel_user_thread+0x14/0x1c
> 
> To avoid this, an architecture hook is added in
> insert_crashkernel_resources(), allowing the architecture to decide
> whether crashkernel memory should be added to iomem_resource.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Baoquan he <bhe@redhat.com>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: kexec@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>   include/linux/crash_reserve.h | 11 +++++++++++
>   kernel/crash_reserve.c        |  3 +++
>   2 files changed, 14 insertions(+)
> 
> diff --git a/include/linux/crash_reserve.h b/include/linux/crash_reserve.h
> index 1fe7e7d1b214..f1205d044dae 100644
> --- a/include/linux/crash_reserve.h
> +++ b/include/linux/crash_reserve.h
> @@ -18,6 +18,17 @@ int __init parse_crashkernel(char *cmdline, unsigned long long system_ram,
>   		unsigned long long *crash_size, unsigned long long *crash_base,
>   		unsigned long long *low_size, bool *high);
>   
> +#ifdef CONFIG_ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
> +
> +#ifndef arch_add_crash_res_to_iomem
> +static inline bool arch_add_crash_res_to_iomem(void)
> +{
> +	return true;
> +}
> +#endif
> +
> +#endif
> +
>   #ifdef CONFIG_ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
>   #ifndef DEFAULT_CRASH_KERNEL_LOW_SIZE
>   #define DEFAULT_CRASH_KERNEL_LOW_SIZE	(128UL << 20)
> diff --git a/kernel/crash_reserve.c b/kernel/crash_reserve.c
> index aff7c0fdbefa..190104f32fe1 100644
> --- a/kernel/crash_reserve.c
> +++ b/kernel/crash_reserve.c
> @@ -460,6 +460,9 @@ void __init reserve_crashkernel_generic(unsigned long long crash_size,
>   #ifndef HAVE_ARCH_ADD_CRASH_RES_TO_IOMEM_EARLY
>   static __init int insert_crashkernel_resources(void)
>   {
> +	if (!arch_add_crash_res_to_iomem())
> +		return 0;
> +
>   	if (crashk_res.start < crashk_res.end)
>   		insert_resource(&iomem_resource, &crashk_res);
>
Re: [PATCH v2 3/6] crash: let arch decide crash memory export to iomem_resource
Posted by Sourabh Jain 11 months ago
Hello Hari,


On 23/01/25 15:56, Hari Bathini wrote:
> Hi Sourabh,
>
> On 21/01/25 5:24 pm, Sourabh Jain wrote:
>> insert_crashkernel_resources() adds crash memory to iomem_resource if
>> generic crashkernel reservation is enabled on an architecture.
>>
>
>> On PowerPC, system RAM is added to iomem_resource. See commit
>> c40dd2f766440 ("powerpc: Add System RAM to /proc/iomem").
>
> The changelog clearly says the patch was added for kdump. But 
> kexec-tools or kernel code for kdump on powerpc don't seem to have relied
> on that. Device-tree memory nodes were used instead? Wondering if
> dropping commit c40dd2f766440 is better..
>

Hmm. Even I thought of removing that patch for the same reason.
But I noticed the below comment:

/*
  * System memory should not be in /proc/iomem but various tools expect it
  * (eg kdump).
  */

Since they said eg kdump. I thought maybe it is needed for tools other 
then kdump.
But I think this was only added for kdump. And as we know this is not
used in kexec/kdump tool anymore.

I will revert `c40dd2f766440 ("powerpc: Add System RAM to /proc/iomem"). `
change in the next series and take feedback from the community.

Thanks for the review.

- Sourabh Jain

>
>>
>> Enabling generic crashkernel reservation on PowerPC leads to a conflict
>> when system RAM is added to iomem_resource because a part of the system
>> RAM, the crashkernel memory, has already been added to iomem_resource.
>>
>> The next commit in the series "powerpc/crash: use generic crashkernel
>> reservation" enables generic crashkernel reservation on PowerPC. If the
>> crashkernel is added to iomem_resource, the kernel fails to add
>> system RAM to /proc/iomem and prints the following traces:
>>
>> CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.13.0-rc2+
>> snip...
>> NIP [c000000002016b3c] add_system_ram_resources+0xf0/0x15c
>> LR [c000000002016b34] add_system_ram_resources+0xe8/0x15c
>> Call Trace:
>> [c00000000484bbc0] [c000000002016b34] 
>> add_system_ram_resources+0xe8/0x15c
>> [c00000000484bc20] [c000000000010a4c] do_one_initcall+0x7c/0x39c
>> [c00000000484bd00] [c000000002005418] do_initcalls+0x144/0x18c
>> [c00000000484bd90] [c000000002005714] kernel_init_freeable+0x21c/0x290
>> [c00000000484bdf0] [c0000000000110f4] kernel_init+0x2c/0x1b8
>> [c00000000484be50] [c00000000000dd3c] 
>> ret_from_kernel_user_thread+0x14/0x1c
>>
>> To avoid this, an architecture hook is added in
>> insert_crashkernel_resources(), allowing the architecture to decide
>> whether crashkernel memory should be added to iomem_resource.
>>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Baoquan he <bhe@redhat.com>
>> Cc: Hari Bathini <hbathini@linux.ibm.com>
>> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
>> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: kexec@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>   include/linux/crash_reserve.h | 11 +++++++++++
>>   kernel/crash_reserve.c        |  3 +++
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/include/linux/crash_reserve.h 
>> b/include/linux/crash_reserve.h
>> index 1fe7e7d1b214..f1205d044dae 100644
>> --- a/include/linux/crash_reserve.h
>> +++ b/include/linux/crash_reserve.h
>> @@ -18,6 +18,17 @@ int __init parse_crashkernel(char *cmdline, 
>> unsigned long long system_ram,
>>           unsigned long long *crash_size, unsigned long long 
>> *crash_base,
>>           unsigned long long *low_size, bool *high);
>>   +#ifdef CONFIG_ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
>> +
>> +#ifndef arch_add_crash_res_to_iomem
>> +static inline bool arch_add_crash_res_to_iomem(void)
>> +{
>> +    return true;
>> +}
>> +#endif
>> +
>> +#endif
>> +
>>   #ifdef CONFIG_ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
>>   #ifndef DEFAULT_CRASH_KERNEL_LOW_SIZE
>>   #define DEFAULT_CRASH_KERNEL_LOW_SIZE    (128UL << 20)
>> diff --git a/kernel/crash_reserve.c b/kernel/crash_reserve.c
>> index aff7c0fdbefa..190104f32fe1 100644
>> --- a/kernel/crash_reserve.c
>> +++ b/kernel/crash_reserve.c
>> @@ -460,6 +460,9 @@ void __init reserve_crashkernel_generic(unsigned 
>> long long crash_size,
>>   #ifndef HAVE_ARCH_ADD_CRASH_RES_TO_IOMEM_EARLY
>>   static __init int insert_crashkernel_resources(void)
>>   {
>> +    if (!arch_add_crash_res_to_iomem())
>> +        return 0;
>> +
>>       if (crashk_res.start < crashk_res.end)
>>           insert_resource(&iomem_resource, &crashk_res);
>