> -----Original Message----- > From: linux-scsi-owner@xxxxxxxxxxxxxxx [mailto:linux-scsi- > owner@xxxxxxxxxxxxxxx] On Behalf Of Hannes Reinecke > Sent: Wednesday, June 28, 2017 9:00 PM > To: Sumit Saxena; Christoph Hellwig > Cc: Martin K. Petersen; James Bottomley; linux-scsi@xxxxxxxxxxxxxxx; > Hannes > Reinecke > Subject: Re: [PATCH 13/47] megaraid: pass in NULL scb for host reset > > On 06/28/2017 03:41 PM, Sumit Saxena wrote: > >> -----Original Message----- > >> From: linux-scsi-owner@xxxxxxxxxxxxxxx [mailto:linux-scsi- > >> owner@xxxxxxxxxxxxxxx] On Behalf Of Hannes Reinecke > >> Sent: Wednesday, June 28, 2017 2:03 PM > >> To: Christoph Hellwig > >> Cc: Martin K. Petersen; James Bottomley; linux-scsi@xxxxxxxxxxxxxxx; > > Hannes > >> Reinecke; Hannes Reinecke > >> Subject: [PATCH 13/47] megaraid: pass in NULL scb for host reset > >> > >> When calling a host reset we shouldn't rely on the command triggering > >> the reset, so allow megaraid_abort_and_reset() to be called with a NULL > scb. > >> And drop the pointless 'bus_reset' and 'target_reset' handlers, which > > just call > >> the same function as host_reset. > > > > If this patch address any functional issue, then we should consider > > this. > > If it's code optimization, can we ignore this as this is being very > > old driver and no more maintained by Broadcom/LSI ? > > > Sadly, ignoring is not an option. > I'm planning to update the calling convention for SCSI EH, to resolve the > long- > standing problem with sg_reset ioctls. > sg_reset ioctl will allocate an out-of-band SCSI command, which does no > longer > work with the new command allocation scheme in multiqueue. > So it's not possible to just 'ignore' it, as then SCSI EH will cease to > function with > that driver. Hannes - We are in process of sending megaraid and 3ware driver removal as LSI/Broadcom stopped supporting those products. I agree we should review this closely, but lack of test coverage and end of life cycle of product is requesting us to know the rational. For now, let's consider NACK for this patch. We will be removing old megaraid (mbox) driver and 3Ware drivers soon. > > Sorry. > > >> > >> Signed-off-by: Hannes Reinecke <hare@xxxxxxxx> > >> --- > >> drivers/scsi/megaraid.c | 42 > >> ++++++++++++++++-------------------------- > >> 1 file changed, 16 insertions(+), 26 deletions(-) > >> > >> diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c index > >> 3c63c29..7e504d3 100644 > >> --- a/drivers/scsi/megaraid.c > >> +++ b/drivers/scsi/megaraid.c > >> @@ -1909,7 +1909,7 @@ static DEF_SCSI_QCMD(megaraid_queue) > >> > >> spin_lock_irq(&adapter->lock); > >> > >> - rval = megaraid_abort_and_reset(adapter, cmd, SCB_RESET); > >> + rval = megaraid_abort_and_reset(adapter, NULL, SCB_RESET); > > > > If cmd=NULL is passed, it will crash inside function > > megaraid_abort_and_reset() while dereferencing "cmd" pointer. > > Below is the code of function megaraid_abort_and_reset() where it > > will > > crash- > > > > static int > > megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd *cmd, int aor) > > { > > struct list_head *pos, *next; > > scb_t *scb; > > > > dev_warn(&adapter->dev->dev, "%s cmd=%x <c=%d t=%d l=%d>\n", > > (aor == SCB_ABORT)? "ABORTING":"RESET", > > cmd->cmnd[0], > > cmd->device->channel,>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>it should > > cmd->device->crash > > here > > cmd->device->id, (u32)cmd->device->lun); > > > > Please correct if I am missing something here.Ah, correct. Will be > > fixing it up. > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke Teamlead Storage & Networking > hare@xxxxxxx +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)
