From patchwork Wed May 2 15:56:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Garry X-Patchwork-Id: 10376071 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 B9A4960384 for ; Wed, 2 May 2018 16:00:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AC6E228FA6 for ; Wed, 2 May 2018 16:00:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A155328F91; Wed, 2 May 2018 16:00:28 +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=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=unavailable 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 F1C2628FA6 for ; Wed, 2 May 2018 16:00:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751808AbeEBP52 (ORCPT ); Wed, 2 May 2018 11:57:28 -0400 Received: from szxga06-in.huawei.com ([45.249.212.32]:54553 "EHLO huawei.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1751635AbeEBP51 (ORCPT ); Wed, 2 May 2018 11:57:27 -0400 Received: from DGGEMS403-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id 0676F33CD5CD5; Wed, 2 May 2018 23:57:24 +0800 (CST) Received: from localhost.localdomain (10.67.212.75) by DGGEMS403-HUB.china.huawei.com (10.3.19.203) with Microsoft SMTP Server id 14.3.361.1; Wed, 2 May 2018 23:57:16 +0800 From: John Garry To: , CC: , , , Xiang Chen , "John Garry" Subject: [PATCH 02/11] scsi: hisi_sas: Add some checks to avoid free'ing a sas_task twice Date: Wed, 2 May 2018 23:56:25 +0800 Message-ID: <1525276594-92173-3-git-send-email-john.garry@huawei.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1525276594-92173-1-git-send-email-john.garry@huawei.com> References: <1525276594-92173-1-git-send-email-john.garry@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.67.212.75] X-CFilter-Loop: Reflected 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 From: Xiang Chen If the SCSI host enters EH, any pending IO will be processed by SCSI EH. However it is possible that SCSI EH will try to abort the IO and also at the same time the IO completes in the driver. In this situation there is a small changes of freeing the sas_task twice. Then if another IO re-uses freed sas_task before the second time of free'ing sas_task, it is possible that freeing incorrect sas_task. So to avoid this situation, add some checks to crease reliability. The sas_task task state flag SAS_TASK_STATE_ABORTED is used to mutually protect the LLDD and libsas free'ing the task. Signed-off-by: Xiang Chen Signed-off-by: John Garry --- drivers/scsi/hisi_sas/hisi_sas_main.c | 4 ++++ drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 22 +++++++--------------- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 21 +++++++-------------- 3 files changed, 18 insertions(+), 29 deletions(-) diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c index d1a61b1..52746e2 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_main.c +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c @@ -1174,10 +1174,14 @@ static int hisi_sas_abort_task(struct sas_task *task) return TMF_RESP_FUNC_FAILED; } + spin_lock_irqsave(&task->task_state_lock, flags); if (task->task_state_flags & SAS_TASK_STATE_DONE) { + spin_unlock_irqrestore(&task->task_state_lock, flags); rc = TMF_RESP_FUNC_COMPLETE; goto out; } + task->task_state_flags |= SAS_TASK_STATE_ABORTED; + spin_unlock_irqrestore(&task->task_state_lock, flags); sas_dev->dev_status = HISI_SAS_DEV_EH; if (task->lldd_task && task->task_proto & SAS_PROTOCOL_SSP) { diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c index 384e4ef..8ca0044 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c @@ -2386,7 +2386,6 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba, struct hisi_sas_complete_v2_hdr *complete_hdr = &complete_queue[slot->cmplt_queue_slot]; unsigned long flags; - int aborted; if (unlikely(!task || !task->lldd_task || !task->dev)) return -EINVAL; @@ -2396,7 +2395,6 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba, sas_dev = device->lldd_dev; spin_lock_irqsave(&task->task_state_lock, flags); - aborted = task->task_state_flags & SAS_TASK_STATE_ABORTED; task->task_state_flags &= ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR); spin_unlock_irqrestore(&task->task_state_lock, flags); @@ -2404,15 +2402,6 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba, memset(ts, 0, sizeof(*ts)); ts->resp = SAS_TASK_COMPLETE; - if (unlikely(aborted)) { - dev_dbg(dev, "slot_complete: task(%p) aborted\n", task); - ts->stat = SAS_ABORTED_TASK; - spin_lock_irqsave(&hisi_hba->lock, flags); - hisi_sas_slot_task_free(hisi_hba, task, slot); - spin_unlock_irqrestore(&hisi_hba->lock, flags); - return ts->stat; - } - if (unlikely(!sas_dev)) { dev_dbg(dev, "slot complete: port has no device\n"); ts->stat = SAS_PHY_DOWN; @@ -2523,13 +2512,16 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba, } out: + hisi_sas_slot_task_free(hisi_hba, task, slot); + sts = ts->stat; spin_lock_irqsave(&task->task_state_lock, flags); + if (task->task_state_flags & SAS_TASK_STATE_ABORTED) { + spin_unlock_irqrestore(&task->task_state_lock, flags); + dev_info(dev, "slot complete: task(%p) aborted\n", task); + return SAS_ABORTED_TASK; + } task->task_state_flags |= SAS_TASK_STATE_DONE; spin_unlock_irqrestore(&task->task_state_lock, flags); - spin_lock_irqsave(&hisi_hba->lock, flags); - hisi_sas_slot_task_free(hisi_hba, task, slot); - spin_unlock_irqrestore(&hisi_hba->lock, flags); - sts = ts->stat; if (task->task_done) task->task_done(task); diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index afc1242..7346110 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -1576,7 +1576,6 @@ static irqreturn_t fatal_axi_int_v3_hw(int irq_no, void *p) hisi_hba->complete_hdr[slot->cmplt_queue]; struct hisi_sas_complete_v3_hdr *complete_hdr = &complete_queue[slot->cmplt_queue_slot]; - int aborted; unsigned long flags; if (unlikely(!task || !task->lldd_task || !task->dev)) @@ -1587,21 +1586,12 @@ static irqreturn_t fatal_axi_int_v3_hw(int irq_no, void *p) sas_dev = device->lldd_dev; spin_lock_irqsave(&task->task_state_lock, flags); - aborted = task->task_state_flags & SAS_TASK_STATE_ABORTED; task->task_state_flags &= ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR); spin_unlock_irqrestore(&task->task_state_lock, flags); memset(ts, 0, sizeof(*ts)); ts->resp = SAS_TASK_COMPLETE; - if (unlikely(aborted)) { - dev_dbg(dev, "slot complete: task(%p) aborted\n", task); - ts->stat = SAS_ABORTED_TASK; - spin_lock_irqsave(&hisi_hba->lock, flags); - hisi_sas_slot_task_free(hisi_hba, task, slot); - spin_unlock_irqrestore(&hisi_hba->lock, flags); - return ts->stat; - } if (unlikely(!sas_dev)) { dev_dbg(dev, "slot complete: port has not device\n"); @@ -1699,13 +1689,16 @@ static irqreturn_t fatal_axi_int_v3_hw(int irq_no, void *p) } out: + hisi_sas_slot_task_free(hisi_hba, task, slot); + sts = ts->stat; spin_lock_irqsave(&task->task_state_lock, flags); + if (task->task_state_flags & SAS_TASK_STATE_ABORTED) { + spin_unlock_irqrestore(&task->task_state_lock, flags); + dev_info(dev, "slot complete: task(%p) aborted\n", task); + return SAS_ABORTED_TASK; + } task->task_state_flags |= SAS_TASK_STATE_DONE; spin_unlock_irqrestore(&task->task_state_lock, flags); - spin_lock_irqsave(&hisi_hba->lock, flags); - hisi_sas_slot_task_free(hisi_hba, task, slot); - spin_unlock_irqrestore(&hisi_hba->lock, flags); - sts = ts->stat; if (task->task_done) task->task_done(task);