[PATCH v5 5/6] crash: forward memory_notify args to arch crash hotplug handler

Eric DeVolder eric.devolder at oracle.com
Wed Nov 23 05:03:15 AEDT 2022



On 11/20/22 17:25, Sourabh Jain wrote:
> The way memory hot remove is handled on PowerPC, it is hard to update
> the elfcorehdr without memory_notify args.
> 
> On PowePC memblock data structure is used to prepare elfcorehdr for kdump.
> Since the notifier used for memory hotplug crash handler get initiated
> before the memblock data structure update happens (as depicted below),
> the newly prepared elfcorehdr still holds the old memory regions. So if
> the system crash with obsolete elfcorehdr, makedumpfile failed to collect
> vmcore.
> 
> Sequence of actions done on PowerPC to serve the memory hot remove:
> 
>   Initiate memory hot remove
>            |
>            v
>   offline pages
>            |
>            v
>   initiate memory notify call chain
>   for MEM_OFFLINE event.
>   (same is used for crash update)
>            |
>            v
>   prepare new elfcorehdr for kdump using
>   memblock data structure
>            |
>            v
>   update memblock data structure
> 
> How passing memory_notify to arch crash hotplug handler will help?
> 
> memory_notify holds the start PFN and page count, with that base address
> and size of hot unplugged memory can calculated and same can be use to
> avoid hot unplugged memory region to get added in the elfcorehdr.
> 
> Signed-off-by: Sourabh Jain <sourabhjain at linux.ibm.com>
> ---
>   arch/powerpc/include/asm/kexec.h |  2 +-
>   arch/powerpc/kexec/core_64.c     |  3 ++-
>   arch/x86/include/asm/kexec.h     |  2 +-
>   arch/x86/kernel/crash.c          |  3 ++-
>   include/linux/kexec.h            |  3 ++-
>   kernel/crash_core.c              | 16 +++++++---------
>   6 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index a4c0a035cb407..f32126a22f6ed 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -109,7 +109,7 @@ int get_crash_memory_ranges(struct crash_mem **mem_ranges);
>   int machine_kexec_post_load(struct kimage *image);
>   #define machine_kexec_post_load machine_kexec_post_load
>   
> -void arch_crash_handle_hotplug_event(struct kimage *image, unsigned int hp_action);
> +void arch_crash_handle_hotplug_event(struct kimage *image, unsigned int hp_action, void *arg);

Be aware in the latest patch series, the hp_action argument was removed. You'll need to introduce it 
again since ppc needs it.

>   #define arch_crash_handle_hotplug_event arch_crash_handle_hotplug_event
>   
>   #endif
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 3dea1ce6b469c..6803d7e352a96 100644
> --- a/arch/powerpc/kexec/core_64.c
> +++ b/arch/powerpc/kexec/core_64.c
> @@ -575,11 +575,12 @@ int update_cpus_node(void *fdt)
>    * arch_crash_hotplug_handler() - Handle hotplug kexec segements changes FDT, elfcorehdr
>    * @image: the active struct kimage
>    * @hp_action: the hot un/plug action being handled
> + * @arg: struct memory_notify data handler
>    *
>    * To accurately reflect CPU hot un/plug changes, the FDT must be updated with the
>    * new list of CPUs.
>    */
> -void arch_crash_handle_hotplug_event(struct kimage *image, unsigned int hp_action)
> +void arch_crash_handle_hotplug_event(struct kimage *image, unsigned int hp_action, void *arg)
>   {
>   	void *fdt;
>   
> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
> index d72d347bd1d3b..e105a6b8a51b6 100644
> --- a/arch/x86/include/asm/kexec.h
> +++ b/arch/x86/include/asm/kexec.h
> @@ -213,7 +213,7 @@ extern crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
>   extern void kdump_nmi_shootdown_cpus(void);
>   
>   void arch_crash_handle_hotplug_event(struct kimage *image,
> -				    unsigned int hp_action);
> +				     unsigned int hp_action, void *arg);
>   #define arch_crash_handle_hotplug_event arch_crash_handle_hotplug_event
>   
>   #ifdef CONFIG_HOTPLUG_CPU
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 2687acf28977f..428121560c3cd 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -457,13 +457,14 @@ int crash_load_segments(struct kimage *image)
>    * arch_crash_handle_hotplug_event() - Handle hotplug elfcorehdr changes
>    * @image: the active struct kimage
>    * @hp_action: the hot un/plug action being handled
> + * @arg: struct memory_notify data handler
>    *
>    * To accurately reflect hot un/plug changes, the new elfcorehdr
>    * is prepared in a kernel buffer, and then it is written on top
>    * of the existing/old elfcorehdr.
>    */
>   void arch_crash_handle_hotplug_event(struct kimage *image,
> -				    unsigned int hp_action)
> +				     unsigned int hp_action, void *arg)
>   {
>   	unsigned long mem, memsz;
>   	unsigned long elfsz = 0;
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index e2dbbcbf37dcc..43b668484264b 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -537,7 +537,8 @@ static inline void arch_unmap_crash_pages(void *ptr)
>   
>   #ifndef arch_crash_handle_hotplug_event
>   static inline void arch_crash_handle_hotplug_event(struct kimage *image,
> -						  unsigned int hp_action)
> +						   unsigned int hp_action,
> +						   void *arg)
>   {
>   }
>   #endif
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index f6cccdcadc9f3..3132466b5e429 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -641,7 +641,7 @@ subsys_initcall(crash_save_vmcoreinfo_init);
>    * list of segments it checks (since the elfcorehdr changes and thus
>    * would require an update to purgatory itself to update the digest).
>    */
> -static void handle_hotplug_event(unsigned int hp_action, unsigned int cpu)
> +static void handle_hotplug_event(unsigned int hp_action, unsigned int cpu, void *arg)
>   {
>   	/* Obtain lock while changing crash information */
>   	if (kexec_trylock()) {
> @@ -704,7 +704,7 @@ static void handle_hotplug_event(unsigned int hp_action, unsigned int cpu)
>   					cpu : KEXEC_CRASH_HP_INVALID_CPU;
>   
>   			/* Now invoke arch-specific update handler */
> -			arch_crash_handle_hotplug_event(image, hp_action);
> +			arch_crash_handle_hotplug_event(image, hp_action, arg);
>   
>   			/* No longer handling a hotplug event */
>   			image->hotplug_event = false;
> @@ -719,17 +719,15 @@ static void handle_hotplug_event(unsigned int hp_action, unsigned int cpu)
>   	}
>   }
>   
> -static int crash_memhp_notifier(struct notifier_block *nb, unsigned long val, void *v)
> +static int crash_memhp_notifier(struct notifier_block *nb, unsigned long val, void *arg)
>   {
>   	switch (val) {
>   	case MEM_ONLINE:
> -		handle_hotplug_event(KEXEC_CRASH_HP_ADD_MEMORY,
> -			KEXEC_CRASH_HP_INVALID_CPU);
> +		handle_hotplug_event(KEXEC_CRASH_HP_ADD_MEMORY, 0, arg);
>   		break;
>   
>   	case MEM_OFFLINE:
> -		handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_MEMORY,
> -			KEXEC_CRASH_HP_INVALID_CPU);
> +		handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_MEMORY, 0, arg);
>   		break;
>   	}
>   	return NOTIFY_OK;
> @@ -742,13 +740,13 @@ static struct notifier_block crash_memhp_nb = {
>   
>   static int crash_cpuhp_online(unsigned int cpu)
>   {
> -	handle_hotplug_event(KEXEC_CRASH_HP_ADD_CPU, cpu);
> +	handle_hotplug_event(KEXEC_CRASH_HP_ADD_CPU, cpu, NULL);
>   	return 0;
>   }
>   
>   static int crash_cpuhp_offline(unsigned int cpu)
>   {
> -	handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_CPU, cpu);
> +	handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_CPU, cpu, NULL);
>   	return 0;
>   }
>   
> 

Otherwise this lgtm.
eric


More information about the Linuxppc-dev mailing list