Re: [PATCH 46/47] scsi: Move eh_device_reset_handler() to use scsi_device as argument

2017-07-24 Thread Steffen Maier 

sparse review comments below...

On 06/28/2017 10:33 AM, Hannes Reinecke wrote:

When resetting a device we shouldn't depend on an existing SCSI
device, so use 'struct scsi_device' as argument for
eh_device_reset_handler().

Signed-off-by: Hannes Reinecke 
---
  Documentation/scsi/scsi_eh.txt  |  2 +-
  Documentation/scsi/scsi_mid_low_api.txt |  4 +--



  drivers/s390/scsi/zfcp_scsi.c   |  4 +--



  drivers/scsi/scsi_debug.c   | 18 ++---
  drivers/scsi/scsi_error.c   | 35 +



  include/scsi/scsi_host.h|  2 +-
  60 files changed, 287 insertions(+), 307 deletions(-)

diff --git a/Documentation/scsi/scsi_eh.txt b/Documentation/scsi/scsi_eh.txt
index cb9f4bc22..f8a6566 100644
--- a/Documentation/scsi/scsi_eh.txt
+++ b/Documentation/scsi/scsi_eh.txt
@@ -206,7 +206,7 @@ hostt EH callbacks.  Callbacks may be omitted and omitted 
ones are
  considered to fail always.

  int (* eh_abort_handler)(struct scsi_cmnd *);
-int (* eh_device_reset_handler)(struct scsi_cmnd *);
+int (* eh_device_reset_handler)(struct scsi_device *);
  int (* eh_bus_reset_handler)(struct Scsi_Host *, int);
  int (* eh_host_reset_handler)(struct Scsi_Host *);

diff --git a/Documentation/scsi/scsi_mid_low_api.txt 
b/Documentation/scsi/scsi_mid_low_api.txt
index e2609a63..28dc029 100644
--- a/Documentation/scsi/scsi_mid_low_api.txt
+++ b/Documentation/scsi/scsi_mid_low_api.txt
@@ -874,7 +874,7 @@ Details:

  /**
   *  eh_device_reset_handler - issue SCSI device reset
- *  @scp: identifies SCSI device to be reset
+ *  @sdev: identifies SCSI device to be reset
   *
   *  Returns SUCCESS if command aborted else FAILED
   *
@@ -887,7 +887,7 @@ Details:
   *
   *  Optionally defined in: LLD
   **/
- int eh_device_reset_handler(struct scsi_cmnd * scp)
+ int eh_device_reset_handler(struct scsi_device * sdev)


  /**




diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index 92a3902..23b5a7d 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -304,9 +304,9 @@ static int zfcp_task_mgmt_function(struct scsi_device 
*sdev, u8 tm_flags)
return retval;
  }

-static int zfcp_scsi_eh_device_reset_handler(struct scsi_cmnd *scpnt)
+static int zfcp_scsi_eh_device_reset_handler(struct scsi_device *sdev)
  {
-   return zfcp_task_mgmt_function(scpnt->device, FCP_TMF_LUN_RESET);
+   return zfcp_task_mgmt_function(sdev, FCP_TMF_LUN_RESET);
  }

  /*




diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 37e511f..9029500 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3793,19 +3793,17 @@ static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
return SUCCESS;
  }

-static int scsi_debug_device_reset(struct scsi_cmnd * SCpnt)
+static int scsi_debug_device_reset(struct scsi_device * sdp)
  {
+   struct sdebug_dev_info *devip =
+   (struct sdebug_dev_info *)sdp->hostdata;
+
++num_dev_resets;
-   if (SCpnt && SCpnt->device) {
-   struct scsi_device *sdp = SCpnt->device;
-   struct sdebug_dev_info *devip =
-   (struct sdebug_dev_info *)sdp->hostdata;

-   if (SDEBUG_OPT_ALL_NOISE & sdebug_opts)
-   sdev_printk(KERN_INFO, sdp, "%s\n", __func__);
-   if (devip)
-   set_bit(SDEBUG_UA_POR, devip->uas_bm);
-   }
+   if (SDEBUG_OPT_ALL_NOISE & sdebug_opts)
+   sdev_printk(KERN_INFO, sdp, "%s\n", __func__);
+   if (devip)
+   set_bit(SDEBUG_UA_POR, devip->uas_bm);
return SUCCESS;
  }

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 368a961..4a7fe97f 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -844,7 +844,7 @@ static int scsi_try_bus_device_reset(struct scsi_cmnd *scmd)
if (!hostt->eh_device_reset_handler)
return FAILED;

-   rtn = hostt->eh_device_reset_handler(scmd);
+   rtn = hostt->eh_device_reset_handler(scmd->device);
if (rtn == SUCCESS)
__scsi_report_device_reset(scmd->device, NULL);
return rtn;


up to here it looks good,
however ...


@@ -1106,6 +1106,7 @@ static int scsi_eh_action(struct scsi_cmnd *scmd, int rtn)
   * scsi_eh_finish_cmd - Handle a cmd that eh is finished with.


__scsi_eh_finish_cmd  ?


   * @scmd: Original SCSI cmd that eh has finished.
   * @done_q:   Queue for processed commands.
+ * @result:Final command status to be set
   *
   * Notes:
   *We don't want to use the normal command completion while we are are


Did this hunk slip in accidentally?
I don't see a new additional argument result being introduced with 
either the new __scsi_eh_finish_cmd() nor the old scsi_eh_finish_cmd().
However, the former __scsi_eh_finish_cmd() seems to

[PATCH 46/47] scsi: Move eh_device_reset_handler() to use scsi_device as argument

2017-06-28 Thread Hannes Reinecke 
When resetting a device we shouldn't depend on an existing SCSI
device, so use 'struct scsi_device' as argument for
eh_device_reset_handler().

Signed-off-by: Hannes Reinecke 
---
 Documentation/scsi/scsi_eh.txt  |  2 +-
 Documentation/scsi/scsi_mid_low_api.txt |  4 +--
 drivers/block/cciss_scsi.c  | 14 +-
 drivers/infiniband/ulp/srp/ib_srp.c |  8 +++---
 drivers/message/fusion/mptfc.c  | 12 -
 drivers/message/fusion/mptscsih.c   | 19 ++
 drivers/message/fusion/mptscsih.h   |  2 +-
 drivers/s390/scsi/zfcp_scsi.c   |  4 +--
 drivers/scsi/a100u2w.c  |  7 +++--
 drivers/scsi/aacraid/linit.c|  9 +++
 drivers/scsi/aha152x.c  |  6 ++---
 drivers/scsi/aha1542.c  |  8 +++---
 drivers/scsi/aic7xxx/aic79xx_osm.c  | 27 ---
 drivers/scsi/aic7xxx/aic7xxx_osm.c  |  4 +--
 drivers/scsi/arm/fas216.c   |  5 ++--
 drivers/scsi/be2iscsi/be_main.c |  8 +++---
 drivers/scsi/bfa/bfad_im.c  |  3 +--
 drivers/scsi/bnx2fc/bnx2fc.h|  2 +-
 drivers/scsi/bnx2fc/bnx2fc_io.c |  6 ++---
 drivers/scsi/csiostor/csio_scsi.c   |  7 +++--
 drivers/scsi/cxlflash/main.c| 15 ---
 drivers/scsi/dpt_i2o.c  | 20 +++---
 drivers/scsi/dpti.h |  2 +-
 drivers/scsi/esas2r/esas2r.h|  2 +-
 drivers/scsi/esas2r/esas2r_main.c   |  3 +--
 drivers/scsi/fnic/fnic.h|  2 +-
 drivers/scsi/fnic/fnic_scsi.c   | 26 +-
 drivers/scsi/hpsa.c | 14 +-
 drivers/scsi/ibmvscsi/ibmvfc.c  |  5 ++--
 drivers/scsi/ibmvscsi/ibmvscsi.c| 19 +++---
 drivers/scsi/ipr.c  | 31 +++---
 drivers/scsi/libfc/fc_fcp.c |  8 +++---
 drivers/scsi/libiscsi.c | 15 +--
 drivers/scsi/libsas/sas_scsi_host.c | 12 -
 drivers/scsi/lpfc/lpfc_scsi.c   | 12 -
 drivers/scsi/mpt3sas/mpt3sas_scsih.c| 29 +---
 drivers/scsi/pmcraid.c  |  6 ++---
 drivers/scsi/qedf/qedf_main.c   |  6 ++---
 drivers/scsi/qla1280.c  | 21 +++
 drivers/scsi/qla2xxx/qla_os.c   |  5 ++--
 drivers/scsi/qla4xxx/ql4_os.c   | 28 +---
 drivers/scsi/scsi_debug.c   | 18 ++---
 drivers/scsi/scsi_error.c   | 35 +
 drivers/scsi/smartpqi/smartpqi_init.c   |  6 ++---
 drivers/scsi/snic/snic.h|  2 +-
 drivers/scsi/snic/snic_scsi.c   |  4 +--
 drivers/scsi/ufs/ufshcd.c   | 16 +--
 drivers/scsi/virtio_scsi.c  | 14 +-
 drivers/scsi/vmw_pvscsi.c   | 10 +++
 drivers/scsi/wd719x.c   |  6 ++---
 drivers/scsi/xen-scsifront.c|  4 +--
 drivers/staging/rts5208/rtsx.c  |  4 +--
 drivers/staging/unisys/visorhba/visorhba_main.c | 14 +++---
 drivers/target/loopback/tcm_loop.c  |  8 +++---
 drivers/usb/storage/scsiglue.c  |  4 +--
 drivers/usb/storage/uas.c   |  3 +--
 include/scsi/libfc.h|  2 +-
 include/scsi/libiscsi.h |  2 +-
 include/scsi/libsas.h   |  2 +-
 include/scsi/scsi_host.h|  2 +-
 60 files changed, 287 insertions(+), 307 deletions(-)

diff --git a/Documentation/scsi/scsi_eh.txt b/Documentation/scsi/scsi_eh.txt
index cb9f4bc22..f8a6566 100644
--- a/Documentation/scsi/scsi_eh.txt
+++ b/Documentation/scsi/scsi_eh.txt
@@ -206,7 +206,7 @@ hostt EH callbacks.  Callbacks may be omitted and omitted 
ones are
 considered to fail always.
 
 int (* eh_abort_handler)(struct scsi_cmnd *);
-int (* eh_device_reset_handler)(struct scsi_cmnd *);
+int (* eh_device_reset_handler)(struct scsi_device *);
 int (* eh_bus_reset_handler)(struct Scsi_Host *, int);
 int (* eh_host_reset_handler)(struct Scsi_Host *);
 
diff --git a/Documentation/scsi/scsi_mid_low_api.txt 
b/Documentation/scsi/scsi_mid_low_api.txt
index e2609a63..28dc029 100644
--- a/Documentation/scsi/scsi_mid_low_api.txt
+++ b/Documentation/scsi/scsi_mid_low_api.txt
@@ -874,7 +874,7 @@ Details:
 
 /**
  *  eh_device_reset_handler - issue SCSI device reset
- *  @scp: identifies SCSI device to be reset
+ *  @sdev: identifies SCSI device to be reset
  *
  *  Returns