On 07/05/2017 12:33 PM, Shivasharan Srikanteshwara wrote: >> -----Original Message----- >> From: Shivasharan Srikanteshwara >> [mailto:[email protected]] >> Sent: Tuesday, July 04, 2017 12:39 PM >> To: 'Hannes Reinecke'; '[email protected]' >> Cc: '[email protected]'; '[email protected]'; >> '[email protected]'; Kashyap Desai; Sumit Saxena; '[email protected]'; >> '[email protected]' >> Subject: RE: [PATCH 05/15] megaraid_sas: Do not re-fire shutdown DCMD >> after >> OCR >> >>> -----Original Message----- >>> From: Hannes Reinecke [mailto:[email protected]] >>> Sent: Friday, June 30, 2017 7:00 PM >>> To: Shivasharan S; [email protected] >>> Cc: [email protected]; [email protected]; >>> [email protected]; [email protected]; >>> [email protected]; [email protected]; [email protected] >>> Subject: Re: [PATCH 05/15] megaraid_sas: Do not re-fire shutdown DCMD >>> after OCR >>> >>> On 06/30/2017 10:29 AM, Shivasharan S wrote: >>>> Signed-off-by: Kashyap Desai <[email protected]> >>>> Signed-off-by: Shivasharan S >>>> <[email protected]> >>>> --- >>>> drivers/scsi/megaraid/megaraid_sas_fusion.c | 9 +++++++++ >>>> 1 file changed, 9 insertions(+) >>>> >>>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c >>>> b/drivers/scsi/megaraid/megaraid_sas_fusion.c >>>> index 0f13c58..a308e14 100644 >>>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c >>>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c >>>> @@ -3618,6 +3618,15 @@ void megasas_refire_mgmt_cmd(struct >>>> megasas_instance *instance) >>>> >>>> if (!smid) >>>> continue; >>>> + >>>> + /* Do not refire shutdown command */ >>>> + if (le32_to_cpu(cmd_mfi->frame->dcmd.opcode) == >>>> + MR_DCMD_CTRL_SHUTDOWN) { >>>> + cmd_mfi->frame->dcmd.cmd_status = MFI_STAT_OK; >>>> + megasas_complete_cmd(instance, cmd_mfi, DID_OK); >>>> + continue; >>>> + } >>>> + >>>> req_desc = megasas_get_request_descriptor >>>> (instance, smid - 1); >>>> refire_cmd = req_desc && ((cmd_mfi->frame->dcmd.opcode != >>>> >>> Please, no. >>> You already have a selector further down whether to refire the >>> command, pending on the DRV_DCMD_SKIP_REFIRE flag. >>> If you always set this flag for the shutdown command you wouldn't need >>> to touch this at all. >>> And if you particularly dislike that solution convert the 'refire_cmd =' >>> statement into a proper switch and blank it out from there. >>> >> >> Hi Hannes, >> The management commands that get skipped further down in this function are >> internally generated by the driver itself so we don’t need to complete it >> back to >> the application. >> In case of shutdown DCMD, it’s an IOCTL command(issued by application) so >> we >> need to return status back to application. >> Combining handling of both the cases or using DRV_DCMD_SKIP_REFIRE will >> not >> be as straightforward. >> >> Thanks, >> Shivasharan >> > Hi Hannes, > We will implement the switch case approach during our next submission as > this > the current code is well tested internally and want to avoid regressions. > Are you ok if we keep it this way for now? > Yes, sure.
Reviewed-by: Hannes Reinecke <[email protected]> Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking [email protected] +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
