On 15.10.24 12:08, Heiko Carstens wrote:
On Tue, Oct 15, 2024 at 10:41:21AM +0200, David Hildenbrand wrote:
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.
...
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.
Oh well, we simply of too many dump options. I'll let Alexander and
Mikhail better comment on this :)
Thanks!
Alexander, Mikhail, the discussion starts here, and we need a sane
is_kdump_kernel() for s390:
https://lore.kernel.org/all/20241014144622.876731-1-david@xxxxxxxxxx/
With the following cleanup in mind:
From 9fbbff43f725a8482ce9e85857316ab8484ff3c8 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@xxxxxxxxxx>
Date: Tue, 15 Oct 2024 11:07:43 +0200
Subject: [PATCH] s390: use !dump_available() instead of "!oldmem_data.start &&
!is_ipl_type_dump()"
Let's make the code more readable:
!dump_available()
-> !(oldmem_data.start || is_ipl_type_dump())
-> !oldmem_data.start && !is_ipl_type_dump()
Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
---
arch/s390/kernel/crash_dump.c | 2 +-
arch/s390/kernel/os_info.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/s390/kernel/crash_dump.c b/arch/s390/kernel/crash_dump.c
index cca1827d3d2e..fbc5de66d03b 100644
--- a/arch/s390/kernel/crash_dump.c
+++ b/arch/s390/kernel/crash_dump.c
@@ -609,7 +609,7 @@ int elfcorehdr_alloc(unsigned long long *addr, unsigned long long *size)
u64 hdr_off;
/* If we are not in kdump or zfcp/nvme dump mode return */
- if (!oldmem_data.start && !is_ipl_type_dump())
+ if (!dump_available())
return 0;
/* If we cannot get HSA size for zfcp/nvme dump return error */
if (is_ipl_type_dump() && !sclp.hsa_size)
diff --git a/arch/s390/kernel/os_info.c b/arch/s390/kernel/os_info.c
index b695f980bbde..09578f400ef7 100644
--- a/arch/s390/kernel/os_info.c
+++ b/arch/s390/kernel/os_info.c
@@ -148,7 +148,7 @@ static void os_info_old_init(void)
if (os_info_init)
return;
- if (!oldmem_data.start && !is_ipl_type_dump())
+ if (!dump_available())
goto fail;
if (copy_oldmem_kernel(&addr, __LC_OS_INFO, sizeof(addr)))
goto fail;
--
2.46.1
It looks like we create /proc/vmcore, allocating the elfcorehdr essentially
if dump_available() && (!is_ipl_type_dump() || !sclp.hsa_size).
So in the condition, we would have previously made is_kdump_kernel() happt *after* the
fs init calls.
So I'm wondering if is_kdump_kernel() should simply translate to dump_available(). It
doesn't sound completely right to create /proc/vmcore for "standard zfcp/nvme dump",
but that is what we currently do.
I would have thought we have that special zcore thingy for that.
So I think we should either have
bool is_kdump_kernel(void)
{
return dump_available();
}
Or
bool is_kdump_kernel(void)
{
return dump_available() && (!is_ipl_type_dump() || !sclp.hsa_size);
}
I'd prefer the first version.
--
Cheers,
David / dhildenb
