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

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

 





Am 21.10.24 um 14:46 schrieb Alexander Egorenkov:
Hi David,

David Hildenbrand <david@xxxxxxxxxx> writes:

Makes sense, so it boils down to either

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

Which means is_kdump_kernel() can be "false" even though /proc/vmcore is
available or

bool is_kdump_kernel(void)
{
           return dump_available();
}

Which means is_kdump_kernel() can never be "false" if /proc/vmcore is
available. There is the chance of is_kdump_kernel() being "true" if
"elfcorehdr_alloc()" fails with -ENODEV.

Thanks for having another look!


Do you consider is_kdump_kernel() returning "true" in case of zfcpdump or
nvme/eckd+ldipl dump (also called NGDump) okay ? Because
dump_available() would return "true" in such cases too.
If yes then please explain why, i might have missed a previous
explanation from you.

I consider it okay because this is the current behavior after elfcorehdr_alloc() succeeded and set elfcorehdr_addr.

Not sure if it is the right think to do, though :)

Whatever we do, we should achieve on s390 that the result of is_kdump_kernel() is consistent throughout the booting stages, just like on all other architectures.

Right now it goes from false->true when /proc/vmcore gets initialized (and only if it gets initialized properly).


I'm afraid everyone will make wrong assumptions while reading the name
of is_kdump_kernel() and assuming that it only applies to kdump or
kdump-alike dumps (like stand-alone kdump), and, therefore, introduce
bugs because the name of the function conveys the wrong idea to code
readers. I consider dump_available() as a superset of is_kdump_kernel()
and, therefore, to me they are not equivalent.
> > I have the feeling you consider is_kdump_kernel() equivalent to
"/proc/vmcore" being present and not really saying anything about
whether kdump is active ?

Yes, but primarily because this is the existing handling.

Staring at the powerpc implementation:

/*
 * Return true only when kexec based kernel dump capturing method is used.
 * This ensures all restritions applied for kdump case are not automatically
 * applied for fadump case.
 */
bool is_kdump_kernel(void)
{
	return !is_fadump_active() && elfcorehdr_addr != ELFCORE_ADDR_MAX;
}
EXPORT_SYMBOL_GPL(is_kdump_kernel);


Which was added by

commit b098f1c32365304633077d73e4ae21c72d4241b3
Author: Hari Bathini <hbathini@xxxxxxxxxxxxx>
Date:   Tue Sep 12 13:59:50 2023 +0530

    powerpc/fadump: make is_kdump_kernel() return false when fadump is active

    Currently, is_kdump_kernel() returns true in crash dump capture kernel
    for both kdump and fadump crash dump capturing methods, as both these
    methods set elfcorehdr_addr. Some restrictions enforced for crash dump
    capture kernel, based on is_kdump_kernel(), are specifically meant for
    kdump case and not desirable for fadump - eg. IO queues restriction in
    device drivers. So, define is_kdump_kernel() to return false when f/w
    assisted dump is active.


For my purpose (virtio-mem), it's sufficient to only support "kexec triggered kdump" either way, so I don't care.

So for me it's good enough to have

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

And trying to document the situation in a comment like powerpc does :)

--
Cheers,

David / dhildenb





[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux