On Tue, Apr 19, 2022 at 04:58:47PM -0500, Eric DeVolder wrote: > So in taking this concept and looking at, in particular, patch 4/8 > "crash: add generic infrastructure for crash hotplug support", I'm not > exactly sure how to apply this technique. So I took your patch 4 and maimed into what I think it should look like, see below. Now there's a single ifdef there and the registration routines are wrapped in IS_ENABLED() so that you register a callback only when the respective stuff - HOTPLUG_CPU or MEMORY_HOTPLUG - is enabled. Otherwise the couple of functions are unused but that's not that big of a deal. I've also fixed up some other issues I've encountered along the way. Holler if there are questions. HTH. --- diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h index de62a722431e..b3c32e04d3f0 100644 --- a/include/linux/crash_core.h +++ b/include/linux/crash_core.h @@ -84,4 +84,12 @@ int parse_crashkernel_high(char *cmdline, unsigned long long system_ram, int parse_crashkernel_low(char *cmdline, unsigned long long system_ram, unsigned long long *crash_size, unsigned long long *crash_base); +#define KEXEC_CRASH_HP_REMOVE_CPU 0 +#define KEXEC_CRASH_HP_ADD_CPU 1 +#define KEXEC_CRASH_HP_REMOVE_MEMORY 2 +#define KEXEC_CRASH_HP_ADD_MEMORY 3 + +struct kimage; +void arch_crash_handle_hotplug_event(struct kimage *image, unsigned int hp_action, + unsigned int cpu); #endif /* LINUX_CRASH_CORE_H */ diff --git a/include/linux/kexec.h b/include/linux/kexec.h index 02daff1f47dd..d907a1f0d3da 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -307,13 +307,11 @@ struct kimage { /* Information for loading purgatory */ struct purgatory_info purgatory_info; -#ifdef CONFIG_CRASH_HOTPLUG bool hotplug_event; unsigned int offlinecpu; bool elfcorehdr_index_valid; int elfcorehdr_index; #endif -#endif #ifdef CONFIG_IMA_KEXEC /* Virtual address of IMA measurement buffer for kexec syscall */ @@ -329,15 +327,6 @@ struct kimage { unsigned long elf_load_addr; }; -#ifdef CONFIG_CRASH_HOTPLUG -void arch_crash_hotplug_handler(struct kimage *image, - unsigned int hp_action, unsigned int cpu); -#define KEXEC_CRASH_HP_REMOVE_CPU 0 -#define KEXEC_CRASH_HP_ADD_CPU 1 -#define KEXEC_CRASH_HP_REMOVE_MEMORY 2 -#define KEXEC_CRASH_HP_ADD_MEMORY 3 -#endif /* CONFIG_CRASH_HOTPLUG */ - /* kexec interface functions */ extern void machine_kexec(struct kimage *image); extern int machine_kexec_prepare(struct kimage *image); diff --git a/kernel/crash_core.c b/kernel/crash_core.c index ecf746243ab2..a6c4ee1d5c86 100644 --- a/kernel/crash_core.c +++ b/kernel/crash_core.c @@ -497,57 +497,50 @@ static int __init crash_save_vmcoreinfo_init(void) subsys_initcall(crash_save_vmcoreinfo_init); -#ifdef CONFIG_CRASH_HOTPLUG -void __weak arch_crash_hotplug_handler(struct kimage *image, - unsigned int hp_action, unsigned int cpu) +void __weak arch_crash_handle_hotplug_event(struct kimage *image, unsigned int hp_action, + unsigned int cpu) { - pr_warn("crash hp: %s not implemented", __func__); + WARN(1, "crash hotplug handler not implemented"); } -static void crash_hotplug_handler(unsigned int hp_action, - unsigned int cpu) +#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_MEMORY_HOTPLUG) +static void handle_hotplug_event(unsigned int hp_action, unsigned int cpu) { - /* Obtain lock while changing crash information */ - if (!mutex_trylock(&kexec_mutex)) + if (!kexec_crash_image) return; - /* Check kdump is loaded */ - if (kexec_crash_image) { - pr_debug("crash hp: hp_action %u, cpu %u", hp_action, cpu); + mutex_lock(&kexec_mutex); - /* Needed in order for the segments to be updated */ - arch_kexec_unprotect_crashkres(); + pr_debug("crash hotplug: hp_action %u, cpu %u", hp_action, cpu); - /* Flag to differentiate between normal load and hotplug */ - kexec_crash_image->hotplug_event = true; + /* Needed in order for the segments to be updated */ + arch_kexec_unprotect_crashkres(); - /* Now invoke arch-specific update handler */ - arch_crash_hotplug_handler(kexec_crash_image, hp_action, cpu); + /* Flag to differentiate between normal load and hotplug */ + kexec_crash_image->hotplug_event = true; - /* No longer handling a hotplug event */ - kexec_crash_image->hotplug_event = false; + /* Now invoke arch-specific update handler */ + arch_crash_handle_hotplug_event(kexec_crash_image, hp_action, cpu); - /* Change back to read-only */ - arch_kexec_protect_crashkres(); - } + /* No longer handling a hotplug event */ + kexec_crash_image->hotplug_event = false; + + /* Change back to read-only */ + arch_kexec_protect_crashkres(); /* Release lock now that update complete */ mutex_unlock(&kexec_mutex); } -#if defined(CONFIG_MEMORY_HOTPLUG) -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 *v) { - struct memory_notify *mhp = v; - switch (val) { case MEM_ONLINE: - crash_hotplug_handler(KEXEC_CRASH_HP_ADD_MEMORY, -1U); + handle_hotplug_event(KEXEC_CRASH_HP_ADD_MEMORY, -1U); break; case MEM_OFFLINE: - crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_MEMORY, -1U); + handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_MEMORY, -1U); break; } return NOTIFY_OK; @@ -557,38 +550,33 @@ static struct notifier_block crash_memhp_nb = { .notifier_call = crash_memhp_notifier, .priority = 0 }; -#endif -#if defined(CONFIG_HOTPLUG_CPU) static int crash_cpuhp_online(unsigned int cpu) { - crash_hotplug_handler(KEXEC_CRASH_HP_ADD_CPU, cpu); + handle_hotplug_event(KEXEC_CRASH_HP_ADD_CPU, cpu); return 0; } static int crash_cpuhp_offline(unsigned int cpu) { - crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_CPU, cpu); + handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_CPU, cpu); return 0; } -#endif static int __init crash_hotplug_init(void) { int result = 0; -#if defined(CONFIG_MEMORY_HOTPLUG) - register_memory_notifier(&crash_memhp_nb); -#endif + if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG)) + register_memory_notifier(&crash_memhp_nb); -#if defined(CONFIG_HOTPLUG_CPU) - result = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, - "crash/cpuhp", - crash_cpuhp_online, crash_cpuhp_offline); -#endif + if (IS_ENABLED(CONFIG_HOTPLUG_CPU)) + result = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, + "crash/cpuhp", + crash_cpuhp_online, + crash_cpuhp_offline); return result; } - subsys_initcall(crash_hotplug_init); -#endif /* CONFIG_CRASH_HOTPLUG */ +#endif -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec