[Skiboot] [PATCH v2 5/6] FSP/ELOG: Fix possible event notifier hangs
Stewart Smith
stewart at linux.vnet.ibm.com
Thu Jul 21 16:49:43 AEST 2016
Vasant Hegde <hegdevasant at linux.vnet.ibm.com> writes:
> On 07/13/2016 02:53 PM, Vasant Hegde wrote:
>> On 07/04/2016 03:18 PM, Mukesh Ojha wrote:
>>> Few questions.
>>>
>>> On Saturday 02 July 2016 09:03 PM, Vasant Hegde wrote:
>>>> In some corner cases host may send acknowledgement without
>>>> reading actual data (fsp_opal_elog_info -> fsp_opal_elog_ack).
>>>> Because of this elog_read_from_fsp_head_state may be stuck in
>>>> wrong state (ELOG_STATE_HOST_INFO) and not able to send remaining
>>>> ELOG's to host. Hence reset ELOG state and start sending remaining
>>>> ELOG's.
>>>>
>>>> Also in normal case we will ACK the logs which are already processed
>>>> (elog_read_processed). Hence rearrange the code such that we go
>>>> through elog_read_processed first.
>>>>
>>>> Finally return OPAL_PARAMETER if we are not able to find ELOG ID.
>>>>
>>>> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>>>> ---
>>>> hw/fsp/fsp-elog-read.c | 18 +++++++++++++++---
>>>> hw/fsp/fsp-elog-write.c | 3 +++
>>>> 2 files changed, 18 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/hw/fsp/fsp-elog-read.c b/hw/fsp/fsp-elog-read.c
>>>> index b606236..8c01a1c 100644
>>>> --- a/hw/fsp/fsp-elog-read.c
>>>> +++ b/hw/fsp/fsp-elog-read.c
>>>> @@ -416,21 +416,33 @@ static int64_t fsp_opal_elog_ack(uint64_t ack_id)
>>>> return rc;
>>>> }
>>>> lock(&elog_read_lock);
>>>> - list_for_each_safe(&elog_read_pending, record, next_record, link) {
>>>> + list_for_each_safe(&elog_read_processed, record, next_record, link) {
>>>> if (record->log_id != ack_id)
>>>> continue;
>>>> list_del(&record->link);
>>>> list_add(&elog_read_free, &record->link);
>>>> + unlock(&elog_read_lock);
>>>> + return rc;
>>>> }
>>>> - list_for_each_safe(&elog_read_processed, record, next_record, link) {
>>>> + list_for_each_safe(&elog_read_pending, record, next_record, link) {
>>>
>>> Is it really necessary to check the ack_id in the pending list?
>>> As this function will be called only for id which is already in the processed
>>> list.
>>> Same we do it for opal errorlogs.
>>
>> Its required. If host ACKs without reading it.. then it will be in pending list.
>>
>>
>>>> if (record->log_id != ack_id)
>>>> continue;
>>>> + /* It means host has sent ACK without reading actual data.
>>>> + * Because of this elog_read_from_fsp_head_state may be
>>>> + * stuck in wrong state (ELOG_STATE_HOST_INFO) and not able
>>>> + * to send remaning ELOG's to host. Hence reset ELOG state
>>>
>>> s/remaning/remaining
>>
>> Ah yes.. will fix .
>
> @Stewart,
> If you don't have any other comment/suggestions, can you fix this spell
> mistake before merging ?
Sure, spelling fixed when I merged.
--
Stewart Smith
OPAL Architect, IBM.
More information about the Skiboot
mailing list