> -----Original Message----- > From: Kashyap Desai [mailto:kashyap.desai@xxxxxxxxxxxxx] > Sent: Wednesday, November 12, 2014 4:39 PM > To: 'Tomas Henzl'; Sumit Saxena; 'linux-scsi@xxxxxxxxxxxxxxx' > Cc: 'martin.petersen@xxxxxxxxxx'; 'hch@xxxxxxxxxxxxx'; > 'jbottomley@xxxxxxxxxxxxx'; 'aradford@xxxxxxxxx' > Subject: RE: [PATCH 3/7] megaraid_sas : Do not process IOCTLs and SCSI > commands during driver removal > > > > > -----Original Message----- > > From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] > > Sent: Tuesday, November 11, 2014 7:13 PM > > To: Sumit.Saxena@xxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx > > Cc: martin.petersen@xxxxxxxxxx; hch@xxxxxxxxxxxxx; > > jbottomley@xxxxxxxxxxxxx; kashyap.desai@xxxxxxxxxxxxx; > > aradford@xxxxxxxxx > > Subject: Re: [PATCH 3/7] megaraid_sas : Do not process IOCTLs and SCSI > > commands during driver removal > > > > On 11/10/2014 01:21 PM, Sumit.Saxena@xxxxxxxxxxxxx wrote: > > > Do not process any SCSI and IOCTL command further(return them with > > > appropriate return values to callers), while driver removal is in > > > progress/PCI > > shutdown is invoked. > > > > > > Signed-off-by: Sumit Saxena <sumit.saxena@xxxxxxxxxxxxx> > > > Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxxxxxxxx> > > > --- > > > drivers/scsi/megaraid/megaraid_sas_base.c | 20 ++++++++++++++++-- > -- > > > 1 files changed, 16 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c > > > b/drivers/scsi/megaraid/megaraid_sas_base.c > > > index f6a69a3..7754eeb 100644 > > > --- a/drivers/scsi/megaraid/megaraid_sas_base.c > > > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c > > > @@ -1572,6 +1572,12 @@ megasas_queue_command(struct Scsi_Host > > *shost, struct scsi_cmnd *scmd) > > > instance = (struct megasas_instance *) > > > scmd->device->host->hostdata; > > > > > > + if (instance->unload == 1) { > > > + scmd->result = DID_NO_CONNECT << 16; > > > + scmd->scsi_done(scmd); > > > + return 0; > > > + } > > > + > > > if (instance->issuepend_done == 0) > > > return SCSI_MLQUEUE_HOST_BUSY; > > > > > > @@ -4957,10 +4963,6 @@ static int megasas_io_attach(struct > > megasas_instance *instance) > > > return -ENODEV; > > > } > > > > > > - /* > > > - * Trigger SCSI to scan our drives > > > - */ > > > - scsi_scan_host(host); > > > return 0; > > > } > > > > > > @@ -5288,6 +5290,10 @@ retry_irq_register: > > > goto fail_io_attach; > > > > > > instance->unload = 0; > > > + /* > > > + * Trigger SCSI to scan our drives > > > + */ > > > + scsi_scan_host(host); > > > > > > /* > > > * Initiate AEN (Asynchronous Event Notification) @@ -6051,6 > > > +6057,12 @@ megasas_mgmt_fw_ioctl(struct megasas_instance > > *instance, > > > megasas_issue_blocked_cmd(instance, cmd, 0); > > > cmd->sync_cmd = 0; > > > > I've expected that you'll not send the command to the card, you first > > issue blocked command and after that test unload state. > > Shouldn't it be reversed? > Actually, there was a two part of this Patch for IOCTL path. One will not allow > to send IOCTL while driver removal. > Another will not use already return IOCTL from FW to avoid illegal memory > access on MFI frame. > Driver wakeup all blocked command in unload path and MFI/MPT frames are > freed as well, so this check is good to have. > > As you suggested, we missed to have another check in IOCTL path, before > driver takes semaphore. It was planned, but somehow we missed while > submitting this patch set. > We will send incremental patch for that part. Correction - First of all sorry for confusion. megasas_mgmt_ioctl_fw and megasas_mgmt_ioctl_aen already has a check for instance->unload. We don't need any changes for this patch. Tomas - We are planning to provide resend patch series which will have changes for below two patch. [PATCH 1/7] megaraid_sas : Driver version upgrade and [PATCH 4/7] megaraid_sas : Online Firmware upgrade support for Extended VD feature ` Kashyap > > > > > > > > > + if (instance->unload == 1) { > > > + dev_info(&instance->pdev->dev, "we are doing unload so no > > need" > > > + "to submit data to application. This may be force exit" > > > + "from driver\n"); > > > > I'm not a native speaker, so I'll not comment on the wording, but you > > should add a space at the line breaks. > > Agree. will address this part. > > > + dev_info(&instance->pdev->dev, "we are doing unload so no > > need " > > + "to submit data to application. This may be force exit > > " > > + "from driver\n"); > > > > > + goto out; > > > + } > > > /* > > > * copy out the kernel buffers to user buffers > > > */ > > > > Aren't there other ioctl paths too - for example > > megasas_mgmt_ioctl_aen - isn't a block needed there too? > This path will be cleanup from megasas_shutdown_controller() call. AEN > and MAP SYNC command will be aborted explicitly. > > > -- 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
