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.
Signed-off-by: Hidehiro Kawai <[email protected]>
Cc: Corey Minyard <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/char/ipmi/ipmi_si_intf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 5e90a18..3d49c70 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -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
--
1.8.3.1
Ah, yes, you are correct. Queued for 4.1. Thanks.
-corey
On 04/22/2015 09:16 PM, Hidehiro Kawai wrote:
> 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.
>
> Signed-off-by: Hidehiro Kawai <[email protected]>
> Cc: Corey Minyard <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/char/ipmi/ipmi_si_intf.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 5e90a18..3d49c70 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -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,
(2015/04/24 22:00), Corey Minyard wrote:
> 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?
--
Hidehiro Kawai
Hitachi, Ltd. Research & Development Group
On 04/28/2015 07:41 AM, Hidehiro Kawai wrote:
> Hello,
>
> (2015/04/24 22:00), Corey Minyard wrote:
>> 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 thought these were called before crash_kexec(). But if not that would
be a good enhancement.
> 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?
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
(2015/05/06 21:27), Corey Minyard wrote:
> On 04/28/2015 07:41 AM, Hidehiro Kawai wrote:
>> 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_completion procedure.
>>
>> 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!
--
Hidehiro Kawai
Hitachi, Ltd. Research & Development Group