[Skiboot] [PATCH v2 5/6] FSP/ELOG: Fix possible event notifier hangs
Vasant Hegde
hegdevasant at linux.vnet.ibm.com
Wed Jul 13 19:27:16 AEST 2016
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 ?
-Vasant
More information about the Skiboot
mailing list