Re: [PATCH v14 2/6] crash: make CPU and Memory hotplug support reporting flexible

[Sourabh Jain <sourabhjain@linux.ibm.com>]

Hello Baoquan,

On 14/12/23 19:43, Baoquan He wrote:
> On 12/11/23 at 02:00pm, Sourabh Jain wrote:
>> Architectures' specific functions `arch_crash_hotplug_cpu_support()` and
>> `arch_crash_hotplug_memory_support()` advertise the kernel's capability
>> to update the kdump image on CPU and Memory hotplug events to userspace
>> via the sysfs interface. These architecture-specific functions need to
>> access various attributes of the `kexec_crash_image` object to determine
>> whether the kernel can update the kdump image and advertise this
>> information to userspace accordingly.
>>
>> As the architecture-specific code is not exposed to the APIs required to
>> acquire the lock for accessing the `kexec_crash_image` object, it calls
>> a generic function, `crash_check_update_elfcorehdr()`, to determine
>> whether the kernel can update the kdump image or not.
>>
>> The lack of access to the `kexec_crash_image` object in
>> architecture-specific code restricts architectures from performing
>> additional architecture-specific checks required to determine if the
>> kdump image is updatable or not. For instance, on PowerPC, the kernel
>> can update the kdump image only if both the elfcorehdr and FDT are
>> marked as updatable for the `kexec_load` system call.
>>
>> So define two generic functions, `crash_hotplug_cpu_support()` and
>> `crash_hotplug_memory_support()`, which are called when userspace
>> attempts to read the crash CPU and Memory hotplug support via the sysfs
>> interface. These functions take the necessary locks needed to access the
>> `kexec_crash_image` object and then forward it to the
>> architecture-specific handler to do the rest.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> Cc: Akhil Raj <lf32.dev@gmail.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
>> Cc: Baoquan He <bhe@redhat.com>
>> Cc: Borislav Petkov (AMD) <bp@alien8.de>
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> Cc: Dave Young <dyoung@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Eric DeVolder <eric.devolder@oracle.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Hari Bathini <hbathini@linux.ibm.com>
>> Cc: Laurent Dufour <laurent.dufour@fr.ibm.com>
>> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Mimi Zohar <zohar@linux.ibm.com>
>> Cc: Naveen N Rao <naveen@kernel.org>
>> Cc: Oscar Salvador <osalvador@suse.de>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Valentin Schneider <vschneid@redhat.com>
>> Cc: Vivek Goyal <vgoyal@redhat.com>
>> Cc: kexec@lists.infradead.org
>> Cc: x86@kernel.org
>> ---
>>   arch/x86/include/asm/kexec.h |  8 ++++----
>>   arch/x86/kernel/crash.c      | 20 +++++++++++++++-----
>>   include/linux/kexec.h        | 13 +++++++------
>>   kernel/crash_core.c          | 23 +++++++++++++++++------
>>   4 files changed, 43 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
>> index 9bb6607e864e..5c88d27b086d 100644
>> --- a/arch/x86/include/asm/kexec.h
>> +++ b/arch/x86/include/asm/kexec.h
>> @@ -212,13 +212,13 @@ void arch_crash_handle_hotplug_event(struct kimage *image, void *arg);
>>   #define arch_crash_handle_hotplug_event arch_crash_handle_hotplug_event
>>   
>>   #ifdef CONFIG_HOTPLUG_CPU
>> -int arch_crash_hotplug_cpu_support(void);
>> -#define crash_hotplug_cpu_support arch_crash_hotplug_cpu_support
>> +int arch_crash_hotplug_cpu_support(struct kimage *image);
>> +#define arch_crash_hotplug_cpu_support arch_crash_hotplug_cpu_support
>>   #endif
>>   
>>   #ifdef CONFIG_MEMORY_HOTPLUG
>> -int arch_crash_hotplug_memory_support(void);
>> -#define crash_hotplug_memory_support arch_crash_hotplug_memory_support
>> +int arch_crash_hotplug_memory_support(struct kimage *image);
>> +#define arch_crash_hotplug_memory_support arch_crash_hotplug_memory_support
>>   #endif
>>   
>>   unsigned int arch_crash_get_elfcorehdr_size(void);
>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>> index 0d7b2657beb6..ad5941665589 100644
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -398,18 +398,28 @@ int crash_load_segments(struct kimage *image)
>>   #undef pr_fmt
>>   #define pr_fmt(fmt) "crash hp: " fmt
>>   
>> -/* These functions provide the value for the sysfs crash_hotplug nodes */
>> +#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_MEMORY_HOTPLUG)
>> +static int crash_hotplug_support(struct kimage *image)
>> +{
>> +	if (image->file_mode)
>> +		return 1;
>> +
>> +	return image->update_elfcorehdr;
>> +}
>> +#endif
>> +
>>   #ifdef CONFIG_HOTPLUG_CPU
>> -int arch_crash_hotplug_cpu_support(void)
>> +/* These functions provide the value for the sysfs crash_hotplug nodes */
>> +int arch_crash_hotplug_cpu_support(struct kimage *image)
>>   {
>> -	return crash_check_update_elfcorehdr();
>> +	return crash_hotplug_support(image);
>>   }
>>   #endif
>>   
>>   #ifdef CONFIG_MEMORY_HOTPLUG
>> -int arch_crash_hotplug_memory_support(void)
>> +int arch_crash_hotplug_memory_support(struct kimage *image)
>>   {
>> -	return crash_check_update_elfcorehdr();
>> +	return crash_hotplug_support(image);
>>   }
>>   #endif
>>   
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index ee28c09a7fb0..0f6ea35879ee 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -486,16 +486,17 @@ 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);
>> -
>> -#ifndef crash_hotplug_cpu_support
>> -static inline int crash_hotplug_cpu_support(void) { return 0; }
>> +#ifndef arch_crash_hotplug_cpu_support
>> +static inline int arch_crash_hotplug_cpu_support(struct kimage *image) { return 0; }
>>   #endif
>>   
>> -#ifndef crash_hotplug_memory_support
>> -static inline int crash_hotplug_memory_support(void) { return 0; }
>> +#ifndef arch_crash_hotplug_memory_support
>> +static inline int arch_crash_hotplug_memory_support(struct kimage *image) { return 0; }
>>   #endif
>>   
>> +extern int crash_hotplug_cpu_support(void);
>> +extern int crash_hotplug_memory_support(void);
>> +
>>   #ifndef crash_get_elfcorehdr_size
>>   static inline unsigned int crash_get_elfcorehdr_size(void) { return 0; }
>>   #endif
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index b9190265fe52..b621f03c1104 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -892,12 +892,14 @@ DEFINE_MUTEX(__crash_hotplug_lock);
>>   #define crash_hotplug_lock() mutex_lock(&__crash_hotplug_lock)
>>   #define crash_hotplug_unlock() mutex_unlock(&__crash_hotplug_lock)
>>   
>> +#define CPU_HOTPLUG	0
>> +#define MEM_HOTPLUG	1
>>   /*
>>    * This routine utilized when the crash_hotplug sysfs node is read.
>>    * It reflects the kernel's ability/permission to update the crash
>>    * elfcorehdr directly.
>>    */
>> -int crash_check_update_elfcorehdr(void)
>> +static int crash_hotplug_support(int hotplug)
> Is it possible that we rename this function to different name? e.g
> crash_hotplug_check_support() or other name. We may easily mixing it up
> with the one of ARCH version. personal opinion

Sure I will rename the common function to crash_hotplug_check_support()
and arch-specific function to arch_crash_hotplug_check_support.

Thanks,
Sourabh

>
>>   {
>>   	int rc = 0;
>>   
>> @@ -909,18 +911,27 @@ int crash_check_update_elfcorehdr(void)
>>   		return 0;
>>   	}
>>   	if (kexec_crash_image) {
>> -		if (kexec_crash_image->file_mode)
>> -			rc = 1;
>> -		else
>> -			rc = kexec_crash_image->update_elfcorehdr;
>> +		if (hotplug == CPU_HOTPLUG)
>> +			rc = arch_crash_hotplug_cpu_support(kexec_crash_image);
>> +		else if (hotplug == MEM_HOTPLUG)
>> +			rc = arch_crash_hotplug_memory_support(kexec_crash_image);
>>   	}
>>   	/* Release lock now that update complete */
>>   	kexec_unlock();
>>   	crash_hotplug_unlock();
>> -
>>   	return rc;
>>   }
>>   
>> +int crash_hotplug_cpu_support(void)
>> +{
>> +	return crash_hotplug_support(CPU_HOTPLUG);
>> +}
>> +
>> +int crash_hotplug_memory_support(void)
>> +{
>> +	return crash_hotplug_support(MEM_HOTPLUG);
>> +}
>> +
>>   /*
>>    * To accurately reflect hot un/plug changes of cpu and memory resources
>>    * (including onling and offlining of those resources), the elfcorehdr
>> -- 
>> 2.41.0
>>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec