Re: [PATCH 7/7] ipmi/kcs: Don't run the KCS state machine when it is KCS_IDLE
From: Corey Minyard
Date: Wed Aug 26 2015 - 16:27:26 EST
On 08/24/2015 10:53 PM, æåèå / KAWAIïHIDEHIRO wrote:
>> From: Corey Minyard [mailto:tcminyard@xxxxxxxxx] On Behalf Of Corey Minyard
>> On 08/23/2015 08:52 PM, æåèå / KAWAIïHIDEHIRO wrote:
>>>> From: Corey Minyard [mailto:tcminyard@xxxxxxxxx] On Behalf Of Corey Minyard
>>>> On 08/17/2015 09:54 PM, æåèå / KAWAIïHIDEHIRO wrote:
>>>>>> From: Corey Minyard [mailto:tcminyard@xxxxxxxxx] 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.
>>> It may be so, but we should address this issue separately (at least
>>> I think above solution reasonably solves the issue).
>>> This issue happens after all queued messages are processed or dropped
>>> by timeout. There is no current message. So what should we set
>>> a timeout against? We can add a timeout into my new flush_messages(),
>>> but that is meaningful only in panic context. That doesn't help
>>> in normal context; we would perform a busy loop of smi_event_handler()
>>> and schedule() in ipmi_thread().
>> I'm a little confused here. Is the problem that the ATN bit is stuck
>> high? If so, it's going to be really hard to work around this without
>> breaking ATN handling.
> Sorry for my insufficient explanation. I assume the case where
> IBF bit is always 1. I don't know what happens when
> BMC hangs up, but I guess IBF stays in 1 because my server's
> BMC behaves as such while rebooting.
Ok, your patch above makes sense, then. IBF is irrelevant when in idle
so ignore it then, and then in your case it will return KCS_IDLE and
operation to complete. I'm ok with the patch you posted above, I think
work correctly and solve the problem.
I would like a detailed comment, though, so people (forgetful people
like me :)
can figure out why it is there. I'd also like to save this one until
4.4 to give it some
time in linux-next for people to find issues.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/