On 02/12/24 at 07:27pm, Sourabh Jain wrote: > Hello Baoquan, > > On 05/02/24 08:40, Baoquan He wrote: > > Hi Sourabh, > > ...... > > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > > > index 802052d9c64b..7880d74dc5c4 100644 > > > --- a/include/linux/kexec.h > > > +++ b/include/linux/kexec.h > > > @@ -317,8 +317,8 @@ struct kimage { > > > /* If set, we are using file mode kexec syscall */ > > > unsigned int file_mode:1; > > > #ifdef CONFIG_CRASH_HOTPLUG > > > - /* If set, allow changes to elfcorehdr of kexec_load'd image */ > > > - unsigned int update_elfcorehdr:1; > > > + /* If set, allow changes to kexec segments of kexec_load'd image */ > > The code comment doesn't reflect the usage of the flag. > I should have updated the comment to indicate that this flag is for both > system calls. > More comments below. > > > You set it too > > when it's kexec_file_load. Speaking of this, I do wonder why you need > > set it too for kexec_file_load, > If we do this one can just access image->hotplug_support to find hotplug > support for currently loaded kdump image without bothering about which > system call was used to load the kdump image. > > > and why we have > > arch_crash_hotplug_support(), then crash_check_hotplug_support() both of > > which have the same effect. > > arch_crash_hotplug_support(): This function processes the kexec flags and > finds the > hotplug support for the kdump image. Based on the return value of this > function, > the image->hotplug_support attribute is set. > > Now, once the kdump image is loaded, we no longer have access to the kexec > flags. > Therefore, crash_check_hotplug_support simply returns the value of > image->hotplug_support > when user space accesses the following sysfs files: > /sys/devices/system/[cpu|memory]/crash_hotplug. > > To keep things simple, I have introduced two functions: One function > processes the kexec flags > and determines the hotplug support for the image being loaded. And other > function simply > accesses image->hotplug_support and advertises CPU/Memory hotplug support to > userspace. >From the function name and their functionality, they seems to be duplicated, even though it's different from the internal detail. This could bring a little confusion to code understanding. It's fine, we can refactor them if needed in the future. So let's keep it as the patch is. Thanks. > > > > > > + unsigned int hotplug_support:1; > > > #endif > > > #ifdef ARCH_HAS_KIMAGE_ARCH > > > @@ -396,9 +396,10 @@ bool kexec_load_permitted(int kexec_image_type); > > > /* List of defined/legal kexec flags */ > > > #ifndef CONFIG_KEXEC_JUMP > > > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR) > > > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_CRASH_HOTPLUG_SUPPORT) > > > #else > > > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR) > > > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \ > > > + KEXEC_CRASH_HOTPLUG_SUPPORT) > > > #endif > > > /* List of defined/legal kexec file flags */ > > > @@ -486,14 +487,18 @@ static inline void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages) { > > > static inline void arch_crash_handle_hotplug_event(struct kimage *image, void *arg) { } > > > #endif > > > -int crash_check_update_elfcorehdr(void); > > > +int crash_check_hotplug_support(void); > > > -#ifndef crash_hotplug_cpu_support > > > -static inline int crash_hotplug_cpu_support(void) { return 0; } > > > -#endif > > > +#ifndef arch_crash_hotplug_support > > > +static inline int arch_crash_hotplug_support(struct kimage *image, unsigned long kexec_flags) > > > +{ > > > -#ifndef crash_hotplug_memory_support > > > -static inline int crash_hotplug_memory_support(void) { return 0; } > > > +#ifdef CONFIG_KEXEC_FILE > > > + if (image->file_mode) > > > + return 1; > > > +#endif > > > + return kexec_flags & KEXEC_CRASH_HOTPLUG_SUPPORT; > > > +} > > > #endif > > > #ifndef crash_get_elfcorehdr_size ...... _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec