Re: [PATCH] [RFC] crash: Lock-free crash hotplug support reporting

On 09/07/24 at 10:30am, Sourabh Jain wrote:

Hello Baoquan,

Do you think this patch would help reduce lock contention when
CPU/Memory resources are removed in bulk from a system?

.....snip...
--

quoted

  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;

I may not understand this well. Both kexec_load and kexec_file_load set
hotplug_support, crash_check_hotplug_support and
crash_handle_hotplug_event are to check the flag. How do you guarantee
the cpu/memory sysfs checking won't have race with kexec_load and
kexec_file_load?

And here I see taking crash_hotplug_lock() is unnecessary in
crash_check_hotplug_support() because it does't have race with
crash_handle_hotplug_event().

quoted

  }
  /*

@@ -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;