Re: [PATCH 1/7] scsi: Use Scsi_Host as argument for eh_host_reset_handler

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Higher-severity actions are taken only when lower-severity actions
  cannot recover some of failed scmds.  Also, note that failure of the
diff --git a/Documentation/scsi/scsi_mid_low_api.rst b/Documentation/scsi/scsi_mid_low_api.rst
index 022198c51350..85b1384ba4fd 100644
--- a/Documentation/scsi/scsi_mid_low_api.rst
+++ b/Documentation/scsi/scsi_mid_low_api.rst
@@ -777,7 +777,7 @@ Details::
/**
      *      eh_host_reset_handler - reset host (host bus adapter)
-    *      @scp: SCSI host that contains this device should be reset
+    *      @shp: SCSI host that contains this device should be reset

Which device are we referring to here? The current comment does not even make sense to me as it is being passed a scsi_cmnd

      *
      *      Returns SUCCESS if command aborted else FAILED
      *
@@ -794,7 +794,7 @@ Details::
      *
      *      Optionally defined in: LLD
      **/
-	int eh_host_reset_handler(struct scsi_cmnd * scp)
+	int eh_host_reset_handler(struct Scsi_Host * shp)

...

-static int zfcp_scsi_eh_host_reset_handler(struct scsi_cmnd *scpnt)
+static int zfcp_scsi_eh_host_reset_handler(struct Scsi_Host *host)
  {
-	struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scpnt->device);
-	struct zfcp_adapter *adapter = zfcp_sdev->port->adapter;
+	struct zfcp_adapter *adapter = (struct zfcp_adapter *)host->hostdata[0]; >   	int ret = SUCCESS, fc_ret;
if (!(adapter->connection_features & FSF_FEATURE_NPIV_MODE)) {
diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index f925f8664c2c..f0efdfcdac44 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -1717,18 +1717,19 @@ static int twa_scsi_biosparam(struct scsi_device *sdev, struct block_device *bde
  } /* End twa_scsi_biosparam() */
/* This is the new scsi eh reset function */
-static int twa_scsi_eh_reset(struct scsi_cmnd *SCpnt)
+static int twa_scsi_eh_reset(struct Scsi_Host *shost)
  {
  	TW_Device_Extension *tw_dev = NULL;
  	int retval = FAILED;
- tw_dev = (TW_Device_Extension *)SCpnt->device->host->hostdata;
+	tw_dev = (TW_Device_Extension *)shost->hostdata;
tw_dev->num_resets++; - sdev_printk(KERN_WARNING, SCpnt->device,
-		"WARNING: (0x%02X:0x%04X): Command (0x%x) timed out, resetting card.\n",
-		TW_DRIVER, 0x2c, SCpnt->cmnd[0]);
+	shost_printk(KERN_WARNING, shost,
+		     "WARNING: (0x%02X:0x%04X): "
+		     "Command timed out, resetting card.\n",
+		     TW_DRIVER, 0x2c);
/* Make sure we are not issuing an ioctl or resetting from ioctl */
  	mutex_lock(&tw_dev->ioctl_lock);
diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
index 55989eaa2d9f..80885200ba52 100644
--- a/drivers/scsi/3w-sas.c
+++ b/drivers/scsi/3w-sas.c
@@ -1423,18 +1423,19 @@ static int twl_scsi_biosparam(struct scsi_device *sdev, struct block_device *bde
  } /* End twl_scsi_biosparam() */
/* This is the new scsi eh reset function */
-static int twl_scsi_eh_reset(struct scsi_cmnd *SCpnt)
+static int twl_scsi_eh_reset(struct Scsi_Host *shost)
  {
  	TW_Device_Extension *tw_dev = NULL;
  	int retval = FAILED;
- tw_dev = (TW_Device_Extension *)SCpnt->device->host->hostdata;
+	tw_dev = (TW_Device_Extension *)shost->hostdata;
tw_dev->num_resets++; - sdev_printk(KERN_WARNING, SCpnt->device,
-		"WARNING: (0x%02X:0x%04X): Command (0x%x) timed out, resetting card.\n",
-		TW_DRIVER, 0x2c, SCpnt->cmnd[0]);
+	shost_printk(KERN_WARNING, shost,
+		     "WARNING: (0x%02X:0x%04X): "
+		     "Command timed out, resetting card.\n",

nit: I think that these lines can be united to make grepping easier. This comment applies elsewhere.

...

diff --git a/drivers/scsi/aha1542.c b/drivers/scsi/aha1542.c
index 9503996c6325..2a88e1c6ffda 100644
--- a/drivers/scsi/aha1542.c
+++ b/drivers/scsi/aha1542.c
@@ -914,15 +914,14 @@ static int aha1542_dev_reset(struct scsi_cmnd *cmd)
  	aha1542_outb(sh->io_port, CMD_START_SCSI);
  	spin_unlock_irqrestore(sh->host_lock, flags);
- scmd_printk(KERN_WARNING, cmd,
+	sdev_printk(KERN_WARNING, sdev,

I'm not sure if this is a related change

  		"Trying device reset for target\n");
return SUCCESS;
  }

...

  static int esas2r_dev_targ_reset(struct scsi_cmnd *cmd, bool target_reset)
diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
index 97816a0e6240..355fec046220 100644
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -2640,9 +2640,9 @@ static int esp_eh_bus_reset_handler(struct scsi_cmnd *cmd)
  }
/* All bets are off, reset the entire device. */
-static int esp_eh_host_reset_handler(struct scsi_cmnd *cmd)
+static int esp_eh_host_reset_handler(struct Scsi_Host *shost)
  {
-	struct esp *esp = shost_priv(cmd->device->host);
+	struct esp *esp = shost_priv(shost);

Why are we not just using shost_priv() everywhere (instead of shost->hostdata[])? It would save all the casting to the driver host structure also.

  	unsigned long flags;
spin_lock_irqsave(esp->host->host_lock, flags);
diff --git a/drivers/scsi/fdomain.c b/drivers/scsi/fdomain.c
index 504c4e0c5d17..347fb668bf29 100644
--- a/drivers/scsi/fdomain.c
+++ b/drivers/scsi/fdomain.c
@@ -456,9 +456,8 @@ static int fdomain_abort(struct scsi_cmnd *cmd)
  	return SUCCESS;
  }
-static int fdomain_host_reset(struct scsi_cmnd *cmd)
+static int fdomain_host_reset(struct Scsi_Host *sh)
  {
-	struct Scsi_Host *sh = cmd->device->host;
  	struct fdomain *fd = shost_priv(sh);
  	unsigned long flags;
diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
index 8ffcafb4687f..2e2b49ba8cdf 100644
--- a/drivers/scsi/fnic/fnic.h
+++ b/drivers/scsi/fnic/fnic.h
@@ -347,7 +347,7 @@ void fnic_update_mac_locked(struct fnic *, u8 *new);
  int fnic_queuecommand(struct Scsi_Host *, struct scsi_cmnd *);
  int fnic_abort_cmd(struct scsi_cmnd *);
  int fnic_device_reset(struct scsi_cmnd *);
-int fnic_host_reset(struct scsi_cmnd *);
+int fnic_host_reset(struct Scsi_Host *);
  int fnic_reset(struct Scsi_Host *);
  void fnic_scsi_cleanup(struct fc_lport *);
  void fnic_scsi_abort_io(struct fc_lport *);
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 25af91081baf..324058bd594d 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -2448,11 +2448,10 @@ int fnic_reset(struct Scsi_Host *shost)
   * host is offlined by SCSI.
   *
   */
-int fnic_host_reset(struct scsi_cmnd *sc)
+int fnic_host_reset(struct Scsi_Host *shost)
  {
  	int ret;
  	unsigned long wait_host_tmo;
-	struct Scsi_Host *shost = sc->device->host;
  	struct fc_lport *lp = shost_priv(shost);
  	struct fnic *fnic = lport_priv(lp);
  	unsigned long flags;
diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
index f5334ccbf2ca..cc9ed9e6c666 100644
--- a/drivers/scsi/hptiop.c
+++ b/drivers/scsi/hptiop.c
@@ -1088,12 +1088,12 @@ static int hptiop_reset_hba(struct hptiop_hba *hba)
  	return 0;
  }


...
  	scb_t		*scb;
@@ -2525,7 +2525,7 @@ megaraid_reset_handler(struct scsi_cmnd *scp)
  	int		i;
  	uioc_t		*kioc;
- adapter = SCP2ADAPTER(scp);
+	adapter		= (adapter_t *)SCSIHOST2ADAP(shost);

SCSIHOST2ADAP is a re-definition of shost_priv() really (so it can be removed)


..
*/
/* Complete pending commands */
  	handle_reset(ms);
-	
+

we seem to have double blank line now, I'm not sure

  	spin_unlock_irqrestore(ms->host->host_lock, flags);
  	return SUCCESS;
  }
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 040031eb0c12..d52412870b54 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -4010,15 +4010,15 @@ static inline void mpi3mr_setup_divert_ws(struct mpi3mr_ioc *mrioc,
...

/* */
diff --git a/drivers/scsi/pcmcia/qlogic_stub.c b/drivers/scsi/pcmcia/qlogic_stub.c
index 310d0b6586a6..59893db166c9 100644
--- a/drivers/scsi/pcmcia/qlogic_stub.c
+++ b/drivers/scsi/pcmcia/qlogic_stub.c
@@ -270,8 +270,8 @@ static int qlogic_resume(struct pcmcia_device *link)
  		outb(0x24, link->resource[0]->start + 0x9);
  		outb(0x04, link->resource[0]->start + 0xd);
  	}
-	/* Ugggglllyyyy!!! */
-	qlogicfas408_host_reset(NULL);

I had a quick check of qlogicfas408_host_reset(), and it does not even seem safe to pass NULL as the arg (which was a Scsi_cmd) as it is dereferenced there. Has this ever worked?

+
+	qlogicfas408_host_reset(info->host);
return 0;
  }
diff --git a/drivers/scsi/pcmcia/sym53c500_cs.c b/drivers/scsi/pcmcia/sym53c500_cs.c
index 278c78d066c4..1c5415ede9b2 100644
--- a/drivers/scsi/pcmcia/sym53c500_cs.c
+++ b/drivers/scsi/pcmcia/sym53c500_cs.c
@@ -583,14 +583,14 @@ static int SYM53C500_queue_lck(struct scsi_cmnd *SCpnt)
  static DEF_SCSI_QCMD(SYM53C500_queue)


..
  		return FAILED;
  	}
diff --git a/drivers/scsi/qlogicfas408.c b/drivers/scsi/qlogicfas408.c
index 3e065d5fc80c..008e908e3aff 100644
--- a/drivers/scsi/qlogicfas408.c
+++ b/drivers/scsi/qlogicfas408.c
@@ -525,20 +525,18 @@ int qlogicfas408_abort(struct scsi_cmnd *cmd)
/*
   *	Reset SCSI bus
- *	FIXME: This function is invoked with cmd = NULL directly by
- *	the PCMCIA qlogic_stub code. This wants fixing

This seems to answer my question, above. I am still not sure if this is the proper approach, since we don't know it works properly now (instead of just crashing).

   */
-int qlogicfas408_host_reset(struct scsi_cmnd *cmd)
+int qlogicfas408_host_reset(struct Scsi_Host *host)
  {
-	struct qlogicfas408_priv *priv = get_priv_by_cmd(cmd);
+	struct qlogicfas408_priv *priv = get_priv_by_host(host);
  	unsigned long flags;
priv->qabort = 2; - spin_lock_irqsave(cmd->device->host->host_lock, flags);
+	spin_lock_irqsave(host->host_lock, flags);
  	ql_zap(priv);
-	spin_unlock_irqrestore(cmd->device->host->host_lock, flags);
+	spin_unlock_irqrestore(host->host_lock, flags);
return SUCCESS;

...

-static int scsi_debug_host_reset(struct scsi_cmnd *SCpnt)
+static int scsi_debug_host_reset(struct Scsi_Host *shost)
  {
  	struct sdebug_host_info *sdbg_host;
  	struct sdebug_dev_info *devip;
@@ -5379,9 +5379,11 @@ static int scsi_debug_host_reset(struct scsi_cmnd *SCpnt)
++num_host_resets;
  	if (SDEBUG_OPT_ALL_NOISE & sdebug_opts)
-		sdev_printk(KERN_INFO, SCpnt->device, "%s\n", __func__);
+		shost_printk(KERN_INFO, shost, "%s\n", __func__);
  	mutex_lock(&sdebug_host_list_mutex);
  	list_for_each_entry(sdbg_host, &sdebug_host_list, host_list) {
+		if (sdbg_host->shost != shost)
+			continue;

This seems to be a change in behaviour, but the current behaviour is quite odd to begin with. I mean, the current code never uses SCpnt to decide which devip to set SDEBUG_UA_BUS_RESET for, so I don't know why we do it now partially per shost.

  		list_for_each_entry(devip, &sdbg_host->dev_info_list,
  				    dev_list) {
  			set_bit(SDEBUG_UA_BUS_RESET, devip->uas_bm);
@@ -5391,7 +5393,7 @@ static int scsi_debug_host_reset(struct scsi_cmnd *SCpnt)
  	mutex_unlock(&sdebug_host_list_mutex);
  	stop_all_queued();
  	if (SDEBUG_OPT_RESET_NOISE & sdebug_opts)
-		sdev_printk(KERN_INFO, SCpnt->device,
+		shost_printk(KERN_INFO, shost,
  			    "%s: %d device(s) found\n", __func__, k);

Again, I am not sure about this...

  	return SUCCESS;
  }
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c67cdcdc3ba8..f022bb1c3e4a 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -881,7 +881,7 @@ static enum scsi_disposition scsi_try_host_reset(struct scsi_cmnd *scmd)
  	if (!hostt->eh_host_reset_handler)
  		return FAILED;
- rtn = hostt->eh_host_reset_handler(scmd);
+	rtn = hostt->eh_host_reset_handler(host);
if (rtn == SUCCESS) {
  		if (!hostt->skip_settle_delay)
diff --git a/drivers/scsi/snic/snic.h b/drivers/scsi/snic/snic.h
index 0b7411624bcf..27acc50bd1a5 100644
--- a/drivers/scsi/snic/snic.h
+++ b/drivers/scsi/snic/snic.h
@@ -365,8 +365,7 @@ extern const struct attribute_group *snic_host_groups[];
  int snic_queuecommand(struct Scsi_Host *, struct scsi_cmnd *);
  int snic_abort_cmd(struct scsi_cmnd *);

...

+/*
+ * SCSI Error handling calls driver's eh_host_reset if all prior
+ * error handling levels return FAILED.
+ *
+ * Host Reset is the highest level of error recovery. If this fails, then
+ * host is offlined by SCSI.

"is offlined by SCSI" sounds odd. Do you mean "SCSI core"?

+ */
  int
-snic_reset(struct Scsi_Host *shost)
+snic_host_reset(struct Scsi_Host *shost)
  {
  	struct snic *snic = shost_priv(shost);
  	enum snic_state sv_state;
@@ -2297,34 +2304,6 @@ snic_reset(struct Scsi_Host *shost)
  	ret = SUCCESS;
reset_end:
-	return ret;
-} /* end of snic_reset */
-
-/*
- * SCSI Error handling calls driver's eh_host_reset if all prior
- * error handling levels return FAILED.
- *
- * Host Reset is the highest level of error recovery. If this fails, then
- * host is offlined by SCSI.
- */
-int
-snic_host_reset(struct scsi_cmnd *sc)
-{
-	struct Scsi_Host *shost = sc->device->host;
-	u32 start_time  = jiffies;
-	int ret;
-
-	SNIC_SCSI_DBG(shost,
-		      "host reset:sc %p sc_cmd 0x%x req %p tag %d flags 0x%llx\n",
-		      sc, sc->cmnd[0], scsi_cmd_to_rq(sc),
-		      snic_cmd_tag(sc), CMD_FLAGS(sc));
-
-	ret = snic_reset(shost);
-
-	SNIC_TRC(shost->host_no, snic_cmd_tag(sc), (ulong) sc,
-		 jiffies_to_msecs(jiffies - start_time),
-		 0, SNIC_TRC_CMD(sc), SNIC_TRC_CMD_STATE_FLAGS(sc));
-
  	return ret;
  } /* end of snic_host_reset */
diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
index 8ffb75be99bc..5e5ecea265ad 100644
--- a/drivers/scsi/stex.c
+++ b/drivers/scsi/stex.c
@@ -1438,14 +1438,13 @@ static int stex_do_reset(struct st_hba *hba)
  	return -1;
  }
-static int stex_reset(struct scsi_cmnd *cmd)
+static int stex_reset(struct Scsi_Host *shost)
  {
  	struct st_hba *hba;
- hba = (struct st_hba *) &cmd->device->host->hostdata[0];
+	hba = (struct st_hba *) &shost->hostdata[0];

Again, can shost_priv() be used for all of these?

- shost_printk(KERN_INFO, cmd->device->host,
-		     "resetting host\n");
+	shost_printk(KERN_INFO, shost, "resetting host\n");
return stex_do_reset(hba) ? FAILED : SUCCESS;
  }
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index a95936b18f69..7a71d0526f6d 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1624,9 +1624,9 @@ static int storvsc_get_chs(struct scsi_device *sdev, struct block_device * bdev,
  	return 0;





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux