RE: [PATCH 05/15] megaraid_sas: Do not re-fire shutdown DCMD after OCR

[Shivasharan Srikanteshwara <shivasharan.srikanteshwara@broadcom.com>]

> -----Original Message-----
> From: Shivasharan Srikanteshwara
> [mailto:shivasharan.srikanteshwara@broadcom.com]
> Sent: Tuesday, July 04, 2017 12:39 PM
> To: 'Hannes Reinecke'; 'linux-scsi@vger.kernel.org'
> Cc: 'martin.petersen@oracle.com'; 'thenzl@redhat.com';
> 'jejb@linux.vnet.ibm.com'; Kashyap Desai; Sumit Saxena; 'hare@suse.com';
> 'hch@lst.de'
> Subject: RE: [PATCH 05/15] megaraid_sas: Do not re-fire shutdown DCMD
> after
> OCR
>
> > -----Original Message-----
> > From: Hannes Reinecke [mailto:hare@suse.de]
> > Sent: Friday, June 30, 2017 7:00 PM
> > To: Shivasharan S; linux-scsi@vger.kernel.org
> > Cc: martin.petersen@oracle.com; thenzl@redhat.com;
> > jejb@linux.vnet.ibm.com; kashyap.desai@broadcom.com;
> > sumit.saxena@broadcom.com; hare@suse.com; hch@lst.de
> > 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 <kashyap.desai@broadcom.com>
> > > Signed-off-by: Shivasharan S
> > > <shivasharan.srikanteshwara@broadcom.com>
> > > ---
> > >  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?

Thanks,
Shivasharan

> > Cheers,
> >
> > Hannes
> > --
> > Dr. Hannes Reinecke		   Teamlead Storage & Networking
> > hare@suse.de			               +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)