>-----Original Message-----
>From: Tomas Henzl [mailto:[email protected]]
>Sent: Tuesday, September 09, 2014 7:39 PM
>To: [email protected]; [email protected]
>Cc: [email protected]; [email protected];
>[email protected]; [email protected];
>[email protected]
>Subject: Re: [PATCH 03/11] megaraid_sas : Update threshold based reply
post
>host index register
>
>On 09/06/2014 03:25 PM, [email protected] wrote:
>> Current driver updates reply post host index to let firmware know that
>> replies are processed, while returning from ISR function, only if there
is no
>oustanding replies in reply queue.
>>
>> Driver will free the request frame immediately from ISR but reply post
host
>index is not yet updated.
>> It means freed request can be used by submission path and there may be
>> a tight loop in request/reply path. In such condition, firmware may
>> crash when it tries to post reply and there is no free reply post
descriptor.
>>
>> Eventually two things needs to be change to avoid this issue.
>>
>> Increase reply queue depth (double than request queue) to accommodate
>worst case scenario.
>> Update reply post host index to firmware once it reach to some
pre-defined
>threshold value.
>>
>> This change will make sure that firmware will always have some buffer
>> of reply descriptor and will never find empty reply descriptor in
completion
>path.
>>
>> Signed-off-by: Sumit Saxena <[email protected]>
>> Signed-off-by: Kashyap Desai <[email protected]>
>> ---
>> drivers/scsi/megaraid/megaraid_sas_fusion.c | 23
>> ++++++++++++++++++++++-
>drivers/scsi/megaraid/megaraid_sas_fusion.h |
>> 1 +
>> 2 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c
>> b/drivers/scsi/megaraid/megaraid_sas_fusion.c
>> index c69c1ac..f30297d 100644
>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
>> @@ -971,7 +971,7 @@ megasas_init_adapter_fusion(struct
>> megasas_instance *instance)
>>
>> max_cmd = instance->max_fw_cmds;
>>
>> - fusion->reply_q_depth = ((max_cmd + 1 + 15)/16)*16;
>> + fusion->reply_q_depth = (((max_cmd * 2) + 1 + 15)/16)*16;
>
>This computation gives for certain values the same result as before, for
some
>the result is 1.5*higher and for others 2*higher. Is this intended?
>(I'm not asking what all the magic numbers mean..) what about a simple
>fusion->reply_q_depth = 2*(((max_cmd + 1 + 15)/16)*16);
I agree to make this computation simple and more accurate to give always
2*higher.
In most of the cases, we need reply queue depth = request_queue_depth +
additional room as provide in THRESHOLD_REPLY_COUNT.
Thanks for the feedback. We will do this change and will resubmit the
patch.
>
>>
>> fusion->request_alloc_sz =
>> sizeof(union MEGASAS_REQUEST_DESCRIPTOR_UNION)
>*max_cmd; @@ -1876,6
>> +1876,7 @@ complete_cmd_fusion(struct megasas_instance *instance, u32
>MSIxIndex)
>> u32 status, extStatus, device_id;
>> union desc_value d_val;
>> struct LD_LOAD_BALANCE_INFO *lbinfo;
>> + int threshold_reply_count = 0;
>>
>> fusion = instance->ctrl_context;
>>
>> @@ -1963,6 +1964,7 @@ complete_cmd_fusion(struct megasas_instance
>> *instance, u32 MSIxIndex)
>>
>> desc->Words = ULLONG_MAX;
>> num_completed++;
>> + threshold_reply_count++;
>>
>> /* Get the next reply descriptor */
>> if (!fusion->last_reply_idx[MSIxIndex])
>> @@ -1982,6 +1984,25 @@ complete_cmd_fusion(struct megasas_instance
>> *instance, u32 MSIxIndex)
>>
>> if (reply_descript_type ==
>MPI2_RPY_DESCRIPT_FLAGS_UNUSED)
>> break;
>> + /*
>> + * Write to reply post host index register after
completing
>threshold
>> + * number of reply counts and still there are more replies
in
>reply queue
>> + * pending to be completed
>> + */
>> + if (threshold_reply_count >= THRESHOLD_REPLY_COUNT) {
>> + if ((instance->pdev->device ==
>> + PCI_DEVICE_ID_LSI_INVADER) ||
>> + (instance->pdev->device ==
>> + PCI_DEVICE_ID_LSI_FURY))
>> + writel(((MSIxIndex & 0x7) << 24) |
>> + fusion->last_reply_idx[MSIxIndex],
>> + instance-
>>reply_post_host_index_addr[MSIxIndex/8]);
>> + else
>> + writel((MSIxIndex << 24) |
>> + fusion->last_reply_idx[MSIxIndex],
>> + instance-
>>reply_post_host_index_addr[0]);
>> + threshold_reply_count = 0;
>> + }
>> }
>>
>> if (!num_completed)
>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.h
>> b/drivers/scsi/megaraid/megaraid_sas_fusion.h
>> index e76af54..d660c4d 100644
>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.h
>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.h
>> @@ -86,6 +86,7 @@ enum MR_RAID_FLAGS_IO_SUB_TYPE {
>>
>> #define MEGASAS_FP_CMD_LEN 16
>> #define MEGASAS_FUSION_IN_RESET 0
>> +#define THRESHOLD_REPLY_COUNT 50
>>
>> /*
>> * Raid Context structure which describes MegaRAID specific IO
>> Parameters
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
