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: 134840 Delivered-To: patch@linaro.org Received: by 10.46.151.6 with SMTP id r6csp846274lji; Wed, 2 May 2018 08:57:35 -0700 (PDT) X-Google-Smtp-Source: AB8JxZoqjPXzypT4oDeHnKKswTbh9cIHwfkLzgrNVVYi7ZqGIwymiiQJ3V3CwL+ooirsG7g8KWql X-Received: by 10.98.14.7 with SMTP id w7mr19814314pfi.50.1525276655631; Wed, 02 May 2018 08:57:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1525276655; cv=none; d=google.com; s=arc-20160816; b=QuCrneCcjcrhv6vk/siHEGL013dYe2P7wo5UXf/+9bqWOHCHiFqNrdWE67Eb1XJjv2 Q+X2u8gvaiNMNMXFtnWp0Pdqh5XXzcTnIqzmNocEb3cn1s36OLnOhR2eVqWmJkfBGmxC QHAExSZC3s5IqD1sI33UcZPVQxvThfLfwFpFAPYyEDGgLd9UE0fz0o55anvs1e16ja+J E1MFZwB5hxYBjA93qzu4t3PPFIz50BDCBicGcVyN5BTzkMDFiw0/Ndqn3413cVg1Lr8N VH45DKu8JaZ7gt8o0uq6LxYkGrkiwWFOCShVW72KCdK/H1f36BjALnbtBQufOnw65yOE FKEg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:arc-authentication-results; bh=GS9Vnp7u+FOcep9SLpNQFjvo2Okv6LrF0cWTf81dcgs=; b=M+USh6we+tlQ0l9tcRQ4ZB/VsYioriJtAOInIS2RQ7YgU3COmGgfwIv1RRE9xMHNEp DQR1JixvUv30g5sEbxVSSisTY4+jXpNGxLV9YDuNZLWx6hJf8s4OgPUp4xiAI9xEFlm8 Ui99EQcEHl+vFtQUrRkZAAHUL8C0HFhupH64riwAnVbwzMqR6q8vwf5myaMARS6eeqU6 xzUvbhPwg9dOBnR+qFspg3Qb4GjQNw5IdS2OcZ/tmmzaKL1CdgTwC6IhzwnHozJQcgZJ MekBRaGfdm1H1PgfX8c9NDA7dpREBgyiX7h+O/ANga307BdW+mywVfte4uZOYMYPh6u0 blsQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id o184-v6si9681120pga.128.2018.05.02.08.57.35; Wed, 02 May 2018 08:57:35 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752030AbeEBP5d (ORCPT + 29 others); Wed, 2 May 2018 11:57:33 -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-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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(-) -- 1.9.1 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);