From: Corey Minyard <minyard@acm.org>
To: "河合英宏 / KAWAI,HIDEHIRO" <hidehiro.kawai.ez@hitachi.com>
Cc: "openipmi-developer@lists.sourceforge.net"
<openipmi-developer@lists.sourceforge.net>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 7/7] ipmi/kcs: Don't run the KCS state machine when it is KCS_IDLE
Date: Sat, 22 Aug 2015 12:45:39 -0500 [thread overview]
Message-ID: <55D8B543.4000805@acm.org> (raw)
In-Reply-To: <04EAB7311EE43145B2D3536183D1A84454938089@GSjpTKYDCembx31.service.hitachi.net>
On 08/17/2015 09:54 PM, 河合英宏 / KAWAI,HIDEHIRO wrote:
>> From: Corey Minyard [mailto:tcminyard@gmail.com] On Behalf Of Corey Minyard
>>
>> This patch will break ATN handling on the interfaces. So we can't do this.
> I understand. So how about doing like this:
>
> /* All states wait for ibf, so just do it here. */
> - if (!check_ibf(kcs, status, time))
> + if (kcs->state != KCS_IDLE && !check_ibf(kcs, status, time))
> return SI_SM_CALL_WITH_DELAY;
>
> I think it is not necessary to wait IBF when the state is IDLE.
> In this way, we can also handle the ATN case.
I think it would be more reliable to go up a level and add a timeout.
One should
be there, anyway. I thought they were all covered, but I may have missed
something.
-corey
>
> Regards,
>
> Hidehiro Kawai
> Hitachi, Ltd. Research & Development Group
>
>> It's going to be extremely hard to recover if the BMC is not working
>> correctly when a panic happens. I'm not sure what can be done, but if
>> you can fix it another way it would be good.
>>
>> -corey
>>
>> On 07/27/2015 12:55 AM, Hidehiro Kawai wrote:
>>> If a BMC is unresponsive for some reason, it ends up completing
>>> the requested message as an error, then kcs_event() is called once
>>> to advance the state machine. However, since the BMC is
>>> unresponsive now, the status of the KCS interface may not be
>>> idle. As the result, the state machine can continue to run and
>>> comsume CPU time indefinitely even if there is no more request
>>> message. Moreover, if this happens in run-to-completion mode
>>> (i.e. context of panic_event()), the kernel hangs up.
>>>
>>> To fix this problem, this patch ignores kcs_event() call if there
>>> is no request message to be processed.
>>>
>>> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
>>> ---
>>> drivers/char/ipmi/ipmi_kcs_sm.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/char/ipmi/ipmi_kcs_sm.c b/drivers/char/ipmi/ipmi_kcs_sm.c
>>> index 8c25f59..0e187fb 100644
>>> --- a/drivers/char/ipmi/ipmi_kcs_sm.c
>>> +++ b/drivers/char/ipmi/ipmi_kcs_sm.c
>>> @@ -353,6 +353,10 @@ static enum si_sm_result kcs_event(struct si_sm_data *kcs, long time)
>>> if (kcs_debug & KCS_DEBUG_STATES)
>>> printk(KERN_DEBUG "KCS: State = %d, %x\n", kcs->state, status);
>>>
>>> + /* We don't want to run the state machine when the state is IDLE */
>>> + if (kcs->state == KCS_IDLE)
>>> + return SI_SM_IDLE;
>>> +
>>> /* All states wait for ibf, so just do it here. */
>>> if (!check_ibf(kcs, status, time))
>>> return SI_SM_CALL_WITH_DELAY;
>>>
>>>
next prev parent reply other threads:[~2015-08-23 23:15 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-27 5:55 [PATCH 0/7] ipmi: various fixes for panic notifier robustness Hidehiro Kawai
2015-07-27 5:55 ` [PATCH 5/7] ipmi: Don't call receive handler in the panic context Hidehiro Kawai
2015-07-27 5:55 ` [PATCH 6/7] ipmi: Handle queued messages more certainly on panic Hidehiro Kawai
2015-08-12 4:13 ` Corey Minyard
2015-08-18 1:59 ` 河合英宏 / KAWAI,HIDEHIRO
2015-08-22 17:39 ` Corey Minyard
2015-07-27 5:55 ` [PATCH 4/7] ipmi: Avoid touching possible corrupted lists in the panic context Hidehiro Kawai
2015-07-27 5:55 ` [PATCH 3/7] ipmi: Don't flush messages in sneder() in run-to-completion mode Hidehiro Kawai
2015-07-27 5:55 ` [PATCH 1/7] ipmi: Remove unneeded set_run_to_completion call Hidehiro Kawai
2015-07-27 5:55 ` [PATCH 2/7] ipmi: Factor out message flushing procedure Hidehiro Kawai
2015-07-27 5:55 ` [PATCH 7/7] ipmi/kcs: Don't run the KCS state machine when it is KCS_IDLE Hidehiro Kawai
2015-08-12 4:15 ` Corey Minyard
2015-08-18 2:54 ` 河合英宏 / KAWAI,HIDEHIRO
2015-08-22 17:45 ` Corey Minyard [this message]
2015-08-24 1:52 ` 河合英宏 / KAWAI,HIDEHIRO
2015-08-24 16:00 ` Corey Minyard
2015-08-25 3:53 ` 河合英宏 / KAWAI,HIDEHIRO
2015-08-26 20:27 ` Corey Minyard
2015-08-27 1:35 ` 河合英宏 / KAWAI,HIDEHIRO
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55D8B543.4000805@acm.org \
--to=minyard@acm.org \
--cc=hidehiro.kawai.ez@hitachi.com \
--cc=linux-kernel@vger.kernel.org \
--cc=openipmi-developer@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).