On Mon, Oct 23, 2023 at 11:28:31AM +0200, Hannes Reinecke wrote:
> diff --git a/drivers/scsi/a100u2w.c b/drivers/scsi/a100u2w.c
> index 5c062fb35cf6..dc562da7b2b9 100644
> --- a/drivers/scsi/a100u2w.c
> +++ b/drivers/scsi/a100u2w.c
> @@ -944,16 +944,15 @@ static int inia100_bus_reset(struct Scsi_Host * shost, unsigned int channel)
> /*****************************************************************************
> Function name : inia100_device_reset
> Description : Reset the device
> - Input : host - Pointer to host adapter structure
> + Input : dev - Pointer to scsi device structure
> Output : None.
> Return : pSRB - Pointer to SCSI request block.
> *****************************************************************************/
> -static int inia100_device_reset(struct scsi_cmnd * cmd)
> +static int inia100_device_reset(struct scsi_device * dev)
> { /* I need Host Control Block Information */
> struct orc_host *host;
> - host = (struct orc_host *) cmd->device->host->hostdata;
> - return orc_device_reset(host, cmd->device);
> -
> + host = (struct orc_host *) dev->host->hostdata;
Nitpick: you could use `shost_priv()` here.
> diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
> index 63d95aa8a5f3..289269140e4e 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_os.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
> @@ -4141,30 +4141,28 @@ static int mpi3mr_eh_target_reset(struct scsi_target *starget)
> -static int mpi3mr_eh_dev_reset(struct scsi_cmnd *scmd)
> +static int mpi3mr_eh_dev_reset(struct scsi_device *sdev)
> {
> - struct mpi3mr_ioc *mrioc = shost_priv(scmd->device->host);
> + struct mpi3mr_ioc *mrioc = shost_priv(sdev->host);
> struct mpi3mr_stgt_priv_data *stgt_priv_data;
> struct mpi3mr_sdev_priv_data *sdev_priv_data;
> u16 dev_handle;
> u8 resp_code = 0;
> int retval = FAILED, ret = 0;
>
> - sdev_printk(KERN_INFO, scmd->device,
> - "Attempting Device(lun) Reset! scmd(%p)\n", scmd);
> - scsi_print_command(scmd);
> + sdev_printk(KERN_INFO, sdev,
Nitpick: you can remove the line-break after `sdev,`
> + "Attempting Device(lun) Reset!\n");
>
> - sdev_priv_data = scmd->device->hostdata;
> + sdev_priv_data = sdev->hostdata;
> if (!sdev_priv_data || !sdev_priv_data->tgt_priv_data) {
> - sdev_printk(KERN_INFO, scmd->device,
> + sdev_printk(KERN_INFO, sdev,
> "SCSI device is not available\n");
> retval = SUCCESS;
> goto out;
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 1f3ce2aafed6..e1da6a811dac 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -3376,20 +3376,17 @@ scsih_dev_reset(struct scsi_cmnd *scmd)
> u8 tr_timeout = 30;
> int r;
>
> - struct scsi_target *starget = scmd->device->sdev_target;
> + struct scsi_target *starget = sdev->sdev_target;
> struct MPT3SAS_TARGET *target_priv_data = starget->hostdata;
>
> - sdev_printk(KERN_INFO, scmd->device,
> - "attempting device reset! scmd(0x%p)\n", scmd);
> - _scsih_tm_display_info(ioc, scmd);
> + sdev_printk(KERN_INFO, sdev,
...
> + "attempting device reset!\n");
>
> - sas_device_priv_data = scmd->device->hostdata;
> + sas_device_priv_data = sdev->hostdata;
> diff --git a/drivers/scsi/pcmcia/nsp_cs.h b/drivers/scsi/pcmcia/nsp_cs.h
> index 01c0d571de90..0d03b361ed72 100644
> --- a/drivers/scsi/pcmcia/nsp_cs.h
> +++ b/drivers/scsi/pcmcia/nsp_cs.h
> @@ -297,8 +297,6 @@ static int nsp_show_info (struct seq_file *m,
> static int nsp_queuecommand(struct Scsi_Host *h, struct scsi_cmnd *SCpnt);
>
> /* Error handler */
> -/*static int nsp_eh_abort (struct scsi_cmnd *SCpnt);*/
> -/*static int nsp_eh_device_reset(struct scsi_cmnd *SCpnt);*/
Hmm, this is a bit off-topic; is it? You don't change anything else in this
header. Hmm, maybe because it's the old device-reset handler that still has
`scsi_cmnd` as arg.
> static int nsp_eh_bus_reset (struct Scsi_Host *host, unsigned int channel);
> static int nsp_eh_host_reset (struct Scsi_Host *host);
> static int nsp_bus_reset (nsp_hw_data *data);
> diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
> index 626bc28d20e2..9bd330610d58 100644
> --- a/drivers/scsi/qla1280.c
> +++ b/drivers/scsi/qla1280.c
> @@ -797,18 +797,18 @@ qla1280_wait_for_pending_commands(struct scsi_qla_host *ha, int bus, int target)
> * wait for the results (or time out).
> *
> * Input:
> + * sdev = Linux SCSI device
> * cmd = Linux SCSI command packet of the command that cause the
> * bus reset.
> - * action = error action to take (see action_t)
> *
> * Returns:
> * SUCCESS or FAILED
> *
> **************************************************************************/
> static int
> -qla1280_error_action(struct scsi_cmnd *cmd, enum action action)
> +qla1280_error_action(struct scsi_device *sdev, struct scsi_cmnd *cmd)
> {
> - struct scsi_qla_host *ha;
> + struct scsi_qla_host *ha = shost_priv(sdev->host);
> int bus, target, lun;
> struct srb *sp;
> int i, found;
> @@ -816,14 +816,14 @@ qla1280_error_action(struct scsi_cmnd *cmd, enum action action)
> int wait_for_bus=-1;
> int wait_for_target = -1;
> DECLARE_COMPLETION_ONSTACK(wait);
> + enum action action = cmd ? ABORT_COMMAND : DEVICE_RESET;
>
> ENTER("qla1280_error_action");
>
> - ha = (struct scsi_qla_host *)(CMD_HOST(cmd)->hostdata);
> sp = scsi_cmd_priv(cmd);
That is wrong now that `cmd` can be `NULL`. In that case this will return
something near address 0 and the rest of the function will fall apart.
static inline void *scsi_cmd_priv(struct scsi_cmnd *cmd)
{
return cmd + 1;
}
> - bus = SCSI_BUS_32(cmd);
> - target = SCSI_TCN_32(cmd);
> - lun = SCSI_LUN_32(cmd);
> + bus = sdev->channel;
> + target = sdev->id;
> + lun = sdev->lun;
>
> dprintk(4, "error_action %i, istatus 0x%04x\n", action,
> RD_REG_WORD(&ha->iobase->istatus));
> diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c
> index 36298dbadb14..e1f6004dcd6b 100644
> --- a/drivers/scsi/snic/snic_scsi.c
> +++ b/drivers/scsi/snic/snic_scsi.c
> @@ -2099,6 +2098,7 @@ snic_device_reset(struct scsi_cmnd *sc)
> int start_time = jiffies;
> int ret = FAILED;
> int dr_supp = 0;
> + struct scsi_cmnd tmf_sc, *sc = &tmf_sc;
No quite sure why you need that `tmf_sc` and it's address in `sc`? `sc` is
first used here:
sc = blk_mq_rq_to_pdu(req);
which overwrites the value, and `tmf_sc` is not used at all. Or am I missing
something?
>
> SNIC_SCSI_DBG(shost, "dev_reset\n");
> dr_supp = snic_dev_reset_supported(sdev);
--
Best Regards, Benjamin Block / Linux on IBM Z Kernel Development
IBM Deutschland Research & Development GmbH / https://www.ibm.com/privacy
Vors. Aufs.-R.: Gregor Pillen / Geschäftsführung: David Faller
Sitz der Ges.: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294
