From f646f325a829d73abc708088d2b67cd11b775fdf Mon Sep 17 00:00:00 2001 From: Brian King Date: Wed, 15 Mar 2017 16:58:39 -0500 Subject: [PATCH] scsi: ipr: Error path locking fixes This patch closes up some potential race conditions observed in the error handling paths in ipr while debugging an issue resulting in a hang with SATA error handling. These patches ensure we are holding the correct lock when adding and removing commands from the free and pending queues in some error scenarios. Signed-off-by: Brian King Reviewed-by: Wendy Xiong Tested-by: Wendy Xiong Signed-off-by: Martin K. Petersen --- drivers/scsi/ipr.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 93 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 19162d7..95f1e64 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -820,7 +820,7 @@ static int ipr_set_pcix_cmd_reg(struct ipr_ioa_cfg *ioa_cfg) } /** - * ipr_sata_eh_done - done function for aborted SATA commands + * __ipr_sata_eh_done - done function for aborted SATA commands * @ipr_cmd: ipr command struct * * This function is invoked for ops generated to SATA @@ -829,7 +829,7 @@ static int ipr_set_pcix_cmd_reg(struct ipr_ioa_cfg *ioa_cfg) * Return value: * none **/ -static void ipr_sata_eh_done(struct ipr_cmnd *ipr_cmd) +static void __ipr_sata_eh_done(struct ipr_cmnd *ipr_cmd) { struct ata_queued_cmd *qc = ipr_cmd->qc; struct ipr_sata_port *sata_port = qc->ap->private_data; @@ -843,7 +843,27 @@ static void ipr_sata_eh_done(struct ipr_cmnd *ipr_cmd) } /** - * ipr_scsi_eh_done - mid-layer done function for aborted ops + * ipr_sata_eh_done - done function for aborted SATA commands + * @ipr_cmd: ipr command struct + * + * This function is invoked for ops generated to SATA + * devices which are being aborted. + * + * Return value: + * none + **/ +static void ipr_sata_eh_done(struct ipr_cmnd *ipr_cmd) +{ + struct ipr_hrr_queue *hrrq = ipr_cmd->hrrq; + unsigned long hrrq_flags; + + spin_lock_irqsave(&hrrq->_lock, hrrq_flags); + __ipr_sata_eh_done(ipr_cmd); + spin_unlock_irqrestore(&hrrq->_lock, hrrq_flags); +} + +/** + * __ipr_scsi_eh_done - mid-layer done function for aborted ops * @ipr_cmd: ipr command struct * * This function is invoked by the interrupt handler for @@ -852,7 +872,7 @@ static void ipr_sata_eh_done(struct ipr_cmnd *ipr_cmd) * Return value: * none **/ -static void ipr_scsi_eh_done(struct ipr_cmnd *ipr_cmd) +static void __ipr_scsi_eh_done(struct ipr_cmnd *ipr_cmd) { struct scsi_cmnd *scsi_cmd = ipr_cmd->scsi_cmd; @@ -866,6 +886,26 @@ static void ipr_scsi_eh_done(struct ipr_cmnd *ipr_cmd) } /** + * ipr_scsi_eh_done - mid-layer done function for aborted ops + * @ipr_cmd: ipr command struct + * + * This function is invoked by the interrupt handler for + * ops generated by the SCSI mid-layer which are being aborted. + * + * Return value: + * none + **/ +static void ipr_scsi_eh_done(struct ipr_cmnd *ipr_cmd) +{ + unsigned long hrrq_flags; + struct ipr_hrr_queue *hrrq = ipr_cmd->hrrq; + + spin_lock_irqsave(&hrrq->_lock, hrrq_flags); + __ipr_scsi_eh_done(ipr_cmd); + spin_unlock_irqrestore(&hrrq->_lock, hrrq_flags); +} + +/** * ipr_fail_all_ops - Fails all outstanding ops. * @ioa_cfg: ioa config struct * @@ -892,9 +932,9 @@ static void ipr_fail_all_ops(struct ipr_ioa_cfg *ioa_cfg) cpu_to_be32(IPR_DRIVER_ILID); if (ipr_cmd->scsi_cmd) - ipr_cmd->done = ipr_scsi_eh_done; + ipr_cmd->done = __ipr_scsi_eh_done; else if (ipr_cmd->qc) - ipr_cmd->done = ipr_sata_eh_done; + ipr_cmd->done = __ipr_sata_eh_done; ipr_trc_hook(ipr_cmd, IPR_TRACE_FINISH, IPR_IOASC_IOA_WAS_RESET); @@ -5948,7 +5988,7 @@ static int ipr_build_ioadl(struct ipr_ioa_cfg *ioa_cfg, } /** - * ipr_erp_done - Process completion of ERP for a device + * __ipr_erp_done - Process completion of ERP for a device * @ipr_cmd: ipr command struct * * This function copies the sense buffer into the scsi_cmd @@ -5957,7 +5997,7 @@ static int ipr_build_ioadl(struct ipr_ioa_cfg *ioa_cfg, * Return value: * nothing **/ -static void ipr_erp_done(struct ipr_cmnd *ipr_cmd) +static void __ipr_erp_done(struct ipr_cmnd *ipr_cmd) { struct scsi_cmnd *scsi_cmd = ipr_cmd->scsi_cmd; struct ipr_resource_entry *res = scsi_cmd->device->hostdata; @@ -5985,6 +6025,26 @@ static void ipr_erp_done(struct ipr_cmnd *ipr_cmd) } /** + * ipr_erp_done - Process completion of ERP for a device + * @ipr_cmd: ipr command struct + * + * This function copies the sense buffer into the scsi_cmd + * struct and pushes the scsi_done function. + * + * Return value: + * nothing + **/ +static void ipr_erp_done(struct ipr_cmnd *ipr_cmd) +{ + struct ipr_hrr_queue *hrrq = ipr_cmd->hrrq; + unsigned long hrrq_flags; + + spin_lock_irqsave(&hrrq->_lock, hrrq_flags); + __ipr_erp_done(ipr_cmd); + spin_unlock_irqrestore(&hrrq->_lock, hrrq_flags); +} + +/** * ipr_reinit_ipr_cmnd_for_erp - Re-initialize a cmnd block to be used for ERP * @ipr_cmd: ipr command struct * @@ -6016,7 +6076,7 @@ static void ipr_reinit_ipr_cmnd_for_erp(struct ipr_cmnd *ipr_cmd) } /** - * ipr_erp_request_sense - Send request sense to a device + * __ipr_erp_request_sense - Send request sense to a device * @ipr_cmd: ipr command struct * * This function sends a request sense to a device as a result @@ -6025,13 +6085,13 @@ static void ipr_reinit_ipr_cmnd_for_erp(struct ipr_cmnd *ipr_cmd) * Return value: * nothing **/ -static void ipr_erp_request_sense(struct ipr_cmnd *ipr_cmd) +static void __ipr_erp_request_sense(struct ipr_cmnd *ipr_cmd) { struct ipr_cmd_pkt *cmd_pkt = &ipr_cmd->ioarcb.cmd_pkt; u32 ioasc = be32_to_cpu(ipr_cmd->s.ioasa.hdr.ioasc); if (IPR_IOASC_SENSE_KEY(ioasc) > 0) { - ipr_erp_done(ipr_cmd); + __ipr_erp_done(ipr_cmd); return; } @@ -6052,6 +6112,26 @@ static void ipr_erp_request_sense(struct ipr_cmnd *ipr_cmd) } /** + * ipr_erp_request_sense - Send request sense to a device + * @ipr_cmd: ipr command struct + * + * This function sends a request sense to a device as a result + * of a check condition. + * + * Return value: + * nothing + **/ +static void ipr_erp_request_sense(struct ipr_cmnd *ipr_cmd) +{ + struct ipr_hrr_queue *hrrq = ipr_cmd->hrrq; + unsigned long hrrq_flags; + + spin_lock_irqsave(&hrrq->_lock, hrrq_flags); + __ipr_erp_request_sense(ipr_cmd); + spin_unlock_irqrestore(&hrrq->_lock, hrrq_flags); +} + +/** * ipr_erp_cancel_all - Send cancel all to a device * @ipr_cmd: ipr command struct * @@ -6074,7 +6154,7 @@ static void ipr_erp_cancel_all(struct ipr_cmnd *ipr_cmd) ipr_reinit_ipr_cmnd_for_erp(ipr_cmd); if (!scsi_cmd->device->simple_tags) { - ipr_erp_request_sense(ipr_cmd); + __ipr_erp_request_sense(ipr_cmd); return; } @@ -6294,7 +6374,7 @@ static void ipr_erp_start(struct ipr_ioa_cfg *ioa_cfg, u32 masked_ioasc = ioasc & IPR_IOASC_IOASC_MASK; if (!res) { - ipr_scsi_eh_done(ipr_cmd); + __ipr_scsi_eh_done(ipr_cmd); return; } -- 2.7.4