[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <641be54a-440f-475b-a62d-fac157ecbbcb@linux.ibm.com>
Date: Sat, 7 Sep 2024 10:30:59 +0530
From: Sourabh Jain <sourabhjain@...ux.ibm.com>
To: bhe@...hat.com
Cc: Hari Bathini <hbathini@...ux.ibm.com>,
Mahesh Salgaonkar <mahesh@...ux.ibm.com>, kexec@...ts.infradead.org,
linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
x86@...nel.org
Subject: Re: [PATCH] [RFC] crash: Lock-free crash hotplug support reporting
Hello Baoquan,
Do you think this patch would help reduce lock contention when
CPU/Memory resources are removed in bulk from a system?
Thanks,
Sourabh Jain
On 23/08/24 17:22, Sourabh Jain wrote:
> On a CPU/Memory hotplug event, the kexec lock is taken to update the
> kdump image. At the same time, this lock is also required to report
> the support for crash hotplug to user-space via the
> /sys/devices/system/[cpu|memory]/crash_hotplug sysfs interface, to
> avoid kdump reload.
>
> The kexec lock is needed to report crash hotplug support because the
> crash_hotplug variable, which tracks crash hotplug support, is part of
> the kdump image, and the kdump image needs to be updated during a
> hotplug event.
>
> Given that only one kdump image can be loaded at any given time, the
> crash_hotplug variable can be placed outside the kdump image and set or
> reset during kdump image load and unload. This allows crash hotplug
> support to be reported without taking the kexec lock.
>
> This would help in situation where CPU/Memory resource are hotplug from
> system in bulk.
>
> Commit e2a8f20dd8e9 ("Crash: add lock to serialize crash hotplug
> handling") introduced to serialize the kexec lock during bulk CPU/Memory
> hotplug events. However, with these changes, the kexec lock for crash
> hotplug support reporting can be avoided altogether.
>
> Cc: Hari Bathini <hbathini@...ux.ibm.com>
> Cc: Mahesh Salgaonkar <mahesh@...ux.ibm.com>
> Cc: kexec@...ts.infradead.org
> Cc: linuxppc-dev@...ts.ozlabs.org
> Cc: linux-kernel@...r.kernel.org
> Cc: x86@...nel.org
> Signed-off-by: Sourabh Jain <sourabhjain@...ux.ibm.com>
> ---
> include/linux/kexec.h | 11 ++++-------
> kernel/crash_core.c | 27 +++++++++------------------
> kernel/kexec.c | 5 ++++-
> kernel/kexec_file.c | 7 ++++++-
> 4 files changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index f0e9f8eda7a3..bd755ba6bac4 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -318,13 +318,6 @@ struct kimage {
> unsigned int preserve_context : 1;
> /* If set, we are using file mode kexec syscall */
> unsigned int file_mode:1;
> -#ifdef CONFIG_CRASH_HOTPLUG
> - /* If set, it is safe to update kexec segments that are
> - * excluded from SHA calculation.
> - */
> - unsigned int hotplug_support:1;
> -#endif
> -
> #ifdef ARCH_HAS_KIMAGE_ARCH
> struct kimage_arch arch;
> #endif
> @@ -370,6 +363,10 @@ struct kimage {
> unsigned long elf_load_addr;
> };
>
> +#ifdef CONFIG_CRASH_HOTPLUG
> +extern unsigned int crash_hotplug_support;
> +#endif
> +
> /* 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 63cf89393c6e..3428deba0070 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -30,6 +30,13 @@
> #include "kallsyms_internal.h"
> #include "kexec_internal.h"
>
> +#ifdef CONFIG_CRASH_HOTPLUG
> +/* if set, it is safe to update kexec segments that are
> + * excluded from sha calculation.
> + */
> +unsigned int crash_hotplug_support;
> +#endif
> +
> /* Per cpu memory for storing cpu states in case of system crash. */
> note_buf_t __percpu *crash_notes;
>
> @@ -500,23 +507,7 @@ static DEFINE_MUTEX(__crash_hotplug_lock);
> */
> int crash_check_hotplug_support(void)
> {
> - int rc = 0;
> -
> - crash_hotplug_lock();
> - /* Obtain lock while reading crash information */
> - if (!kexec_trylock()) {
> - pr_info("kexec_trylock() failed, elfcorehdr may be inaccurate\n");
> - crash_hotplug_unlock();
> - return 0;
> - }
> - if (kexec_crash_image) {
> - rc = kexec_crash_image->hotplug_support;
> - }
> - /* Release lock now that update complete */
> - kexec_unlock();
> - crash_hotplug_unlock();
> -
> - return rc;
> + return crash_hotplug_support;
> }
>
> /*
> @@ -552,7 +543,7 @@ static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int cpu,
> image = kexec_crash_image;
>
> /* Check that kexec segments update is permitted */
> - if (!image->hotplug_support)
> + if (!crash_hotplug_support)
> goto out;
>
> if (hp_action == KEXEC_CRASH_HP_ADD_CPU ||
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index a6b3f96bb50c..d5c6b51eaa8b 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -116,6 +116,9 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
> /* Uninstall image */
> kimage_free(xchg(dest_image, NULL));
> ret = 0;
> +#ifdef CONFIG_CRASH_HOTPLUG
> + crash_hotplug_support = 0;
> +#endif
> goto out_unlock;
> }
> if (flags & KEXEC_ON_CRASH) {
> @@ -136,7 +139,7 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>
> #ifdef CONFIG_CRASH_HOTPLUG
> if ((flags & KEXEC_ON_CRASH) && arch_crash_hotplug_support(image, flags))
> - image->hotplug_support = 1;
> + crash_hotplug_support = 1;
> #endif
>
> ret = machine_kexec_prepare(image);
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 3d64290d24c9..b326edb90fd7 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -378,7 +378,7 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>
> #ifdef CONFIG_CRASH_HOTPLUG
> if ((flags & KEXEC_FILE_ON_CRASH) && arch_crash_hotplug_support(image, flags))
> - image->hotplug_support = 1;
> + crash_hotplug_support = 1;
> #endif
>
> ret = machine_kexec_prepare(image);
> @@ -432,6 +432,11 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> arch_kexec_protect_crashkres();
> #endif
>
> +#ifdef CONFIG_CRASH_HOTPLUG
> + if (flags & KEXEC_FILE_UNLOAD)
> + crash_hotplug_support = 0;
> +#endif
> +
> kexec_unlock();
> kimage_free(image);
> return ret;
Powered by blists - more mailing lists