From patchwork Wed Mar 15 21:58:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian King X-Patchwork-Id: 9626789 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B07C560244 for ; Wed, 15 Mar 2017 21:58:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A24F0283C9 for ; Wed, 15 Mar 2017 21:58:48 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9730728433; Wed, 15 Mar 2017 21:58:48 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 10306283C9 for ; Wed, 15 Mar 2017 21:58:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753993AbdCOV6r (ORCPT ); Wed, 15 Mar 2017 17:58:47 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:57453 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753895AbdCOV6p (ORCPT ); Wed, 15 Mar 2017 17:58:45 -0400 Received: from pps.filterd (m0098417.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v2FLsT0T124193 for ; Wed, 15 Mar 2017 17:58:44 -0400 Received: from e12.ny.us.ibm.com (e12.ny.us.ibm.com [129.33.205.202]) by mx0a-001b2d01.pphosted.com with ESMTP id 297bu6nanp-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 15 Mar 2017 17:58:44 -0400 Received: from localhost by e12.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 15 Mar 2017 17:58:43 -0400 Received: from b03cxnp08026.gho.boulder.ibm.com (9.17.130.18) by e12.ny.us.ibm.com (146.89.104.199) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 15 Mar 2017 17:58:41 -0400 Received: from b03ledav005.gho.boulder.ibm.com (b03ledav005.gho.boulder.ibm.com [9.17.130.236]) by b03cxnp08026.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v2FLweuV6554074; Wed, 15 Mar 2017 14:58:40 -0700 Received: from b03ledav005.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id B6863BE042; Wed, 15 Mar 2017 15:58:40 -0600 (MDT) Received: from localhost.localdomain (unknown [9.10.86.20]) by b03ledav005.gho.boulder.ibm.com (Postfix) with ESMTP id 51F71BE03B; Wed, 15 Mar 2017 15:58:40 -0600 (MDT) Subject: [PATCH 4/6] ipr: Error path locking fixes To: James.Bottomley@HansenPartnership.com Cc: martin.petersen@oracle.com, linux-scsi@vger.kernel.org, wenxiong@linux.vnet.ibm.com, brking@linux.vnet.ibm.com From: Brian King Date: Wed, 15 Mar 2017 16:58:39 -0500 X-TM-AS-GCONF: 00 x-cbid: 17031521-0048-0000-0000-0000012B6DCE X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006789; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000206; SDB=6.00834348; UDB=6.00409731; IPR=6.00612004; BA=6.00005214; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00014664; XFM=3.00000013; UTC=2017-03-15 21:58:42 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17031521-0049-0000-0000-00003FB74C2E Message-Id: <20170315215840.51F71BE03B@b03ledav005.gho.boulder.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-03-15_09:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=3 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1702020001 definitions=main-1703150166 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 --- drivers/scsi/ipr.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 93 insertions(+), 13 deletions(-) diff -puN drivers/scsi/ipr.c~ipr_eh_locking_fixes drivers/scsi/ipr.c --- linux-2.6.git/drivers/scsi/ipr.c~ipr_eh_locking_fixes 2017-03-13 14:56:49.994452448 -0500 +++ linux-2.6.git-bjking1/drivers/scsi/ipr.c 2017-03-13 14:56:50.000452424 -0500 @@ -820,7 +820,7 @@ static int ipr_set_pcix_cmd_reg(struct i } /** - * 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 i * 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_ } /** - * 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_ * 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_ } /** + * 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_ 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_io } /** - * 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_io * 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_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( } /** - * 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( * 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_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 ip 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 u32 masked_ioasc = ioasc & IPR_IOASC_IOASC_MASK; if (!res) { - ipr_scsi_eh_done(ipr_cmd); + __ipr_scsi_eh_done(ipr_cmd); return; }