Re: [PATCH v2 1/7] s390/kdump: implement is_kdump_kernel()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 15.10.24 10:30, Heiko Carstens wrote:
On Mon, Oct 14, 2024 at 09:26:03PM +0200, David Hildenbrand wrote:
On 14.10.24 20:20, Heiko Carstens wrote:
Looks like this could work. But the comment in smp.c above
dump_available() needs to be updated.

A right, I remember that there was some outdated documentation.


Are you willing to do that, or should I provide an addon patch?


I can squash the following:

diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
index 4df56fdb2488..a4f538876462 100644
--- a/arch/s390/kernel/smp.c
+++ b/arch/s390/kernel/smp.c
@@ -587,16 +587,16 @@ int smp_store_status(int cpu)
   *    with sigp stop-and-store-status. The firmware or the boot-loader
   *    stored the registers of the boot CPU in the absolute lowcore in the
   *    memory of the old system.
- * 3) kdump and the old kernel did not store the CPU state,
- *    or stand-alone kdump for DASD
- *    condition: OLDMEM_BASE != NULL && !is_kdump_kernel()
+ * 3) kdump or stand-alone kdump for DASD
+ *    condition: OLDMEM_BASE != NULL && !is_ipl_type_dump() == false
   *    The state for all CPUs except the boot CPU needs to be collected
   *    with sigp stop-and-store-status. The kexec code or the boot-loader
   *    stored the registers of the boot CPU in the memory of the old system.
- * 4) kdump and the old kernel stored the CPU state
- *    condition: OLDMEM_BASE != NULL && is_kdump_kernel()
- *    This case does not exist for s390 anymore, setup_arch explicitly
- *    deactivates the elfcorehdr= kernel parameter
+ *
+ * Note that the old Kdump mode where the old kernel stored the CPU state

To be consistent with the rest of the comment, please write kdump in
all lower case characters, please.

It obviously was too late in the evening for me :) Thanks!


+ * does no longer exist: setup_arch explicitly deactivates the elfcorehdr=
+ * kernel parameter. The is_kudmp_kernel() implementation on s390 is independent

Typo: kudmp.

Does that sound reasonable? I'm not so sure about the "2) stand-alone kdump for
SCSI/NVMe (zfcp/nvme dump with swapped memory)": is that really "kdump" ?

Yes, it is some sort of kdump, even though a bit odd.

My concern is that we'll now have

bool is_kdump_kernel(void)
{
       return oldmem_data.start && !is_ipl_type_dump();
}

Which matches 3), but if 2) is also called "kdump", then should it actually be

bool is_kdump_kernel(void)
{
       return oldmem_data.start;
}

?

When I wrote that code I was rather convinced that the variant in this patch is the right thing to do.

--
Cheers,

David / dhildenb





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux