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. 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 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)
