>-----Original Message----- >From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] >Sent: Tuesday, September 09, 2014 7:39 PM >To: Sumit.Saxena@xxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx >Cc: martin.petersen@xxxxxxxxxx; hch@xxxxxxxxxxxxx; >jbottomley@xxxxxxxxxxxxx; kashyap.desai@xxxxxxxxxxxxx; >aradford@xxxxxxxxx >Subject: Re: [PATCH 03/11] megaraid_sas : Update threshold based reply post >host index register > >On 09/06/2014 03:25 PM, Sumit.Saxena@xxxxxxxxxxxxx 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 <sumit.saxena@xxxxxxxxxxxxx> >> Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxxxxxxxx> >> --- >> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
