ipmi: Fix a problem that messages are not issued in run_to_completion mode
PATCH 1/1 ipmi: Fix a problem that messages are not issued in run_to_completion mode
start_next_msg() issues a message placed in smi_info->waiting_msg if it is non-NULL. However, sender() sets a message to smi_info->curr_msg and NULL to smi_info->waiting_msg in the context of run_to_completion mode. As the result, it leads an infinite loop by waiting the completion of unissued message when leaving dying message after kernel panic.
sender() should set the message to smi_info->waiting_msg not curr_msg.
--- drivers/char/ipmi/ipmi_si_intf.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/char/ipmi/ipmi_si_intf.c +1 -2
@@ -942,8 +942,7 @@ static void sender(void *send_info,* If we are running to completion, start it and run * transactions until everything is clear. */ - smi_info->curr_msg = msg; - smi_info->waiting_msg =NULL ; + smi_info->waiting_msg = msg;/* * Run to completion means we are single-threaded, no
Signature · 1 line
1.8.3.1
Ah, yes, you are correct. Queued for 4.1. Thanks.
-corey
Quoted 32 lines · Hidehiro Kawai
start_next_msg()issues a message placed insmi_info->waiting_msgif it is non-NULL. However,sender()sets a message tosmi_info->curr_msgand NULL tosmi_info->waiting_msgin the context ofrun_to_completionmode. As the result, it leads an infinite loop by waiting the completion of unissued message when leaving dying message after kernel panic.
sender()should set the message tosmi_info->waiting_msgnotcurr_msg.Signed-off-by Hidehiro Kawai <[email protected]>Cc Corey Minyard <[email protected]>--- drivers/char/ipmi/ipmi_si_intf.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/char/ipmi/ipmi_si_intf.c +1
-2@@ -942,8 +942,7 @@ static void sender(void *send_info,* If we are running to completion, start it and run * transactions until everything is clear. */ - smi_info->curr_msg = msg; - smi_info->waiting_msg =NULL ; + smi_info->waiting_msg = msg;/* * Run to completion means we are single-threaded, no
Hello,
Quoted 1 line · Corey Minyard
Ah, yes, you are correct. Queued for 4.1. Thanks.
Thank you for the review.
By the way, I'm planning some enhancements of IPMI driver in panic context. Currently, we can call panic notifiers before crash_kexec() by specifying crash_kexec_post_notifiers as a boot parameter. By utilizing this feature, we can write SEL records before entering kdump process; we can save some information even if kdump fails. Here, notifier calls shouldn't prevent the kdump process. So, the reliability of panic notifier calls is very important.
I noticed that there are possible infinite loops in the panic notifier call of IPMI driver (we assume BMC is unreliable). To evict possible infinite loops, I'm considering introducing some retry timeout or retry count limit to the run_to_completion procedure.
Do you have any opinions?
Signature · 2 lines
Hidehiro Kawai Hitachi, Ltd. Research & Development Group
Quoted 6 lines · Hidehiro Kawai
Hello,
Quoted 1 line · Corey Minyard
Ah, yes, you are correct. Queued for 4.1. Thanks.
Thank you for the review.
Quoted 8 lines
By the way, I'm planning some enhancements of IPMI driver in panic context. Currently, we can call panic notifiers before
crash_kexec()by specifyingcrash_kexec_post_notifiersas a boot parameter. By utilizing this feature, we can write SEL records before entering kdump process; we can save some information even if kdump fails. Here, notifier calls shouldn't prevent the kdump process. So, the reliability of panic notifier calls is very important.
I thought these were called before crash_kexec(). But if not that would be a good enhancement.
Quoted 7 lines
I noticed that there are possible infinite loops in the panic notifier call of IPMI driver (we assume BMC is unreliable). To evict possible infinite loops, I'm considering introducing some retry timeout or retry count limit to the
run_to_completionprocedure.Do you have any opinions?
That's probably a good idea. My thought was to keep trying in hopes of getting something out, but you are probably right, a timeout after a few minutes is probably appropriate.
-corey
Quoted 9 lines · Corey Minyard
Quoted 8 lines · Hidehiro Kawai
Hello,
I noticed that there are possible infinite loops in the panic notifier call of IPMI driver (we assume BMC is unreliable). To evict possible infinite loops, I'm considering introducing some retry timeout or retry count limit to the
run_to_completionprocedure.
Quoted 4 lines
Quoted 1 line
Do you have any opinions?
That's probably a good idea. My thought was to keep trying in hopes of getting something out, but you are probably right, a timeout after a few minutes is probably appropriate.
OK, I'll do that as the next task.
Thank you!
Signature · 2 lines
Hidehiro Kawai Hitachi, Ltd. Research & Development Group