[Skiboot] [PATCH v2 5/6] FSP/ELOG: Fix possible event notifier hangs

[Vasant Hegde]

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 .

-Vasant