Re: [PATCH] powerpc/crash: Add only the onlined CPU's node to the crash FDT

[samir <samir@linux.ibm.com>]

Hi All,

On 04/11/26 11:08 am, Vishal Chourasia wrote:
> A CPU coming online never changes the nodes that are already in the FDT,
> so add only the node of that CPU. update_crash_fdt() looks up the device
> node of the onlined CPU and does nothing if the FDT already has a node of
> that name, which is the case when an offlined CPU comes back online. Only
> when the node is missing, the crash image is invalidated while the node is
> created with fdt_add_subnode() and its properties are copied from the live
> device node with fdt_setprop(). All of this is done directly in
> update_crash_fdt().
>
> fdt_add_subnode() inserts the node as the first child of /cpus, which is
> the same position a node attached to the live device tree takes, so the
> FDT keeps the order of the live tree.
>
> Signed-off-by: Vishal Chourasia <vishalc@linux.ibm.com>
> ---
>   arch/powerpc/kexec/crash.c | 65 +++++++++++++++++++++++++++++++-------
>   arch/x86/kernel/crash.c    |  2 +-
>   kernel/crash_core.c        | 11 +++++--
>   3 files changed, 64 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
> index 775895f31037..ca37006bddea 100644
> --- a/arch/powerpc/kexec/crash.c
> +++ b/arch/powerpc/kexec/crash.c
> @@ -18,6 +18,7 @@
>   #include <linux/types.h>
>   #include <linux/libfdt.h>
>   #include <linux/memory.h>
> +#include <linux/of.h>
>   
>   #include <asm/processor.h>
>   #include <asm/machdep.h>
> @@ -584,32 +585,71 @@ static int get_fdt_index(struct kimage *image)
>   }
>   
>   /**
> - * update_crash_fdt - updates the cpus node of the crash FDT.
> + * update_crash_fdt - add the node of the onlined CPU to the crash FDT.
>    *
>    * @image: a pointer to kexec_crash_image
> + * @cpu: the CPU that was onlined
> + *
> + * The node is added as the first child of /cpus, the same position a node
> + * attached to the live device tree takes. Nothing is done if the FDT already
> + * has the node, for example when an offlined CPU comes back.
>    */
> -static void update_crash_fdt(struct kimage *image)
> +static void update_crash_fdt(struct kimage *image, unsigned int cpu)
>   {
> +	struct device_node *dn;
> +	struct property *pp;
>   	void *fdt;
> -	int fdt_index;
> +	int fdt_index, cpus_offset, node_offset;
> +
> +	dn = of_get_cpu_node(cpu, NULL);
> +	if (!dn) {
> +		pr_err("No device node found for CPU %u\n", cpu);
> +		return;
> +	}
>   
>   	fdt_index = get_fdt_index(image);
>   	if (fdt_index < 0) {
>   		pr_err("Unable to locate FDT segment.\n");
> -		return;
> +		goto put;
>   	}
>   
>   	fdt = __va((void *)image->segment[fdt_index].mem);
>   
> -	/* Temporarily invalidate the crash image while it is replaced */
> +	cpus_offset = fdt_path_offset(fdt, "/cpus");
> +	if (cpus_offset < 0) {
> +		pr_err("Malformed device tree: error reading /cpus node: %s\n",
> +		       fdt_strerror(cpus_offset));
> +		goto put;
> +	}
> +
> +	/* Node already present, e.g. an offlined CPU came back online */
> +	if (fdt_subnode_offset(fdt, cpus_offset, dn->full_name) >= 0)
> +		goto put;
> +
> +	/* Temporarily invalidate the crash image while it is updated */
>   	xchg(&kexec_crash_image, NULL);
>   
> -	/* update FDT to reflect changes in CPU resources */
> -	if (update_cpus_node(fdt))
> -		pr_err("Failed to update crash FDT");
> +	node_offset = fdt_add_subnode(fdt, cpus_offset, dn->full_name);
> +	if (node_offset < 0) {
> +		pr_err("Unable to add %s subnode: %s\n", dn->full_name,
> +		       fdt_strerror(node_offset));
> +		goto out;
> +	}
>   
> +	for_each_property_of_node(dn, pp) {
> +		int ret = fdt_setprop(fdt, node_offset, pp->name, pp->value, pp->length);
> +
> +		if (ret < 0) {
> +			pr_err("Unable to add %s property: %s\n", pp->name,
> +			       fdt_strerror(ret));
> +			break;
> +		}
> +	}
> +out:
>   	/* The crash image is now valid once again */
>   	xchg(&kexec_crash_image, image);
> +put:
> +	of_node_put(dn);
>   }
>   
>   int arch_crash_hotplug_support(struct kimage *image, unsigned long kexec_flags)
> @@ -625,10 +665,11 @@ int arch_crash_hotplug_support(struct kimage *image, unsigned long kexec_flags)
>    * arch_crash_handle_hotplug_event - Handle crash CPU/Memory hotplug events to update the
>    *				     necessary kexec segments based on the hotplug event.
>    * @image: a pointer to kexec_crash_image
> - * @arg: struct memory_notify handler for memory hotplug case and NULL for CPU hotplug case.
> + * @arg: struct memory_notify handler for memory hotplug case and pointer to the
> + *	 cpu number for CPU hotplug case.
>    *
>    * Update the kdump image based on the type of hotplug event, represented by image->hp_action.
> - * CPU add: Update the FDT segment to include the newly added CPU.
> + * CPU add: Add the node of the newly added CPU to the FDT segment, if missing.
>    * CPU remove: No action is needed, with the assumption that it's okay to have offline CPUs
>    *	       part of the FDT.
>    * Memory add/remove: No action is taken as this is not yet supported.
> @@ -636,13 +677,15 @@ int arch_crash_hotplug_support(struct kimage *image, unsigned long kexec_flags)
>   void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
>   {
>   	struct memory_notify *mn;
> +	unsigned int cpu;
>   
>   	switch (image->hp_action) {
>   	case KEXEC_CRASH_HP_REMOVE_CPU:
>   		return;
>   
>   	case KEXEC_CRASH_HP_ADD_CPU:
> -		update_crash_fdt(image);
> +		cpu = *(unsigned int *)arg;
> +		update_crash_fdt(image, cpu);
>   		break;
>   
>   	case KEXEC_CRASH_HP_REMOVE_MEMORY:
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index e6f23933a6df..1660f26f3a44 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -441,7 +441,7 @@ unsigned int arch_crash_get_elfcorehdr_size(void)
>    * arch_crash_handle_hotplug_event() - Handle hotplug elfcorehdr changes
>    * @image: a pointer to kexec_crash_image
>    * @arg: struct memory_notify handler for memory hotplug case and
> - *       NULL for CPU hotplug case.
> + *       pointer to the cpu number (unused here) for CPU hotplug case.
>    *
>    * Prepare the new elfcorehdr and replace the existing elfcorehdr.
>    */
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index d0bd2d0cf899..53d4f2c1760c 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -651,6 +651,9 @@ int crash_check_hotplug_support(void)
>    * NR_CPUS and CRASH_MAX_MEMORY_RANGES. The elfcorehdr is
>    * excluded from SHA verification by default if the architecture
>    * supports crash hotplug.
> + *
> + * @arg is a struct memory_notify pointer for memory hotplug events and a
> + * pointer to the unsigned int cpu number for CPU hotplug events.
>    */
>   static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int cpu, void *arg)
>   {
> @@ -676,10 +679,14 @@ static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int cpu,
>   		goto out;
>   
>   	if (hp_action == KEXEC_CRASH_HP_ADD_CPU ||
> -		hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
> +		hp_action == KEXEC_CRASH_HP_REMOVE_CPU) {
>   		pr_debug("hp_action %u, cpu %u\n", hp_action, cpu);
> -	else
> +		/* CPU events are raised with no argument, the CPU number is passed instead */
> +		WARN_ON(arg);
> +		arg = &cpu;
> +	} else {
>   		pr_debug("hp_action %u\n", hp_action);
> +	}
>   
>   	/*
>   	 * The elfcorehdr_index is set to -1 when the struct kimage

I tested the patch with the following configuration and performed 
performance and functional validation

Test Configuration:
-------------------
System         : PowerPC LPAR
LPAR Mode      : Dedicated
CPU/Memory     : 50 cores / 80 GB
Kernel Version : 7.4.0-rc1

Case 1: SMT Switching Performance
---------------------------------
We measured SMT ON/OFF times with and without the patches, with
expedited RCU enabled via /sys/kernel/rcu_expedited and the crash
kernel image loaded (kexec_crash_loaded verified).

SMT Mode | Without Patch (s) | With Patch (s) | Improvement (%)
----------------------------------------------------------------
SMT ON   |      66.331       |      6.155     |  90.72% faster
SMT OFF  |       6.402       |      6.339     |   0.98% faster

Observation:
With the crash kernel loaded, SMT ON takes significantly longer
without the patches. The patches reduce SMT ON time by 90.72%,
while SMT OFF performance remains nearly unchanged.

Case 2: Functional Validation
-----------------------------

Test 1: Crash Dump Collection
-----------------------------
After performing CPU DLPAR with the crash kernel enabled, we
triggered a crash on CPU 3 using:

echo 1 > /proc/sys/kernel/sysrq
taskset -c 3 bash -c "echo c > /proc/sysrq-trigger"

Result: PASS - The crash kernel successfully collected the vmcore.

Test 2: CPU/NUMA Node Verification
----------------------------------
Verified that the newly added CPUs after DLPAR were correctly
reflected in NUMA node 5, including the updated CPU count and
adjusted logical CPU numbering.

Result: PASS

Overall, the patch significantly improve SMT ON performance,
and both functional validation tests passed.


Regards,
Samir