[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1e4a8e18-cda9-45f5-a842-8ffcd725efc9@linux.ibm.com>
Date: Tue, 20 Aug 2024 12:10:27 +0530
From: Sourabh Jain <sourabhjain@...ux.ibm.com>
To: Baoquan He <bhe@...hat.com>
Cc: Michael Ellerman <mpe@...erman.id.au>,
Hari Bathini <hbathini@...ux.ibm.com>, kexec@...ts.infradead.org,
linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
x86@...nel.org, Sachin P Bappalige <sachinpb@...ux.vnet.ibm.com>
Subject: Re: [PATCH] kexec/crash: no crash update when kexec in progress
Hello Baoquan,
On 19/08/24 11:45, Baoquan He wrote:
> On 08/19/24 at 09:45am, Sourabh Jain wrote:
>> Hello Michael and Boaquan
>>
>> On 01/08/24 12:21, Sourabh Jain wrote:
>>> Hello Michael,
>>>
>>> On 01/08/24 08:04, Michael Ellerman wrote:
>>>> Sourabh Jain <sourabhjain@...ux.ibm.com> writes:
>>>>> The following errors are observed when kexec is done with SMT=off on
>>>>> powerpc.
>>>>>
>>>>> [ 358.458385] Removing IBM Power 842 compression device
>>>>> [ 374.795734] kexec_core: Starting new kernel
>>>>> [ 374.795748] kexec: Waking offline cpu 1.
>>>>> [ 374.875695] crash hp: kexec_trylock() failed, elfcorehdr may
>>>>> be inaccurate
>>>>> [ 374.935833] kexec: Waking offline cpu 2.
>>>>> [ 375.015664] crash hp: kexec_trylock() failed, elfcorehdr may
>>>>> be inaccurate
>>>>> snip..
>>>>> [ 375.515823] kexec: Waking offline cpu 6.
>>>>> [ 375.635667] crash hp: kexec_trylock() failed, elfcorehdr may
>>>>> be inaccurate
>>>>> [ 375.695836] kexec: Waking offline cpu 7.
>>>> Are they actually errors though? Do they block the actual kexec from
>>>> happening? Or are they just warnings in dmesg?
>>> The kexec kernel boots fine.
>>>
>>> This warning appears regardless of whether the kdump kernel is loaded.
>>>
>>> However, when the kdump kernel is loaded, we will not be able to update
>>> the kdump image (FDT).
>>> I think this should be fine given that kexec is in progress.
>>>
>>> Please let me know your opinion.
>>>
>>>> Because the fix looks like it could be racy.
>>> It seems like it is racy, but given that kexec takes the lock first and
>>> then
>>> brings the CPU up, which triggers the kdump image, which always fails to
>>> update the kdump image because it could not take the same lock.
>>>
>>> Note: the kexec lock is not released unless kexec boot fails.
>> Any comments or suggestions on this fix?
> Is this a little better?
>
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 63cf89393c6e..0355ffb712f4 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -504,7 +504,7 @@ int crash_check_hotplug_support(void)
>
> crash_hotplug_lock();
> /* Obtain lock while reading crash information */
> - if (!kexec_trylock()) {
> + if (!kexec_trylock() && kexec_in_progress) {
> pr_info("kexec_trylock() failed, elfcorehdr may be inaccurate\n");
> crash_hotplug_unlock();
> return 0;
> @@ -539,7 +539,7 @@ static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int cpu,
>
> crash_hotplug_lock();
> /* Obtain lock while changing crash information */
> - if (!kexec_trylock()) {
> + if (!kexec_trylock() && kexec_in_progress) {
> pr_info("kexec_trylock() failed, elfcorehdr may be inaccurate\n");
> crash_hotplug_unlock();
> return;
Ideally, when `kexec_in_progress` is True, there should be no way to
acquire the kexec lock.
Therefore, calling `kexec_trylock()` before checking `kexec_in_progress`
is not helpful.
The kernel will print the error message "kexec_trylock() failed,
elfcorehdr may be inaccurate."
So, with the above changes, the original problem remains unsolved.
However, after closely inspecting the
`kernel/kexec_core.c:kernel_kexec()` function, I discovered
an exceptional case where my patch needs an update. The issue arises
when the system returns
from the `machine_kexec()` function, which indicates that kexec has failed.
In this scenario, the kexec lock is released, but `kexec_in_progress`
remains True.
I am unsure why `kexec_in_progress` is NOT set to False when kexec
fails. Was this by design,
or was it an oversight because returning from the `machine_kexec()`
function is highly unlikely?
Here is my proposal to address the original problem along with the
exceptional case I described
above.
Let's implement two patches:
1. A patch that sets `kexec_in_progress` to False if the system returns
from `machine_kexec()` before
unlocking the kexec lock in the `kernel_kexec()` function.
```
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index c0caa14880c3..b41277183455 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1069,6 +1069,7 @@ int kernel_kexec(void)
#endif
Unlock:
+ kexec_in_progress = false;
kexec_unlock();
return error;
```
2. A patch to return early from the `crash_handle_hotplug_event()`
function if `kexec_in_progress` is
set to True. This is essentially my original patch.
Please share your comments on the new approach.
Thank you for review.
- Sourabh Jain
Powered by blists - more mailing lists