Re: [PATCH] kexec/crash: no crash update when kexec in progress

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

 



Hello Baoquan,

On 05/09/24 08:53, Baoquan He wrote:
On 09/04/24 at 02:55pm, Sourabh Jain wrote:
Hello Baoquan,

On 30/08/24 16:47, Baoquan He wrote:
On 08/20/24 at 12:10pm, Sourabh Jain wrote:
Hello Baoquan,

......snip...
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.
There's a race gap between the kexec_in_progress checking and the
setting it to true which Michael has mentioned.
The window where kernel is holding kexec_lock to do kexec boot
but kexec_in_progress is yet not set to True.

If kernel needs to handle crash hotplug event, the function
crash_handle_hotplug_event()  will not get the kexec_lock and
error out by printing error message about not able to update
kdump image.
But you wanted to avoid the erroring out if it's being in
kernel_kexec().  Now you are seeing at least one the noising
message, aren't you?

Yes, but it is very rare to encounter.

My comments on your updated code are inline below.


I think it should be fine. Given that lock is already taken for
kexec kernel boot.

Am I missing something major?

That's why I think
maybe checking kexec_in_progress after failing to retriving
__kexec_lock is a little better, not very sure.
Try for kexec lock before kexec_in_progress check will not solve
the original problem this patch trying to solve.

You proposed the below changes earlier:

-	if (!kexec_trylock()) {
+	if (!kexec_trylock() && kexec_in_progress) {
  		pr_info("kexec_trylock() failed, elfcorehdr may be inaccurate\n");
  		crash_hotplug_unlock();
Ah, I meant as below, but wrote it mistakenly.

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 63cf89393c6e..e7c7aa761f46 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;



Once the kexec_in_progress is set to True there is no way one can get
kexec_lock. So kexec_trylock() before kexec_in_progress is not helpful
for the problem I am trying to solve.
With your patch, you could still get the error message if the race gap
exist. With above change, you won't get it. Please correct me if I am
wrong.

The above code will print an error message during the race gap. Here's why:

Let’s say the kexec lock is acquired in the kernel_kexec() function,
but kexec_in_progress is not yet set to True. In this scenario, the code will print
an error message.

There is another issue I see with the above code:

Consider that the system is on the kexec kernel boot path, and kexec_in_progress is set to True. If crash_hotplug_unlock() is called, the kernel will not only update the kdump image without acquiring the kexec lock, but it will also release the
kexec lock in the out label. I believe this is incorrect.

Please share your thoughts.

Thanks,
Sourabh Jain



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




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux